Comments 23
- Не нужны отдельные TODO листы в разрезе маленьких проектов.
- Быстро вспоминается что хотел делать.
- Становится привычкой зайдя в проект машинально запускать тест, чтобы «вспомнить»
- Дополнительная мотивация все таки начать с теста, даже если проект не приоритетный.
Вот, допустим, мне надо протестировать метод, сохраняющий некую сущность в базе данных, которая задействует несколько таблиц (свойства сущности, её связи с разными категориями, метаданные) — можно ли тут что-то сделать «по уму», а не тупым функциональным тестом, который пишет в базу, читает из нее и сравнивает отправленное с полученным?
Если у вас нет слоя абстракции между менеджером этой сущности и БД, то кроме функционального тут ничего особо не придумаешь. По хорошему, должен быть слой (DBL, UnitOfWork,.etc), который берет на себя общение с БД. Он может быть изолированно покрыт функциональными тестами, либо же юнитами на предмет корректного генерирования SQL в разных условиях. Вы же для своего менеджера, который сохраняет сущность, проверяете только то, что он корректно с корректными аргументами дергает методы этого слоя для работы с БД.
А по теме функциональных тестов: я ничего плохого в них не вижу, когда речь идет действительно о стыке разных систем. Единственная проблема с ними — скорость выполнения. По этому там где возможно и понятно как писать юниты — пишем юниты, там где напрашивается функциональный — соответсвенно.
Только убедитесь, что разные тесты выполняются изолированно друг от друга.
Статья про идеальный мир, где проекты в богатых команиях пишутся с самого начала по TDD и с архитектурой угадали.
В реальности часто бывает так: бюджет ограничен, заказчику надо вчера, написали на коленке проект, тестов нет, на архитектуру много где забили, лишь бы быстрее выпустить проект на рынок и проверить выстрелит ли (думали сдохнет, шансов было мало). Проект взял и разросся, увеличился штат, увеличилось требуемое количетво фич. Чтобы безопасно менять код такого проекта, надо его покрыть тестами, хоть какими. Вот тут и появляется тестирование сингтонов, гигантские фикстуры, вызовы тестируемых методов через рефлексию. По мне лучше когда есть хотя бы функциональные тесты, или хотя бы приемочные, пусть и некрасивые, так спокойнее, чем вообще без них.
PS: Если дело касается веб-приложений на PHP я за функциональные тесты, я думаю у них максимальный профит (повышение надежности проекта) при минимальном времени написания. Под них архитектуру чаще не надо менять, в отличие от unitов
Я как раз взял кодсепшион и долго мучался над подключением ядра в тесте. Хоть что-то можно тестировать, но уровень, конечно, не Ларавеля. Больше всего печалит отсутствие возможности подключения тестовой БД.
Ну и то, что тестить в основном можно только аякс запросы и свои кастомные классы. Компонентный подход Битрикса делает очень сложным проверку логики где-нибудь в result_modifier
Никогда не тестируйте защищённые/приватные методыЕсть приложения в которых наследование является точкой расширения. Поэтому protected методы являются частью публичного API, который само собой нужно тестировать.
Никогда не тестируйте защищённые/приватные методы
Тоже не согласен с данным утверждением. Получается, чтобы протестировать приватный метод с парой ветвлений мне надо прогнать через публичный все эти ветвления. Ок, а если метод используется в 5 публичных функциях, на основе какой мне тестировать? Как другой разработчик узнает, что данный метод тестируется именной в этой функции, а не напишет свои тесты покрыв еще раз тот же самый код (вскроется только в отчете покрытия тестами). Поэтому я забил на юниты, пишу функциональные — проверяется еще работа со связкой в бд.
Юнит тесты остальных только на расчетных и очень критичных функциях, чтобы выявить ошибку быстрее, а не ждать пока прогонятся функциональные.
Ок, а если метод используется в 5 публичных функциях, на основе какой мне тестировать?Как раз приватный метод тестировать обычно не нужно совсем, потому что он относится к деталям реализации. Если все public/protected методы покрыты тестами, то можно быть уверенным, что этот класс (юнит) работает в соответстветствии со спецификацией/контрактом.
Хотя бывают исключения. Иногда надо убедиться, что класс делал или не делал вызовы к внешним API, например через системые вызовы. Если это трудно замокать, то придётся тестировать именно реализацию.
Пример.
final class RecordStorage {
/** Cache backend */
private $cache;
public function load($id): Record
{
if ($record = $this->cache->get($id)) {
return $record;
}
$record = $this->doLoad($id);
$this->cache->set($id, $record);
return $record;
}
private function doLoad(): Record
{
// Call some external API.
$record = ...;
return $record;
}
}
По хорошему:
$this->cache->get($id)нужно протестировать отдельным тестом.$this->cache->set($id, $record);тоже нужно протестировать отдельным тестом.doLoad()должен быть публичным, с вынесением в отдельный класс — тоже должен быть протестирован на граничные случаи.
Тогда RecordStorage:load() тестировать нет необходимости. RecordStorage превратиться сервис, который получает данные от протестированных компонентов и делегирует их в другие протестированные компоненты.
Так что для пункта 5. Никогда не тестируйте защищённые/приватные методы исключений не бывает ;)
Свойство
$cache$record$this->cache->get($id) нужно протестировать отдельным тестом.$cache это зависимость, которая само собой должна иметь свой отдельный тест. В данном случае задача состоит только в том, чтобы протестировать логику кеширования в RecordStorage и интеграцию с cache сервисом.
$this->cache->set($id, $record); тоже нужно протестировать отдельным тестом.
doLoad() должен быть публичным, с вынесением в отдельный классЗачем? doLoad используется только внутри этого класса. Внешним потребителям не нужно знать про него. В общем случае его может вообще не быть.
public function load($id): Record
{
if ($record = $this->cache->get($id)) {
return $record;
}
$record = $this->db->select('...') ; // Some slow SQL query to the database.
$this->cache->set($id, $record);
return $record;
}
Тут дело в том, что классу RecordStorage позволительно очень многое. По сути он создавался как сервис получения данных из хранилища.
В качестве хранилища у вас выступает doLoad() или $this->db->select('...') или т.д., по хорошему этот код должен быть в отдельном классе.
Сейчас за хранилище отвечает Сервис API получения данных из хранилища. Это и есть причина того, что есть потребность тестировать RecordStorage:load(). А если переписать код как указано ниже, то и тестировать нечего.
final class RecordStorage {
/** Cache backend */
private $cache;
/** Cache backend */
private $storage;
public function load($id): Record
{
if ($record = $this->cache->get($id)) { // протестировано отдельным тестом
return $record;
}
$record = $this->storage->dataById($id); // протестировано отдельным тестом
$this->cache->set($id, $record); // протестировано отдельным тестом
return $record;
}
}Пункт "Никогда не тестируйте защищённые/приватные методы", да и вообще рекомендации из статьи в целом хороши тем, что заставляют думать и писать "чистый код".
// протестировано отдельным тестомТестирования зависимостей по отдельности, ничего не гарантирует. В данном случае нужен интеграционный тест, чтобы убедится что RecordStorage умеет корректно с этими зависимостями.
В качестве хранилища у вас выступает doLoad() или $this->db->select('...') или т.д., по хорошему этот код должен быть в отдельном классе.doLoad() это просто внутренняя обёртка над хранилищем. То что вы убрали, её никак не поможет проверить в тесте, то что данные при повторном запросе берутся из кеша, а не из БД.
Пункт «Никогда не тестируйте защищённые/приватные методы», да и вообще рекомендации из статьи в целом хороши тем, что заставляют думать и писать «чистый код».Удаление приватных методов, не сделает автоматически код «чистым».
Тема тестирования приватных методов это частный случай более широкой темы тестирования деталей реализации. В общем случае реализация может находится и в публичном методе (как в вашем примере). Нужно ли её тестировать? Зависит от цели тестрования. Если ваша цель просто убедиться в том, что все публичные интерфейсы работают как надо, то тестировать детали реализации нет смысла. Но большое заблуждение при этом думать, то что корректность работы публичного интерфейса сервиса гарантирует отсутствие багов в нём. Сервис может отдавать корректные результаты внешним потребителям, но при этом делать не нужные запросы к внешним API, создавать/удалять сущности в базе, засорять системный лог и много других не запланированных вещей, которые тест публичного интерфейса не обнаружит.
Я полностью согласен — RecordStorage нужно покрыть интеграционным тестом, чтобы гарантировать его работу, как ожидается. Просто с тем кодом, где использовался private, вам нужна магия PHP, чтобы добраться до private, нам требуется знание черного ящика, тела функции, знания о её реализации...
Никогда не тестируйте защищённые/приватные методы
Основная причина: они влияют на то, как мы тестируем функции, определяя сигнатуру поведения: при таком-то условии, когда я ввожу А, то ожидаю получить Б. Приватные/защищённые методы не являются частью сигнатур функций.
А с кодом, который без private, мы проверяем результат RecordStorage::load($id) — подсовывая в constructor нужные нам состояния $cache и $storage, проверяем возвращаемый Record — никакой магии и тест будет читаться просто:
Дано RecordStorage и когда $cache и $storage такие-то, тогда RecordStorage::load($id) возвращает такой-то Record.
При private тестируется: будет ли второй раз при запросе к load отрабатывать кэш. А в третий, а в четвёртый — уверены?
Без private тестируется:
- будет ли возвращён по id ожидаемый Record, который в cache, если он там имеется
- будет ли возвращён по id ожидаемый Record, который в storage если в cache его нет.
Просто с тем кодом, где использовался private, вам нужна магия PHP, чтобы добраться до private, нам требуется знание черного ящика, тела функции, знания о её реализации...В данном конкретном случае магии не требуется. Можно мокая, кеш и задать что то вроде
$cache->expects($this->never()). По поводу знания реализации. Конечно для тестирования реализации нужно знать реализацию. Тут возникает интересный вопрос. Должна ли инкапсуляция распостранятся на тесты? Я думаю для этого нет причин.А с кодом, который без private, мы проверяем результат RecordStorage::load($id) — подсовывая в constructor нужные нам состояния $cache и $storage, проверяем возвращаемый Record — никакой магии и тест будет читаться простоprivate это просто способ организовать внутреннее устройство класса. Оно может быть любым, как вам захочется. Отказываться от private методов только из-за того чтобы упростить тестирование, как то не правильно, имхо.
Это всё были детали конкретного сильно упрощённого примера. Что если внешних зависимостей вообще нет? Что если класс сам прозводит данные, например вычисляет какой нибудь сложный хеш для переданной строки и кеширует это в приватной переменной?
[удалено] не туда..
Чистые тесты на PHP и PHPUnit