Автоматизированные тесты – это хорошо. Проект, который на 100% покрыт тестами, преподносит покрытие как преимущество. Но…
Думаю, что в этом процессе нет осознанности. А она сильно облегчает жизнь. Возможно, что половина тестов в вашем проекте не только бесполезна, более того — несет вред. Ниже расскажу о том, какие тесты писать не нужно.
Рассмотрим компонент на ReactJS:
class FilterForm extends React.Component {
constructor(props) {
super(props);
this.state = {
value: '',
frameworks: ['react', 'angular', 'ember', 'backbone']
};
this.handleChange = this.handleChange.bind(this);
}
handleChange(event) {
this.setState({value: event.target.value});
}
render() {
const filteredElements = this.state.frameworks
.filter(e => e.includes(this.state.value))
.map(e => <li>{ e }</li>)
return (
<div>
<input type="text" value={this.state.value} onChange={this.handleChange} />
<ul>{ filteredElements }</ul>
</div>
);
}
}
Работающий пример
Тесты для этого возможно выглядят как-то так:
test('should work', () => {
const wrapper = shallow(<FilterForm />);
expect(wrapper.find('li').length).to.equal(4);
wrapper.find('input').simulate('change', {target: {value: 'react'}});
expect(wrapper.find('li').length).to.equal(1);
});
Давайте подумаем, что этот код тестирует. Это важно. А тестирует он по большому счету эту строку:
.filter(e => e.includes(this.state.value))
Конечно, мы тестируем еще и то, как ReactJS рендерит изменения и как навешены события на DOM, как они обрабатываются и прочее. Но именно эта строка здесь явно главная.
А так ли нам нужно тестировать весь компонент? При этом подходе к написанию кода мы не можем иначе. У нас нет выбора.
В конкретном примере это не вызывает проблем, но почти всегда компоненты с течением времени становятся сильно сложнее. Логика и DOM компоненты перемещаются из одного компонента в другой.
Например, следующим шагом имеет смысл разбить один компонент на два: форма ввода и лист. Каждый элемент листа тоже имеет смысл сделать отдельным компонентом. После таких изменений придется менять тесты. Вернее не менять, а выкидывать и писать заново. Они станут бесполезными, так как завязаны на DOM. Возникает вопрос: «А зачем такие тесты нужны!?»
Тесты, которые нужно менять после каждого изменения в коде, бесполезны и даже вредны, поскольку требуют затрат на сопровождение и не приносят никакой пользы.
Конечно, можно использовать некую абстракцию, типа PageObject (eng), но проблема не будет решена полностью.
Дело в том, что сам компонент написан плохо. Код для фильтрации нельзя использовать повторно. Если нам понадобится фильтрация в другом компоненте, то при таком подходе придется писать и тестировать все заново.Нарушен принцип единственности ответственности.
Помимо логики для вывода в компоненте есть еще и логика для фильтрации. Решение есть, и оно очень простое. Можно перенести код, который отвечает за фильтрацию в другое место и тестировать там.
export function filter(list, value){
return list.filter(e => e.includes(value))
}
expect(filter(list, ‘react’).length).to.equal(1);
Теперь нам не нужно специальных инструментов и рендеринга компонент. Меняйте компоненты сколько угодно, тесты останутся актуальными и помогут вам при каждом коммите.
Если сильно хочется, то можно тестировать и сам компонент. Но что это дает? Вопрос открытый.
Напрашивается вывод: Сложность написания тестов – признак плохого дизайна, повод для того, чтобы еще раз подумать над архитектурой кода. Вместо того, чтобы изворачиваться и писать неподдерживаемые тесты с рендерингом в псевдобраузере, проще навести порядок и писать простые понятные юнит тесты, которые дополнительно документируют ваше приложение.
Комментарии (37)
impwx
28.11.2016 14:00+7Тесты, которые нужно менять после каждого изменения в коде, бесполезны и даже вредны, поскольку требуют затрат на сопровождение и не приносят никакой пользы.
Тесты должны падать при изменении системы. Иначе это бесполезные тесты, которые ничего не проверяют.
zolotyh
28.11.2016 14:06+3Абсолютно с вами согласен, но если после каждого изменения системы падают все тесты, эффективность этой системы снижается почти до нуля.
impwx
28.11.2016 14:12+8Так работают тесты — чем выше надежность, тем выше избыточность системы. Нахождение золотой середины между ними ближе к искусству, чем к науке.
iivvaall
28.11.2016 15:39+8Тесты должны падать при изменении системы. Иначе это бесполезные тесты, которые ничего не проверяют.
А если точнее, то тысты должны падать при невалидных изменениях системы. В идеале, если мы меняем реализацию компонента без изменения внешнего интерфейса и новая реализация работает, ниодного теста упасть не должно.impwx
28.11.2016 16:15-2Что вы понимаете под «невалидным изменением»? Возьмём пример: виджет, который показывает аватар пользователя. Он обращается к некому API, получает оттуда ссылку и отображает его.
Изменение первое: в код API, который возвращает ссылку, случайно добавили очень долго работающий код — например,Thread.Sleep(100000). Реализация поменялась, а интерфейс — нет. Регресс функционала налицо, но если в тесте те указан timeout, то он не упадет.
Изменение второе: API возвращает теперь не одну ссылку, а несколько в разном разрешении, и виджет на клиенте показывает наиболее подходящую. Система осталась в согласованном виде, потому что изменения есть и в API, и в клиентском виджете, но промежуточные тесты должны упасть.
Ohar
28.11.2016 18:33Что вы понимаете под «невалидным изменением»?
Очевидно то поведение системы, которые не предполагалось и не проектировалось и при обнаружении которого будет заведена задача на его исправление.impwx
28.11.2016 18:58+1Так ведь тесты и фиксируют предполагаемое поведение системы в определенный момент времени. Когда вы вносите изменения в систему, старые тесты первое время должны падать. Если они не падают, значит покрытие низкое и весь ваш новый функционал поместился в «люфт» старой спецификации.
zolotyh
28.11.2016 19:30+1Если кто-то написал Thread.Sleep или просто код долго работает или вообще API возвращает что-то другое, не то, что мы ожидали, то это совсем другая история.
Как тут могут помочь тесты с псевдорендерингом? Думаю что примерно никак. В нашем конкретном случае, если вместо листа строчек пришел бы объект или что-то другое, то обычный юнит тест упал бы точно также, как и тест при помощи рендеринга.
Весь смысл статьи в том, что логика рендеринга смешана с логикой для работы с данными, из-за этого нельзя нормально тестировать и нормально сопровождать тесты. И что если так происходит, то время задуматься.impwx
28.11.2016 20:35Окей, я взял довольно далекий от React пример, давайте выберем поближе:
class FaultyList extends React.Component { render() { let items = this.state.items; let filtered = items.filter(x => x.someProperty == 1); return ( <ul> {items.map(x => <li>{x}</li>)} </ul> ) } }
Такие досадные ошибки тоже частенько случаются, особенно в ходе рефакторинга. Логику работы с данными можно вынести и наверняка тестировать ее отдельно действительно будет удобно, но полностью от написания интеграционных тестов не спасет.zolotyh
28.11.2016 21:41+1Вопрос: а зачем здесь эта логика вообще нужна с фильтрацией? Про это ведь и разговор. Можно её вынести и тестировать станет полегче и использовать снова. И код будет целостным.
impwx
28.11.2016 21:50Прочитайте пример внимательно. Ошибка в том, что мы отфильтровали данные, а дальше случайно отобразили оригинальную коллекцию вместо отфильтрованной. В комментарии ниже i_user говорит примерно о том же, но сформулировал лучше меня.
zolotyh
28.11.2016 21:58Я понял, что в этом месте ошибка. Но это не объясняет зачем здесь эта логика нужна и почему бы её не вынести и тестировать отдельно.
impwx
28.11.2016 22:27В этом и суть примера: даже если вы вынесете всю логику наружу и протестируете ее отдельно, в компоненте всегда останется место, где можно накосячить.
zolotyh
28.11.2016 22:22+1class FaultyList extends React.Component { render() { return ( <ul> {filter(this.state.items).map(x => <li>{x}</li>)} </ul> ) } }
Я думаю что здесь гораздо сложнее ошибиться. И шаблон как-то почище.
Tramway
28.11.2016 14:07+2Статья скрорее не про когда вредно тестировать, а как вредно тестировать. Да, мы выделили логику фильтрации, протестировали ее, но само отображение тоже надо тестировать. На моем опыте видел кучу ситуаций, когда переименовывали поле где-нибудь в скрипте, но забывали про шаблон, в итоге кусок компонента не отображается.
zolotyh
28.11.2016 16:07+1Я думаю что здесь прекрасно работает правило 20/80 (20% тестов покроют 80% вашего кода). Бывают ситуации, когда нужно тестировать все целиком, возможно даже эти кейсы очень важны. Но из моего опыта видно, что зачастую тестирование всего подряд приводит только к дополнительным затратам.
i_user
28.11.2016 14:30+3Дело в том, что сам компонент написан плохо. Код для фильтрации нельзя использовать повторно. Если нам понадобится фильтрация в другом компоненте, то при таком подходе придется писать и тестировать все заново.Нарушен принцип единственности ответственности.
Помимо логики для вывода в компоненте есть еще и логика для фильтрации. Решение есть, и оно очень простое. Можно перенести код, который отвечает за фильтрацию в другое место и тестировать там.
1. Если нам понадобится применять ровно эту фильтрацию в другом месте… А если не понадобится?
2. Если мы протестировали код для фильтрации в другом месте — по-хорошему, при тестировании компонента мы должны протестировать, что он вызывает именно эту функцию фильтрации, а не какую-то другую. Так как это внутренняя и неявная зависимость модуля, фактически, для формальной чистоты — мы должны прогнать тесты, которые покроют эту зависимость.zolotyh
28.11.2016 22:091. Если не понадобится — значит не понадобилось. Писать код чисто никому и некогда еще не вредило.
2. Вот в том то и дело, что для формальной чистоты. А насколько и когда это реально нужно?justboris
29.11.2016 00:13+1У вас два пункта противоречат друг другу.
В первом вы говорите, что чистота кода полезна, а во втором — что формальность. В чем же разница?zolotyh
29.11.2016 08:10+1Разница в том, что чистота тестов (полное покрытие) не гарантирует работу без ошибок и расходует очень много денег. А чистота кода (код который удобно поддерживать) гарантирует удобство работы. На разных уровнях: CVS, изменения в бизнес логике и прочее.
justboris
29.11.2016 14:36+1Как тест React-компонента в 4 строчки будет расходовать больше денег?
Из моего опыта, намного больше времени разработчиков уходит на поиски кода, когда одна функциональность размазана по нескольким модулям ради абстрактной архитектуры ради архитектуры.
justboris
28.11.2016 14:30+3С одной стороны, вы правы. Если тесты будет писать сложно, то их будет написано мало, и стабильность продукта будет под угрозой.
Но в то же время, если вынести логику в сервисные модули для легкого тестирования, остальное будет непротестировано. В вашем примере компонента тест проверяет не только вызов
.filter(), но и весь компонент в целом. Разработчики могут ошибаться и в коде метода render и в constructor. И тест все это покроет и будет проходить, только если компонент в порядке.
Тем более, что приведенный тест несложный и полностью себя оправдывает.
Ohar
28.11.2016 18:38У меня некоторые тесты просто импортируют NodeJS-модуль и проверяют что они импортировали функцию.
И ничего больше.
Потому что работа модуля завязана на внешние источники и я пока не научился его тестировать юнит-тестами.
Эти тесты не бесполезны — они как минимум проверяют что модуль в принципе существует и нормально импортируется.
Тесты тестам рознь.justboris
28.11.2016 18:43+1Не очень хороший пример. Например, возьмем такой модуль
"use strict"; module.exports = function() { console.log(test); }
Модуль импортируется успешно, а ошибка в нем все равно есть. Статический анализ, например с eslint здесь принесет намного больше пользы.
justboris
28.11.2016 14:31+2И еще порекомендую хороший доклад с FrontTalks по этой теме
https://youtu.be/ni9Zz1j8fI4?list=PLKaafC45L_SRke8G1qiE0ZTJovI0FYKRw
Scf
28.11.2016 16:43Имхо, при написании тестов на UI нужно почаще задавать себе вопрос "что именно мы хотим проверить". Для этого компонета я бы назвал самым важным тестом "он показывает все элементы, если поле фильтра пустое". Ключевое слово — "показывает".
zolotyh
28.11.2016 19:21А почему такой выбор? Чем он обусловлен?
Scf
28.11.2016 19:58+2Прежде всего компонент должен "работать" — т.е. быть пригодным для использования пользователем, пусть и с багами. Если он падает с ошибкой или не позволяет показать все данные, то на выходе получается принципиально не работающее приложение.
zolotyh
29.11.2016 08:07+1https://habrahabr.ru/company/wrike/blog/315908/#comment_9935566
Scf
29.11.2016 12:23Ага, но если бы всё было так просто. Тестировать надо а) то, что сломается и б) то, что, сломавшись, нанесет наибольший ущерб. Обычный баланс качество/трудозатраты, и, чтобы его соблюсти, нужно уметь предсказывать будущее. Или иметь очень большой опыт.
Opaspap
28.11.2016 18:27+1Тесты к vast-player и vastacular, по-моему прекрасные иллюстрации к статье.
arvitaly
29.11.2016 01:26+1> Конечно, мы тестируем еще и то, как ReactJS рендерит изменения и как навешены события на DOM, как они обрабатываются и прочее. Но именно эта строка здесь явно главная.
Вовсе не явно. В современном мире View слишком часто бывает важнее, чем Model, чтобы однозначно судить. Именно поэтому фильтрация тут связана с компонентом и не должна быть отдельно от него.
Почему? Все просто, отображение может быть связано непосредственно с этой моделью, и при ее, даже малейшейм, изменении компонент может быть отрисован по другому. Т.е. нам важен не результат этой функции, а именно то, что она принимает и как работает внутри именно этого компонента.zolotyh
29.11.2016 08:06+1Конечно продукт целиком важнее чем часть продукта. С этим тяжело спорить. Но зачем тогда мы используем моки? Можно сразу писать полностью интеграционные тесты, сразу с сохранением в базу и с отправкой писем скажем, чтобы данные не терялись и письма приходили.
Есть куча примеров, когда проверка вместе с отображением просто необходима. Особенно если все запихать в один метод.
Вот здесь и появляется осознанность. Ведь если вынести код для сортировки, протестировать его отдельно, то необходимость тестирования всего вместе, снижается в разы. Уменьшается сложность и тесты становятся не такими хрупкими.arvitaly
30.11.2016 06:26Я комментировал высказывание статьи про очевидность именно в данном примере «главности строки», а не про то, что любая композиция — вредная вещь.
jetcar
тесты как минимум в данном примере не несут вреда, больше работы, да, но в идеале тестировать надо абсолютно все действия приложения, если она чтото рендерит то иногда и это нужно потестить, если же кусок для рендеринга используется ещё гдето не не тестить уже нельзя, тесты бывают плохими когда нужно кусок функционала перенести и сделав это надо переписать ещё и сотню тестов, но это значит лишь то что надо тесты правильнее писать