Когда вредно тестировать ваши компоненты

    image

    Автоматизированные тесты – это хорошо. Проект, который на 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);
    

    Теперь нам не нужно специальных инструментов и рендеринга компонент. Меняйте компоненты сколько угодно, тесты останутся актуальными и помогут вам при каждом коммите.

    Если сильно хочется, то можно тестировать и сам компонент. Но что это дает? Вопрос открытый.

    Напрашивается вывод: Сложность написания тестов – признак плохого дизайна, повод для того, чтобы еще раз подумать над архитектурой кода. Вместо того, чтобы изворачиваться и писать неподдерживаемые тесты с рендерингом в псевдобраузере, проще навести порядок и писать простые понятные юнит тесты, которые дополнительно документируют ваше приложение.
    Wrike 80,54
    Wrike делает совместную работу над проектами проще
    Поделиться публикацией

    Вакансии компании Wrike

    Комментарии 37
    • +1
      тесты как минимум в данном примере не несут вреда, больше работы, да, но в идеале тестировать надо абсолютно все действия приложения, если она чтото рендерит то иногда и это нужно потестить, если же кусок для рендеринга используется ещё гдето не не тестить уже нельзя, тесты бывают плохими когда нужно кусок функционала перенести и сделав это надо переписать ещё и сотню тестов, но это значит лишь то что надо тесты правильнее писать
      • +7
        Тесты, которые нужно менять после каждого изменения в коде, бесполезны и даже вредны, поскольку требуют затрат на сопровождение и не приносят никакой пользы.
        Тесты должны падать при изменении системы. Иначе это бесполезные тесты, которые ничего не проверяют.
        • +3
          Абсолютно с вами согласен, но если после каждого изменения системы падают все тесты, эффективность этой системы снижается почти до нуля.
          • +8
            Так работают тесты — чем выше надежность, тем выше избыточность системы. Нахождение золотой середины между ними ближе к искусству, чем к науке.
          • +8
            Тесты должны падать при изменении системы. Иначе это бесполезные тесты, которые ничего не проверяют.

            А если точнее, то тысты должны падать при невалидных изменениях системы. В идеале, если мы меняем реализацию компонента без изменения внешнего интерфейса и новая реализация работает, ниодного теста упасть не должно.
            • –2
              Что вы понимаете под «невалидным изменением»? Возьмём пример: виджет, который показывает аватар пользователя. Он обращается к некому API, получает оттуда ссылку и отображает его.

              Изменение первое: в код API, который возвращает ссылку, случайно добавили очень долго работающий код — например, Thread.Sleep(100000). Реализация поменялась, а интерфейс — нет. Регресс функционала налицо, но если в тесте те указан timeout, то он не упадет.

              Изменение второе: API возвращает теперь не одну ссылку, а несколько в разном разрешении, и виджет на клиенте показывает наиболее подходящую. Система осталась в согласованном виде, потому что изменения есть и в API, и в клиентском виджете, но промежуточные тесты должны упасть.
              • 0
                Что вы понимаете под «невалидным изменением»?
                Очевидно то поведение системы, которые не предполагалось и не проектировалось и при обнаружении которого будет заведена задача на его исправление.
                • +1
                  Так ведь тесты и фиксируют предполагаемое поведение системы в определенный момент времени. Когда вы вносите изменения в систему, старые тесты первое время должны падать. Если они не падают, значит покрытие низкое и весь ваш новый функционал поместился в «люфт» старой спецификации.
                  • 0
                    Все верно, но если тесты падают по каждому чиху, то это – плохо.
                • +1
                  Если кто-то написал Thread.Sleep или просто код долго работает или вообще API возвращает что-то другое, не то, что мы ожидали, то это совсем другая история.
                  Как тут могут помочь тесты с псевдорендерингом? Думаю что примерно никак. В нашем конкретном случае, если вместо листа строчек пришел бы объект или что-то другое, то обычный юнит тест упал бы точно также, как и тест при помощи рендеринга.
                  Весь смысл статьи в том, что логика рендеринга смешана с логикой для работы с данными, из-за этого нельзя нормально тестировать и нормально сопровождать тесты. И что если так происходит, то время задуматься.
                  • 0
                    Окей, я взял довольно далекий от 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>
                            )
                        }    
                    }
                    

                    Такие досадные ошибки тоже частенько случаются, особенно в ходе рефакторинга. Логику работы с данными можно вынести и наверняка тестировать ее отдельно действительно будет удобно, но полностью от написания интеграционных тестов не спасет.
                    • +1
                      Вопрос: а зачем здесь эта логика вообще нужна с фильтрацией? Про это ведь и разговор. Можно её вынести и тестировать станет полегче и использовать снова. И код будет целостным.
                      • 0
                        Прочитайте пример внимательно. Ошибка в том, что мы отфильтровали данные, а дальше случайно отобразили оригинальную коллекцию вместо отфильтрованной. В комментарии ниже i_user говорит примерно о том же, но сформулировал лучше меня.
                        • 0
                          Я понял, что в этом месте ошибка. Но это не объясняет зачем здесь эта логика нужна и почему бы её не вынести и тестировать отдельно.
                          • 0
                            В этом и суть примера: даже если вы вынесете всю логику наружу и протестируете ее отдельно, в компоненте всегда останется место, где можно накосячить.
                          • +1
                            class FaultyList extends React.Component {
                                render() {
                                    return (
                                        <ul>
                                            {filter(this.state.items).map(x => <li>{x}</li>)}
                                        </ul>
                                    )
                                }    
                            }
                            


                            Я думаю что здесь гораздо сложнее ошибиться. И шаблон как-то почище.
              • +2
                Статья скрорее не про когда вредно тестировать, а как вредно тестировать. Да, мы выделили логику фильтрации, протестировали ее, но само отображение тоже надо тестировать. На моем опыте видел кучу ситуаций, когда переименовывали поле где-нибудь в скрипте, но забывали про шаблон, в итоге кусок компонента не отображается.
                • +1
                  Я думаю что здесь прекрасно работает правило 20/80 (20% тестов покроют 80% вашего кода). Бывают ситуации, когда нужно тестировать все целиком, возможно даже эти кейсы очень важны. Но из моего опыта видно, что зачастую тестирование всего подряд приводит только к дополнительным затратам.
                • +3
                  Дело в том, что сам компонент написан плохо. Код для фильтрации нельзя использовать повторно. Если нам понадобится фильтрация в другом компоненте, то при таком подходе придется писать и тестировать все заново.Нарушен принцип единственности ответственности.

                  Помимо логики для вывода в компоненте есть еще и логика для фильтрации. Решение есть, и оно очень простое. Можно перенести код, который отвечает за фильтрацию в другое место и тестировать там.


                  1. Если нам понадобится применять ровно эту фильтрацию в другом месте… А если не понадобится?

                  2. Если мы протестировали код для фильтрации в другом месте — по-хорошему, при тестировании компонента мы должны протестировать, что он вызывает именно эту функцию фильтрации, а не какую-то другую. Так как это внутренняя и неявная зависимость модуля, фактически, для формальной чистоты — мы должны прогнать тесты, которые покроют эту зависимость.
                  • 0
                    1. Если не понадобится — значит не понадобилось. Писать код чисто никому и некогда еще не вредило.
                    2. Вот в том то и дело, что для формальной чистоты. А насколько и когда это реально нужно?
                    • +1

                      У вас два пункта противоречат друг другу.
                      В первом вы говорите, что чистота кода полезна, а во втором — что формальность. В чем же разница?

                      • +1
                        Разница в том, что чистота тестов (полное покрытие) не гарантирует работу без ошибок и расходует очень много денег. А чистота кода (код который удобно поддерживать) гарантирует удобство работы. На разных уровнях: CVS, изменения в бизнес логике и прочее.
                        • +1

                          Как тест React-компонента в 4 строчки будет расходовать больше денег?


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

                  • +3

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


                    Но в то же время, если вынести логику в сервисные модули для легкого тестирования, остальное будет непротестировано. В вашем примере компонента тест проверяет не только вызов .filter(), но и весь компонент в целом. Разработчики могут ошибаться и в коде метода render и в constructor. И тест все это покроет и будет проходить, только если компонент в порядке.


                    Тем более, что приведенный тест несложный и полностью себя оправдывает.

                    • 0
                      У меня некоторые тесты просто импортируют NodeJS-модуль и проверяют что они импортировали функцию.
                      И ничего больше.
                      Потому что работа модуля завязана на внешние источники и я пока не научился его тестировать юнит-тестами.
                      Эти тесты не бесполезны — они как минимум проверяют что модуль в принципе существует и нормально импортируется.
                      Тесты тестам рознь.
                      • +1

                        Не очень хороший пример. Например, возьмем такой модуль


                        "use strict";
                        module.exports = function() {
                            console.log(test);
                        }

                        Модуль импортируется успешно, а ошибка в нем все равно есть. Статический анализ, например с eslint здесь принесет намного больше пользы.

                        • 0
                          Есть конечно. Я обратного и не утверждал.
                    • +2

                      И еще порекомендую хороший доклад с FrontTalks по этой теме
                      https://youtu.be/ni9Zz1j8fI4?list=PLKaafC45L_SRke8G1qiE0ZTJovI0FYKRw

                      • 0

                        Имхо, при написании тестов на UI нужно почаще задавать себе вопрос "что именно мы хотим проверить". Для этого компонета я бы назвал самым важным тестом "он показывает все элементы, если поле фильтра пустое". Ключевое слово — "показывает".

                        • 0
                          А почему такой выбор? Чем он обусловлен?
                          • +2

                            Прежде всего компонент должен "работать" — т.е. быть пригодным для использования пользователем, пусть и с багами. Если он падает с ошибкой или не позволяет показать все данные, то на выходе получается принципиально не работающее приложение.

                            • +1
                              https://habrahabr.ru/company/wrike/blog/315908/#comment_9935566
                              • 0

                                Ага, но если бы всё было так просто. Тестировать надо а) то, что сломается и б) то, что, сломавшись, нанесет наибольший ущерб. Обычный баланс качество/трудозатраты, и, чтобы его соблюсти, нужно уметь предсказывать будущее. Или иметь очень большой опыт.

                        • +1
                          Тесты к vast-player и vastacular, по-моему прекрасные иллюстрации к статье.
                          • +1
                            > Конечно, мы тестируем еще и то, как ReactJS рендерит изменения и как навешены события на DOM, как они обрабатываются и прочее. Но именно эта строка здесь явно главная.
                            Вовсе не явно. В современном мире View слишком часто бывает важнее, чем Model, чтобы однозначно судить. Именно поэтому фильтрация тут связана с компонентом и не должна быть отдельно от него.
                            Почему? Все просто, отображение может быть связано непосредственно с этой моделью, и при ее, даже малейшейм, изменении компонент может быть отрисован по другому. Т.е. нам важен не результат этой функции, а именно то, что она принимает и как работает внутри именно этого компонента.
                            • +1
                              Конечно продукт целиком важнее чем часть продукта. С этим тяжело спорить. Но зачем тогда мы используем моки? Можно сразу писать полностью интеграционные тесты, сразу с сохранением в базу и с отправкой писем скажем, чтобы данные не терялись и письма приходили.
                              Есть куча примеров, когда проверка вместе с отображением просто необходима. Особенно если все запихать в один метод.
                              Вот здесь и появляется осознанность. Ведь если вынести код для сортировки, протестировать его отдельно, то необходимость тестирования всего вместе, снижается в разы. Уменьшается сложность и тесты становятся не такими хрупкими.
                              • 0
                                Я комментировал высказывание статьи про очевидность именно в данном примере «главности строки», а не про то, что любая композиция — вредная вещь.

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

                            Самое читаемое