Добрый день! Статья будет про один совершенно незначительный момент в кодировании.
Например, многие мои коллеги сказали - Doch, das ist ganz normal! "А, чё, все норм, нашел к чему придраться!" Но, тем не менее.
Есть несколько вещей в написании кода, которые мне не нравятся. Ну, на самом деле не несколько, чем старше становлюсь, тем больше и больше таких вещей, характер не лучшает. Но в топе - когда девелоперы пишут тесты к private методам. А, нет! Когда они ради этого убирают слово private у метода и метод становится package-private.
Вот, ты такой ревьювишь класс, строк этак на 3К, написанный в лучших традициях процедурного программирования, а там 10 методов public, 10 private, а еще 15 package-private. И про интерфейсы мы не слышали - это ж сервис, давай так прям так и инжектить! Ок, думаешь ты, сейчас рукава засучим, вынесем все public методы в интерфейс, helper-ы, их в отдельный package - короче распилим это безумство на части согласно SRP. Но тут же обнаруживаешь, что package-private методы давно дергаются из других мест. Дык у нас все классы в один package свалены! If something can be misused, it will be misused for sure, как говорится.
Да-да, все знают, писать тесты к private методам не надо, если ты это делаешь, то что-то не так с дизайном, и так далее. Но, блин, люди все равно это делают, и надо сказать, не без причин. Представьте, что упомянутый выше класс вообще был с zero test coverage. А рефакторить надо. Вот так и появляются package-private методы. Но как же нам быть - вон и Whitebox из Mockito выпилили, конечно в PowerMock он остался, но ради этого тащить PowerMock в проект, да и синтаксис не лучше прямого вызова через reflection. Можно конечно свой велосипед написать, но... И так и так не айс получается.
А вот представьте, что для доступа к private элементам класса - методам, полям, у нас есть интерфейс. Что за фигня, вы скажете, совсем парень заработался. А вот так:
Есть класс:
public class ObjectWithPrivates {
private final AtomicInteger count = new AtomicInteger(0);
private String name;
private String methodToTest(String in) {
return name + in + count.incrementAndGet();
}
}
Тест:
interface TestPrivates {
void setName(String name);
String methodToTest(String in);
AtomicInteger getCount();
}
@Test
void testPrivates() {
ObjectWithPrivates obj = new ObjectWithPrivates();
TestPrivates objWithPrivates = API.lookupPrivatesIn(obj)
.usingInterface(TestPrivates.class);
objWithPrivates.setName("Andromeda");
String in = objWithPrivates.methodToTest("in");
AtomicInteger count = objWithPrivates.getCount();
Assertions.assertEquals("Andromedain1", in);
Assertions.assertEquals(1, count.get());
}
Как видите, с помощью интерфейса мы можем дергать private методы, и получать доступ к private полям. Ну и по коду сразу можно сказать, что происходит. Вы делаете интерфейс прям в тесте, добавляее методы совпадающие по сигнатуре с private методами и getters/setters для private полей и как будто кастите свой объект к этому интерфейсу.
Имейте ввиду, что все манипуляции происходят с obj и можно, например, засетить все private поля у obj и потом дернуть public метод.
Можно получить доступ к private полям/методам родительского класса:
public class ObjectWithPrivates {
private static String name;
}
public class ObjectWithPrivatesSub extends ObjectWithPrivates {
private static String name;
}
interface TestPrivates {
void setName(String name);
}
@Test
void testPrivates() {
ObjectWithPrivatesSub obj = new ObjectWithPrivatesSub();
TestPrivates accessToPrivates = API.lookupPrivatesIn(obj)
.lookupInSuperclass()
.usingInterface(TestPrivates.class);
accessToPrivates.setName("Vasya Pupkin");
:
}
А можно и статические методы/поля дергать:
public class ObjectWithPrivates {
private static String name;
private static String whatIsYourName(){
return name;
}
}
interface TestPrivates {
void setName(String name);
private String whatIsYourName(){
}
@Test
void testPrivates() {
TestPrivates accessToPrivates = API.lookupPrivatesIn(ObjectWithPrivates.class)
.usingInterface(TestPrivates.class);
accessToPrivates.setName("Vasya Pupkin");
Assertions.assertEquals("Vasya Pupkin", accessToPrivates.whatIsYourName());
:
}
А можно и объект создать через private конструктор.
public class ClassC {
:
:
private ClassC(int a, String b, Object c, Long d) {
:
}
@Test
void createObject() {
ClassC classC = API.createInstanceOf(ClassC.class)
.withArguments(new Integer(5), "yo!", new Long(123L), 15L);
:
}
При этом анализируется количество параметром и их типы и ищется подходящий конструктор.
Под капотом там стандартный reflection и Dynamic Proxy, никаких дополнительных зависимостей.
Вот ссылка на проект testprivates. Использовать только для тестов.
Всех с наступающим, не болейте!
mmMike
Как то Вы слишком безапелляционно.
Все — это кто?
Например, вот совсем недавно писал. Сложная функция парсинга/расчета. Однозначно нужна только в одном классе (поэтому private). А отладить ее нужно на разных входных данных. (в процессе отладки на разных данных заодно и JUnit тест на нее появляется)
На публичную функцию, использующую эту приватную накладывать специфичные тесты как то не очень. На нее другие тесты, специфичные для ее логики.
Дергается в JUnit private функция через reflection… Что Вы во второй половине статьи и приводите, предлагаю «свою обертку» для.
Какой велосипед?! Вызов через reflection метода — это велоcипед?
Что за странные подходы? На каждый частный случай писать чуть ли не свой специфичный фреймворк для разового использования вместо 3-строчек вызова приватного метода класса. Заняться больше нечем что ли…
«Разового использования» — потому что задача отладки/junit тестов на приватные функции редкая.
И откуда столько возмущения на целую статью?
довольно сумбурного возмущения…
aleksandy
Вынести в отдельный package-private класс с package-private же методом парсинга/расчёта который обложить тестами. В изначальном приватном методе, используя экземпляр этого класса, вызвать расчёт. Экземпляр можно создавать прямо внутри приватного метода.
Если совсем упарываться, то package-private класс сделать внутренним для исходного.
А вообще приватные методы нужно тестировать посредством тестирования публичных, их использующих.
sshikov
>А вообще приватные методы нужно тестировать посредством тестирования публичных, их использующих.
Ну у них далеко не всегда одинаковая логика. Ну т.е. эта идея — она имеет право на существование, но вряд ли такое решение универсально.
А про все остальное возникает один вопрос — а нафига? Ну т.е., с чем мы боремся, проводя такое изменение? Какие реальные недостатки есть у тестирования приватного метода?
oxidmod
Для клиента нет разницы какая там логика в приватных методах. Приватные методы по большому счету просто инструмент, который позволяет в общем алгоритме выделить отдельные логические кусочки. чтоб новый человек глядя на код публичного метода видел там ряд вызовов приватных методов, имена которых дают ему понять что в общих чертах происходит. Также, приватные методы позволяют не дублировать одинаковую логику, которая нужна в двух и более публичных методах, но не более того.
Вот подумайте, перестанет ли ваша программа работать, если вы удалите из нее все приватные методы, а код в них содержащийся просто «заинлайните»? Не перестанет очевидно, но тесты упадут. Тесты не должны падать, если логика в программе не поменялась.
sshikov
>Тесты не должны падать, если логика в программе не поменялась.
Ну, идея мне кажется немного фантастическая, но как аргумент — годится. Выглядит как пусть и маловероятный, но реалистичный случай.
Но опять же — я не вижу проблемы в самом тестировании таких методов. Если метод заинлайнили — тест упал. Мы убедились, что это следствие рефакторинга, и тест удалили как неактуальный. Такая проблема в целом не стоит выеденного яйца. Некрасиво, да — но настолько маловажно…
oxidmod
Вот как раз просто удалив этот тест вы ухудшите тестовое покрытие. Ведь ваши тесты публичного метода не покрывали все кейсы, потому что часть логики тестировалось отдельно в тесте приватного метода.
Если изначально отказаться от тестов приватных методов, а тестировать только публичные — то такого происходить не будет. Да, тесты будут выглядеть более громоздко. А может это повод подумать над реализацией как таковой. Действительно ли вся эта логика должна быть в одном месте? Выше по ветке была отсылка, что в приватном методе сложная логика парсинга\рассчетов. Может стоит эту логику вынести в отдельный парсер\калькулятор? SRP и вот это вот все, ведь не зря говорят, что тесты заставляют писать более качественный код
sshikov
Ну, тут есть только одно соображение — что приватные методы могут содержать совсем не ту логику, что доступна из публичных. Ну так, просто что в голову пришло — публичный метод будет считать доходность бонда, а один из приватных — решать уравнение. Понятно что у них разные тестовые сценарии, в общем случае.
Но в целом да, я согласен, что если в приватной логике вообще есть какой-то смысл — то лучше ее вынести в другое место, и там сделать публичной.
dimpon Автор
Послушайте, друзья, идея статьи была не в том, что тестировать или не тестировать private методы/поля, а удобно ли это делать testprivates. Посмотрите хотя бы на https://github.com/dimpon/testprivate
sshikov
Ну, вы так это завуалировали в тексте, что мы прицепились к другому, и это теперь обсуждаем :)
aleksandy
И, да, вызов метода через рефлексию — это низкоуровневый костыль. Иногда удобный, но всё-таки костыль.
oxidmod
Тестировать нужно результат, а не способ его получения.
Если вы поменяете логику в своем приватном методе, то вам придется править тесты приватного метода + тесты публичного, ведь публичный использует приватный и при смене результата в приватном у вас сломается и тесты публичного метода.
Кроме того, вот вы покрыли разные варианты входных данных для приватного метода. А потом поменяли логику публичного так, так что часть возможных входных данных для приватного перестала быть актуальной. Тесты тестят то, что не может произойти. Будет еще хуже, если внезапно появятся новые входные данные, которые раньше считались невозможными. Теперь тесты не тестят все что должны.
Именно потому и тестят только публичные методы. У вас есть контракт, который будет использоваться в системе и вы тестите то, что можно туда подать.
dimpon Автор
Добрый день! mmMike давайте по-порядку.
Насчет тестирования private методов есть разные мнения, и я как раз оправдываю ситуации когда их тестируют. Но ряд авторов считает, что так делать не надо www.infoq.com/news/2008/01/private-methods-tdd-design
Я считаю, что выставлять наружу детали реализации (package-private методы) это плохо. Это ухудшает readability. И эти методы рано или поздно начнут дергать не только из тестов. К сожалению, в моем текущем проекте (10-летний монолит без четкой архитектуры, разрабатывается 3-мя командами, нет code ownership) это стандартная практика.
reflection в одном случае это нормально. в сто одном — уже не очень. а еще каждый делает это по-своему. А представьте, что надо засетить 10 private полей через reflection и потом протестировать один public метод?
а если потом прочитать их нужно? а еще NoSuchFieldException и IllegalAccessException ловить.
Идея статьи — не писать фреймворк, а использовать библиотеку testprivate и описывать доступ к private полям/методам через интерфейс. И делать это одинаково во всем проекте.
Вы делаете интерфейс прям в тесте, добавляее методы совпадающие по сигнатуре с private методами и getters/setters для private полей и как будто кастите свой объект к этому интерфейсу.
mmMike
Полностью согласен. Когда я вижу в коде private у метода, я точно знаю что он только для этого класса.
Да. специально:
(*) — Может я консервативен, но, например, предпочитаю вставить в код парсинг PEM файла руками или пасинг строки subj x509 (и то и другое — несколько строк), чем тянуть bouncy castle для этого. Если, кончено, ничего более не требуется от bouncy castle.
А вот это уже не очень хорошая практика. Ладно частные случаи с тестами на приватные методы. Но превращать это в общую практику… как то коряво.
Собственно противоречие в статье между «не хорошо писать тесты на приватные методы» и «вот вам библиотека для тестирования приватных методов» побудило написать комментарий.
sergey-gornostaev
Кент Бек, например.
Кому как не создателю TDD знать, что стоит тестировать, а что нет.
Maccimo
Сложную функцию парсинга логичнее было бы вынести в отдельный класс так как не делать этого нет ни единой причины.