Комментарии 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
И, да, вызов метода через рефлексию — это низкоуровневый костыль. Иногда удобный, но всё-таки костыль.
Насчет тестирования 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 полей и как будто кастите свой объект к этому интерфейсу.
Да. специально:
- этим я говорю будущим редактором кода «этот метод отлажен. детерминирован и трогать его не нужно.» Если трогаете, то будьте добры делать это осознано и JUnit тест поменяйте заодно.
- JUnit тесты на private методы это очень частный и редкий случай. Использование какой то внешней библиотеки, как замены 3-х строк вызова через голый reflection (что понятно всем ибо базовая Java) подразумевает вникание (*) в эту библиотеку для разовой ситуации. Лично меня, такое бы раздражало в чужом коде.
(*) — Может я консервативен, но, например, предпочитаю вставить в код парсинг PEM файла руками или пасинг строки subj x509 (и то и другое — несколько строк), чем тянуть bouncy castle для этого. Если, кончено, ничего более не требуется от bouncy castle.
А представьте, что надо засетить 10 private полей через reflection
А вот это уже не очень хорошая практика. Ладно частные случаи с тестами на приватные методы. Но превращать это в общую практику… как то коряво.
Собственно противоречие в статье между «не хорошо писать тесты на приватные методы» и «вот вам библиотека для тестирования приватных методов» побудило написать комментарий.
Сложную функцию парсинга логичнее было бы вынести в отдельный класс так как не делать этого нет ни единой причины.
Ок, думаешь ты, сейчас рукава засучим, вынесем все public методы в интерфейс, helper-ы, их в отдельный package — короче распилим это безумство на части согласно SRP. Но тут же обнаруживаешь, что package-private методы давно дергаются из других мест.
Такие мысли в головку приходят (все переделать), когда дофига свободного времени.
Ну и опять же… Стоматолог глядя на пломбу сделанную самой же пару лет назад мне говорила «ну кто это вам ее так паршиво сделал».
«Не судите и не судимы будите.» «Работаете — не трогай».
Вот и начинаешь разносить код, потому что всех кейсов проследить не можешь.
Рефакторинг кода, когда не понимаешь что код делает, позволит получить рабочий код с функциональностью аналогичной исходному?
Оригинальная мысль :)
Проблема же не в том, что на приватные методы пишутся тесты. Ну пишутся и пишутся — что и с того? Проблема, как я понимаю, в том, что методы сначала делаются не приватными (для тестирования), и потом применяются не там и не так, как было задумано. Я прав?
вот например:
www.infoq.com/news/2008/01/private-methods-tdd-design
и там длинное обсуждение.
Ну, наверно эта логика все-таки доступна через public метод. Но если очень хочется конечно можно тестировать, только не делать ее package-private, а то оглянуться не успеете как она будет не в одном месте вызывается.
Тестовые данные для тестирования логики приватного метода (предположим это довольно сложный валидатор, возвращающий boolean) — M наборов данных.
Тестирование публичного метода одновременно с тестированием внутреннего приватного метода — набор из N и M входных данных. Не очень удобно и обозримо. Мешается в кучу тесты разного плана.
Наличие же отдельного класса предполагает его переиспользование. Зачем выносить в отдельный класс метод, если он небольшой и его использование не предполагается нигде более принципиально?
Да, я видел код в котором в классах не более 30..80 строк и все это размазано по 10ку классов в пакете. И размазано не по принципу полезности для восприятия, а по принципу «классы должны быть маленькие».
Кто то вот разработчику джуну сказал что это круто и так надо делать. (на курсах повидимому)
Продираться через эту размазанную логику в разных файлах — это ужас и вырви глаз.
Если платят за объем кода, то так можно сделать. Раздув код раза в 3 больше только за счет одних только стандартных заголовков файлов классов и пр.
Вынос различных аспектов логики в разные классы оправдан также с точки зрения тестирования. Удобнее мокнуть класс-валидатор, чем private метод.
Оставляя всю логику в одном классе мы скатывается к процедурному стилю, о котором я писал в статье.
И очень наивно полагать, что логика будет использоваться только в одном месте. Вы упомянули функцию парсинга- так завтра появится новое требование, что для нового клиента парсинг отличается на пару символов, отсупов итд. и время — на имплементацию — час. В этом случае не будет времени ни на какой рефактонинг и прогматичный разработчик просто скопирует старый класс подправить пару строчек. Пожар потушен- но в долгосрочной перспективе- деградация кода.
Не нарушайте мою приватность