Как стать автором
Поиск
Написать публикацию
Обновить

Комментарии 28

Есть несколько вещей в написании кода которые меня бесят. Ну, на самом деле не несколько. :), чем старше становлюсь, тем больше и больше таких вещей, характер не лучшает. Но в топе — когда девелоперы пишут тесты к private методам.

Как то Вы слишком безапелляционно.
Да-да, все знают, писать тесты к private методам не надо, если ты это делаешь, то что-то не так с дизайном, и так далее

Все — это кто?

Например, вот совсем недавно писал. Сложная функция парсинга/расчета. Однозначно нужна только в одном классе (поэтому private). А отладить ее нужно на разных входных данных. (в процессе отладки на разных данных заодно и JUnit тест на нее появляется)

На публичную функцию, использующую эту приватную накладывать специфичные тесты как то не очень. На нее другие тесты, специфичные для ее логики.

Дергается в JUnit private функция через reflection… Что Вы во второй половине статьи и приводите, предлагаю «свою обертку» для.
Можно конечно свой велосипед написать, но… И так и так не айс получается.

Какой велосипед?! Вызов через reflection метода — это велоcипед?
А вот представьте, что для доступа к private элементам класса — методам, полям, у нас есть интерфейс. Что за фигня, вы скажете, совсем парень заработался.

Что за странные подходы? На каждый частный случай писать чуть ли не свой специфичный фреймворк для разового использования вместо 3-строчек вызова приватного метода класса. Заняться больше нечем что ли…
«Разового использования» — потому что задача отладки/junit тестов на приватные функции редкая.

И откуда столько возмущения на целую статью?
довольно сумбурного возмущения…

Однозначно нужна только в одном классе (поэтому private). А отладить ее нужно на разных входных данных.

Вынести в отдельный package-private класс с package-private же методом парсинга/расчёта который обложить тестами. В изначальном приватном методе, используя экземпляр этого класса, вызвать расчёт. Экземпляр можно создавать прямо внутри приватного метода.


Если совсем упарываться, то package-private класс сделать внутренним для исходного.


А вообще приватные методы нужно тестировать посредством тестирования публичных, их использующих.

>А вообще приватные методы нужно тестировать посредством тестирования публичных, их использующих.
Ну у них далеко не всегда одинаковая логика. Ну т.е. эта идея — она имеет право на существование, но вряд ли такое решение универсально.

А про все остальное возникает один вопрос — а нафига? Ну т.е., с чем мы боремся, проводя такое изменение? Какие реальные недостатки есть у тестирования приватного метода?
НЛО прилетело и опубликовало эту надпись здесь
>Тесты не должны падать, если логика в программе не поменялась.
Ну, идея мне кажется немного фантастическая, но как аргумент — годится. Выглядит как пусть и маловероятный, но реалистичный случай.

Но опять же — я не вижу проблемы в самом тестировании таких методов. Если метод заинлайнили — тест упал. Мы убедились, что это следствие рефакторинга, и тест удалили как неактуальный. Такая проблема в целом не стоит выеденного яйца. Некрасиво, да — но настолько маловажно…
НЛО прилетело и опубликовало эту надпись здесь
Ну, тут есть только одно соображение — что приватные методы могут содержать совсем не ту логику, что доступна из публичных. Ну так, просто что в голову пришло — публичный метод будет считать доходность бонда, а один из приватных — решать уравнение. Понятно что у них разные тестовые сценарии, в общем случае.

Но в целом да, я согласен, что если в приватной логике вообще есть какой-то смысл — то лучше ее вынести в другое место, и там сделать публичной.

Послушайте, друзья, идея статьи была не в том, что тестировать или не тестировать private методы/поля, а удобно ли это делать testprivates. Посмотрите хотя бы на https://github.com/dimpon/testprivate

Ну, вы так это завуалировали в тексте, что мы прицепились к другому, и это теперь обсуждаем :)

И, да, вызов метода через рефлексию — это низкоуровневый костыль. Иногда удобный, но всё-таки костыль.

НЛО прилетело и опубликовало эту надпись здесь
Добрый день! mmMike давайте по-порядку.
Насчет тестирования private методов есть разные мнения, и я как раз оправдываю ситуации когда их тестируют. Но ряд авторов считает, что так делать не надо www.infoq.com/news/2008/01/private-methods-tdd-design

Я считаю, что выставлять наружу детали реализации (package-private методы) это плохо. Это ухудшает readability. И эти методы рано или поздно начнут дергать не только из тестов. К сожалению, в моем текущем проекте (10-летний монолит без четкой архитектуры, разрабатывается 3-мя командами, нет code ownership) это стандартная практика.

reflection в одном случае это нормально. в сто одном — уже не очень. а еще каждый делает это по-своему. А представьте, что надо засетить 10 private полей через reflection и потом протестировать один public метод?
 ClassC obj = new ClassC();

 Field aField = ClassC.class.getDeclaredField("a");
 aField.setAccessible(true);
 aField.set(obj,42);
// и так 10 раз

а если потом прочитать их нужно? а еще NoSuchFieldException и IllegalAccessException ловить.

Идея статьи — не писать фреймворк, а использовать библиотеку testprivate и описывать доступ к private полям/методам через интерфейс. И делать это одинаково во всем проекте.

Вы делаете интерфейс прям в тесте, добавляее методы совпадающие по сигнатуре с private методами и getters/setters для private полей и как будто кастите свой объект к этому интерфейсу.

Я считаю, что выставлять наружу детали реализации (package-private методы) это плохо.

Полностью согласен. Когда я вижу в коде private у метода, я точно знаю что он только для этого класса.

Вы делаете интерфейс прям в тесте, добавляее методы совпадающие по сигнатуре с private методами и getters/setters для private полей и как будто кастите свой объект к этому интерфейсу.

Да. специально:
  1. этим я говорю будущим редактором кода «этот метод отлажен. детерминирован и трогать его не нужно.» Если трогаете, то будьте добры делать это осознано и JUnit тест поменяйте заодно.
  2. JUnit тесты на private методы это очень частный и редкий случай. Использование какой то внешней библиотеки, как замены 3-х строк вызова через голый reflection (что понятно всем ибо базовая Java) подразумевает вникание (*) в эту библиотеку для разовой ситуации. Лично меня, такое бы раздражало в чужом коде.


(*) — Может я консервативен, но, например, предпочитаю вставить в код парсинг PEM файла руками или пасинг строки subj x509 (и то и другое — несколько строк), чем тянуть bouncy castle для этого. Если, кончено, ничего более не требуется от bouncy castle.

А представьте, что надо засетить 10 private полей через reflection

А вот это уже не очень хорошая практика. Ладно частные случаи с тестами на приватные методы. Но превращать это в общую практику… как то коряво.

Собственно противоречие в статье между «не хорошо писать тесты на приватные методы» и «вот вам библиотека для тестирования приватных методов» побудило написать комментарий.
Все — это кто?

Кент Бек, например.

image

Кому как не создателю TDD знать, что стоит тестировать, а что нет.

Сложную функцию парсинга логичнее было бы вынести в отдельный класс так как не делать этого нет ни единой причины.

Ок, думаешь ты, сейчас рукава засучим, вынесем все public методы в интерфейс, helper-ы, их в отдельный package — короче распилим это безумство на части согласно SRP. Но тут же обнаруживаешь, что package-private методы давно дергаются из других мест.

Такие мысли в головку приходят (все переделать), когда дофига свободного времени.

Ну и опять же… Стоматолог глядя на пломбу сделанную самой же пару лет назад мне говорила «ну кто это вам ее так паршиво сделал».

«Не судите и не судимы будите.» «Работаете — не трогай».
Да к сожалению не всегда. Бывает надо изменить логику, а ты просто не можешь понять как это работает, потому что там 30 private и package-private методов, каждый в 3 экрана, которые друг друга вызывают. Вот и начинаешь разносить код, потому что всех кейсов проследить не можешь.
Вот и начинаешь разносить код, потому что всех кейсов проследить не можешь.

Рефакторинг кода, когда не понимаешь что код делает, позволит получить рабочий код с функциональностью аналогичной исходному?

Оригинальная мысль :)

Сначала надо тестами код покрыть, а потом рефакторить. И я не писал, что не понимаешь, что код делает — понимаешь конечно, просто какие-то ветвления, аспекты можешь упустить.

Простите, но мне кажется, автор неверно сформулировал свою претензию.

Проблема же не в том, что на приватные методы пишутся тесты. Ну пишутся и пишутся — что и с того? Проблема, как я понимаю, в том, что методы сначала делаются не приватными (для тестирования), и потом применяются не там и не так, как было задумано. Я прав?
Добрый день! да, правы, я ж написал «Когда они ради этого убирают слово private у метода и метод становится package-private.». С тестирование private методов я начал чтобы разжечь дискуссию, конечно это opinion-based. Но ряд авторов считает, что так делать не надо. Основная идея, что класс поддерживает контракт, предоставляемый public методами, его и надо тестировать. А все остальное — внутренние детали.
вот например:
www.infoq.com/news/2008/01/private-methods-tdd-design
и там длинное обсуждение.
Не, ну так было же выше возражение — у вас есть сложная внутренняя (приватная) логика, встречается ровно в одном месте. И что, ее теперь не тестировать что-ли?

Ну, наверно эта логика все-таки доступна через public метод. Но если очень хочется конечно можно тестировать, только не делать ее package-private, а то оглянуться не успеете как она будет не в одном месте вызывается.

Тестовые данные для тестирования логики публичного метода — N случаев/наборах данных.
Тестовые данные для тестирования логики приватного метода (предположим это довольно сложный валидатор, возвращающий boolean) — M наборов данных.

Тестирование публичного метода одновременно с тестированием внутреннего приватного метода — набор из N и M входных данных. Не очень удобно и обозримо. Мешается в кучу тесты разного плана.
НЛО прилетело и опубликовало эту надпись здесь
Выносить метод, который используется только один раз и только в одном классе в отдельный класс — по мне так это хуже решение. Тем более, что метод не более 100 строк.
Наличие же отдельного класса предполагает его переиспользование. Зачем выносить в отдельный класс метод, если он небольшой и его использование не предполагается нигде более принципиально?

Да, я видел код в котором в классах не более 30..80 строк и все это размазано по 10ку классов в пакете. И размазано не по принципу полезности для восприятия, а по принципу «классы должны быть маленькие».
Кто то вот разработчику джуну сказал что это круто и так надо делать. (на курсах повидимому)

Продираться через эту размазанную логику в разных файлах — это ужас и вырви глаз.

Если платят за объем кода, то так можно сделать. Раздув код раза в 3 больше только за счет одних только стандартных заголовков файлов классов и пр.
НЛО прилетело и опубликовало эту надпись здесь

Вынос различных аспектов логики в разные классы оправдан также с точки зрения тестирования. Удобнее мокнуть класс-валидатор, чем private метод.


Оставляя всю логику в одном классе мы скатывается к процедурному стилю, о котором я писал в статье.
И очень наивно полагать, что логика будет использоваться только в одном месте. Вы упомянули функцию парсинга- так завтра появится новое требование, что для нового клиента парсинг отличается на пару символов, отсупов итд. и время — на имплементацию — час. В этом случае не будет времени ни на какой рефактонинг и прогматичный разработчик просто скопирует старый класс подправить пару строчек. Пожар потушен- но в долгосрочной перспективе- деградация кода.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации