воскресенье, декабря 07, 2008

Опять про тестирование приватных функций.

На RSDN-е опять обсуждают "Как тестировать приватные функции класса?", предлагают разные решения и спорят какое из них лучше.
Подобные споры меня всегда очень удручают. Почему? Потому что проблемы на самом деле нет. А попытки решать проблему которой нет порождают чудовищно уродливые решения.

На вопрос "Как тестировать приватные функции класса?" есть только один ответ: - "Только через публичный контракт этого класса." Если в голову лезут еще какие либо "решения", значит смысл модульного тестирования, а тем более TDD, до вас еще не дошел.

Что есть модульный тест? Это пример того, как внешний код будет использовать ваш класс. Зачем может понадобиться модульный тест для приватных методов класса, неужели вы рассчитываете на то, что внешний код будет вызывать приватные методы вашего класса? Так почему не сделать эти методы публичными?
Вопрос о тестировании приватных членов - это вопрос новичков. Вот у меня есть куча приватных методов и мне надо покрыть их тестами, как же мне это сделать? Это делается всегда одним и только одним способом - тестированием публичного контракта класса. Если у вас есть приватный метод, который вы не можете протестировать через публичный контракт, значит у вас что то не так с дизайном класса.

Если вы пользуетесь методикой TDD то вопрос о тестировании приватных членов у вас никогда не возникнет. Сначала вы пишете тест, в котором отрабатываете какой либо из сценариев использования вашего класса (которого еще нет). В результате, после написания теста, вы имеете публичный контракт (или часть контракта) своего будущего класса, который складывается из методов, использованных в тесте. Далее, вы начинаете писать реализацию класса, и совершенно ясно, что все мясо в виде приватных членов, которое вы теперь накодите, уже тестируется через вызовы публичных членов.

Я вот недавно писал библиотеку, в которой доля публичных методов менее 20%, остальные private и protected. Модульные тесты покрывают 95% кода библиотеки, причем тестируются только публичные члены. Я легко достиг бы и 100% покрытия, дописав guard-тесты, поскольку непокрытый код, это по большей части, валидация входных параметров и строки, выкидывающие исключения, но мне просто лень. В этой библиотеке есть полностью приватные классы, которых не видно снаружи, и они покрыты тестами на 100%.

Скажу более, в паре приватных методов я нашел большие куски по 6 -10 строк кода, не покрытые тестами. При внимательном рассмотрении, оказалось что этот код просто лишний :). Он никогда не вызывался ни при каких тестовых сценариях, и я его просто выкинул. Это, кстати, хорошая иллюстрация того, что понимается под последней буквой D (design) в TDD.

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

Небольшое дополнение, по прошествии одного дня.
Очень редко бывают случаи когда, действительно надо тестировать приватные члены. Предположим, у нас есть класс хранящий множество объектов и выполняющий с ними какие-то действия. Внутри этого класса, объекты хранятся в массиве, или в линейном списке. И вот по каким либо причинам, например для достижения большей скорости работы, нам необходимо реорганизовать внутренний механизм хранения объектов, вместо списка нам нужно сбалансированное двоичное дерево. Заметьте, что при этом публичный контракт класса не меняется. Но нам надо проверить, что внутренний механизм хранения в классе работает соответствующим образом. Тут без тестирования приватных членов, вероятно, не обойтись. Повторю, что такие случаи возникают крайне редко.

19 комментариев:

Павел комментирует...

По хорошему говоря смысла в приватных методах нет вообще. Т.е. если есть приватный метод, который делает что-то осмысленное, то его нужно сделать доступным хотя-бы в потомках. А если метод ничего внятного не делает, то нужно рефакторить и рефакторить. И голову лечить.
Навеяно многолетней борьбой. Реально утомляет, когда какой-то дятел прячет класс или метод, а тебе он нужен и без него никак.

Sergey Rozovik комментирует...

> смысла в приватных методах нет вообще

Я бы не был столь категоричен. Достаточно вспомнить хотя-бы такой паттерн рефакторинга, как "выделение метода". Обычно нет никакого смысла делать выделенный метод публичным. Зачастую такие методы "ничего внятного не делают" и рефакторить их больше нукуда :)

Илья Казначеев комментирует...

Я считаю, что если код не является реюзабельным в данный момент, то есть не используется вне домашнего проекта, не имеет смысла делать методы protected, кроме тех, которые реально переопределяются в потомках.

Особенно в том случае, когда этот метод - просто вынесенная часть тракта другого метода, в целях достижения "коротких методов".

Просто protected - это тоже заявка, контракт некий - "внимание, этот метод имеет дополнительное значение, которое не отражено в его коде". Надо сразу смотреть дерево наследования.
А если он private - смело меняешь при надобности, ни о чем лишнем не думая.

Meowth комментирует...

Что верно, то верно - тема всплывает регулярно и везде :) В сознании спорщиков средство ("юнит-тесты") замещает саму цель ("тестирование вариантов использования класса"), для которой эти средства, вообще-то говоря изобретались. Вот и выходит, что тесты пишутся ради тестов, отсюда и непонимание.
Хотя тестирование предназначено тестировать (извините, тавтология) контракты (ибо в первую очередь - tdD(sic!)), а не код. Как эти контракты реализованы - это отдельный вопрос, соответвенно, privates тут не при чем.

Aikin комментирует...

>>По хорошему говоря смысла в приватных
По-хорошему говоря, смысл в приватных методах огромен -- это увеличение читабельности кода.

Анонимный комментирует...

Тема всплывает по одной простой причине. Приватные методы которые хочется тестировать, как правило находятся "внизу" иерархии вызовов. Поэтому начиная писать юнит тесты, их хочется потестировать в первую очередь. При том, что с эволюцией тестирования данного класса, эти "юнит тесты низкого уровня" скорее всего уйдут.

denis miller комментирует...

Aikin, Анонимный Анонимный

вы отметили в точку. Сергей не вник в однимаемый вопрос и понакатанной написал свою реакцию.

Действительно добавляемая специфика может быть скрыта внтури приватов, а контроль этого должен быть однозначно. Это поведение может развиваться, а может лежать до восстребования. А поди пойми, что будет в жизни. А тест уже контролирует процесс.

Кстати, Павел, рефакторинг проводится над смеллами. Рефакторинг ради рефакторинга никогда не проводится или рефакторинг над приватными методами намерения.

Aikin комментирует...

Еще раз по поводу приватных методов. 10 мин назад я выделил в отдельный метод всего лишь 1 строчку:
private bool isNew(User user)
{
return user.Id != 0;
}

Aikin комментирует...

И только после поста сюда заметил, что ошибся с названием (либо с проверкой условия) :))

Sergey Rozovik комментирует...

> И только после поста сюда заметил, что ошибся с названием (либо с проверкой условия) :))

Во! Хоть какая-то практическая польза от этого блога появилась :)

Sergey Rozovik комментирует...

to denis miller
> Действительно добавляемая специфика может быть скрыта внтури приватов, а контроль этого должен быть однозначно.

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

> Это поведение может развиваться, а может лежать до восстребования.

Ага, "сделаем это на будущее". Денис, будет очень полезно, если вы, как евангелист, возьмете в толк сами и будете втолковывать всем своим прозерлитам одну простую истину - никогда не следует писать код "на будущее".

Sergey Rozovik комментирует...

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

Meowth комментирует...

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

Мое мнение, что такое достаточно сложное поведение лучше выносить в отдельную сущность, для которой заданный функционал и будет публичным контрактом => при этом смысл юнит-тестов в тестировании именно контракта не меняется.

kmmbvnr комментирует...

Присоединяюсь к Meowth.

Возникновение потребности протестировать приватные методы, неплохой индикатор нарушения Single Responsibility Principle.

Restuta комментирует...

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

Можно это реальзовать так как предложил Meowth, а можно написать тест, который проверяет скорость выполнения, т.к. выполнение операции быстрее чем X - это вполне функциональное требование.

COTOHA комментирует...

2 Restuta

омг. не позорь меня...

COTOHA комментирует...

Кстати, дополнение - имхо - лишнее.

если измение внутренней реализации не поломало юнит-тесты публичного контракта, то или всё ОК (и совершенно по боку что внутри - массив или дерево) или юнит-тесты что-то не покрывают.

Sergey Rozovik комментирует...

> Кстати, дополнение - имхо - лишнее.
Я тоже так по-началу думал. Тем не менее кейс приведен реальный, и единственная альтернатива, как тут уже говорили, вынос внутренней логики дерева в отдельный класс с публичным контрактом. Но не всегда это приемлимо, особенно в библиотеках.

COTOHA комментирует...

хм. так я не понял - что именно надо протестировать?

что данные внутри хранятся правильно? а зачем, если поведение класса осталось корректным?