Pull to refresh

Comments 27

А зачем тестировать приватные методы? Это же детали реализации. Есть два контракта — публичный (для пользователя) и внутренний (для наследника). Их и надо тестировать.
Справедливое замечание, но, предположим, что у класса есть один публичный метод, решающий конкретную задачу. Решает он её путем последовательного вызова 5 приватных методов передавая результат прошлого в последующий — т.е. вам становится очень тяжело пытаться через этот интерфейс протестировать сразу все 5 методов на граничных ситуациях — выносить их не имеет смысла т.к. они, допустим, узкоспециализированы и нужны только для этой задачи — делать публичными тоже не нужно (а ради теста — глупо. InternalsVisibleToAttribute и protected — в ту же кучу). На мой взгляд — было бы неплохо иметь в языке более тесную интеграцию с юнит тестированием, например, специальный модификатор доступа.
А вот если бы тесты были написаны ДО кода, тогда после рефакторинга приватные методы уже были бы автоматически. Бесплатно.
TDD предназначен для проектирования интерфейсов, а не написания кода.

Давайте рассмотрим простой пример с погодой: float getCurrentTemperature() должен сходить в интернет, скачать страницу, распарсить ее, и вернуть температуру на улице. Вооружившись книжкой по TDD мы пишем тест: assertEqual(25, getCurrentTemperature()); Но до тех пор, пока мы не завершим всю работу, не напишем все вспомогательные функции, этот тест будет красный и приносить особой пользы не будет.

Наверное джедаи тестирования могут и так работать, но я — нет. В таких случаях я пишу тесты и вспомогательные методы парами:

assertEquals("Moscow", getCurrentLocation());
assertEquals("www.weather-service.com/?city=Moscow", generateUrl("Moscow"));
assertTrue(downloadPage("www.weather-service.com/?city=Moscow"));
assertEquals(25, parseHtml());

и только потом на getCurrentTemprature();

Когда работа завершена, методы downloadPage/parseHtml стали приватными. Выкидывать тесты для них?

Можно возразить, что это не обязанность класса погоды качать данные и парсить HTML, но это будет всего лишь придирка к конкретному примеру.
Нет никаких вспомогательных функций. Код пишется после теста, весь написанный код — пишется под влиянием тестов. Все приватные методы получаются в результате рефакторинга. Таким образом они уже автоматически протестированы.
Да получаются. Но код крайне редко остается статичным: то исправить баг, то добавить фичу — и «автоматически протестированный» код превращается в «непокрытый». Выделели метод из оттестированной функции — кто будет гарантировать его работоспособность в будущем?
Баг — это ещё один тест, который код в данный момент не проходит, но проходить будет.

Добавить фичу — это написать тест, который код должен пройти.

Выделили метод — сломали тесты. Починили тесты, и гарантии вернулись.
> Баг — это ещё один тест

Баг — это, в первую очередь, грусть заказчика, гемор руководителя и, следовательно, минус к зарплате. От написания и починки теста предыдущие пункты не ревертятся.
Вообще спрашивали про покрытие а не про эмоции участвующих в процессе людей. Если есть баг, то мы должны подумать о том, почему тесты его пропустили и написать недостающий тест, проверить что он валится, исправить код, проверить что тест проходит.
Предположу, что все эти методы должны быть реализованы в разных классах. Тогда и GetCurrentLocation и другие, не имеющие непосредственного отношения к температуре можно и нужно тестировать отдельно. А метод получения текущей температуры в текущем месте сводится к вызову нескольких уже хорошо и отдельно протестированных методов. И GetLocation — обязательно выделяется в интерфейс, который потом можно при желании и зафейкать. (Fake framework).
Выносить законченные куски, которые можно тестировать, в отдельные классы — самый лучший вариант.

Но если кто-то упорно не хочет выносить код, который «больше нигде не понадобится», то можно так поступить: весь внешний контракт класса выделить в интерфейс. Тестируемые функции сделать public, но не включать в интерфейс. Клиентам, работающим с классом только через интерфейс, вспомогательные функции будут не видны, а тест будет работать с классом напрямую.
>>>Можно возразить, что это не обязанность класса погоды качать данные и парсить HTML, но это будет всего лишь придирка к конкретному примеру.

Дык необходимость писать тесты для приватных методов это подсказка, что нарушен SRP — single responsibility principle.

для assertEqual(25, getCurrentTemperature()) простейшая реализация return 25;
тогда вы получите зеленый тест, но не все требования удовлетворены: реализация должна зависеть от состояния температурного сайта.

Тогда придется задуматься о том, как объекту подавать на вход фейковые сайты и что такое сайт (например, можно подумать, что у разных сайтов могут быть разные форматы и схемы url и инкапсулировать генерацию url и разбор формата туда).

>>>Когда работа завершена, методы downloadPage/parseHtml стали приватными. Выкидывать тесты для них?

Red->Green->Refactor — на этапе refactor рефакторятся не только продуктивный код, но и тесты. Как только вам захотелось тестировать что-то приватное отдельно, воспринимайте это как сигнал о том, что у класса слишком много обязанностей и переносите их в отдельный класс. И тестовые методы, если они стали приватными либо выкидываем (если то же самое продублировано тестами публичных методов) либо переносятся в тест этого отдельного класса.

Скорее всего мы имеем дело с «фасадом» — выносим все методы в отдельный интерфейс (сервис) и тогда в нашем исхожном фасадном методе остаётся только простейшая логика и прямые вызовы сервисных методов. Обычно фасадный метод получается настолько простым, что можно позволить себе его не тестировать (если только вы не гонитесь за процентом покрытия), а направить усилия на тестирование отдельных сервисных методов.

Этот же подход позволит вам разработать контракт передачи данных (чтобы не было соблазна хранить состояние в сервисе), что значительно увеличит прозрачность работы метода и лёгкость его тестирования (все данные приходят только через параметры).
UFO just landed and posted this here
Скорей всего тестирование всего и всея приведет к «Тестам ради тестов». И любой шаг в сторону приведет к куче красных тестов.
Классическая ситуация, когда фанатизм приводит к тому, что идея вырождается в собственную противоположность.
Тесты помогают делать правильный дизайн, выделять интерфейсы, поэтому для тестирования мы сделаем приватные методы доступными извне, т.е. испоганим интерфейс вынесением наружу того, чего не надо.
Ну скажем так не сами тесты, а подход по их написанию (тесты в начале), а так очень хорошо сказали :)
UFO just landed and posted this here
И получаю грязный, непонятный код, с кечей проверок в рантайме?
Конечно слышал, но вот пихать логику тестов в исполняемый код, такое для меня дикость.
Я вообще кибрмерлину писал :)
А он не скалал, что пихает код тестов в продакшн код, а что использует ассерты. Я предположил, что у него там не тесткейзы, а просто разные предусловия и др.
Оу, извиняюсь) Ну вообщем мне кажется, имелось в виду как раз тестирование, судя по заголовку поста.
Я придерживаюсь мнения, что тестировать надо публичные методы, но жизнь такая штука, что иногда хочется/надо тестировать и приватные методы тоже.
Тестировать непубличные методы не нужно. Совсем.
Если тесты есть на все публичные кейсы, то непубличные методы будут ими покрыты.
Если логика публичного метода сложна и нужны тесты для непубличных методов — значит нарушен принцип единственной ответственности и часть непубличных методов должны стать публичными… в интерфейсах и других классах.
мне всегда казалось что юниттестируют логику кода, вне зависимости от того под какой видимостью он находится
Sign up to leave a comment.

Articles