Можно мокать тестируемый обьект?

#php #юнит_тесты #phpunit #любой_язык

Есть:


Класс который нужно протестировать

  class Message extends Data
  {
     protected $text = '';

     public function check()
     {
        return empty($this->text);
     }

     public function setText($text)
     {
        if ($this->check() === true) {
           $this->text = $text;
        } else {
           $this->text .= $text;
        }
     }
  }

Разработчики Вася и Петя, которые спорят как правильно написать
тест для setText, нужно мокать метод check или нет?


Вася: за mock check

При юнит тестировании нужно мокать все что возможно кроме спрятанных данных (private,
protected), что бы протестировать все сценарии которые не зависят друг от друга. Итого
будет 3 сценария, где мок check вернет либо true, false или иное, для метода setText.
И отдельно протестировать метод check. Также незабыть компонентный тест, который протестирует
обьект (Message) в общем, то есть интерфейс этого объекта, не мокая ничего что касаеться
Message и Data.

Петя: против mock check

Нельзя мокать методы тестируемого класса (Message) и его родителей (Data), можно
только внешние классы. При изменении метода check реально сломается setText но юнит
об этом не сообщит. Сами юниты будут показывать как работает класс, ненужно делать
компонентные тесты только на один объект, но сами тесты будут дублировать в себе проверку
одних и тех же внутренних зависимостей (метод check, будет 2 раза протестирован)

Вопрос:

Кто из них прав, или как правильно тестировать?

Примеченные:

Как обычно в моих вопросах, этот обучающий вопрос. Проблемы с конкретным кодом нету,
это просто пример (на примерах лучше понять)
    

Ответы

Ответ 1

За идею мокать тестируемый класс я бы сразу расстреливал. Тестируемый класс — это
единое целое, и должно тестироваться как единое целое. Пролезая своими грязными ручками
в метод check, вы предполагете конкретную реализацию метода setText. Это не юнит-тест,
это профанация: вы тестируете не внешний интерфейс класса, а проверяете, что операции
в нём написаны точно так же, как в тесте.

Где в интерфейсе класса говорится, что setText должен вызывать check, а не кэшировать
состояние к какой-то переменной? Теперь вы не можете изменить реализацию при сохранении
интерфейса и поведения, не сломав юнит-тесты. Так зачем же юнит-тесты, которые ломаются
от изменения внутренней реализации? Они только раздражать будут.

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



Я вообще сторонник идей Мартина Фаулера: мокать нужно только то, что слишком сложно
не мокать: базы данных, рассылку почты и т. п. Это позицию он называет "классическим TDD".

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

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

"Мокеры" любят плодить миниатюрные юнит-тесты на каждый геттер и сеттер, но забывают,
что классы существуют не в безвоздушном пространстве, а взаимодействуют. В результате
у каждого класса покрытие 99%, а вместе почему-то всё ломается. Нельзя забывать про
интеграционные тесты. И юнит-тесты, которые "слегка интеграционные" — это отличный путь.

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



Что касается конкретно вашего кода, то у вас метод выглядит как сеттер, но по сути
им не является. Это провал теста до написания каких-либо тестов.


Ответ 2

В данном конкретном случае я бы изменил интерфейс. Метод setText выглядит как обычный
сеттер, но в действительности сеттером не является.

Очень похоже, что его стоит разбить на методы clearText и appendText, и вынести логику,
спрятанную за check, выше.

Спор Васи с Петей косвенно свидетельствует о проблемах в проекте.
Позиция Пети сильнее (нельзя мокать методы тестируемого класса), поскольку она соответствует
правилам декомпозиции, описанным у книге дядюшки Боба Мартина «Чистый код».

Если кратко: на одном слое декомпозиции должны находиться методы одного логического
уровня, такой код гораздо проще читать. В Вашем примере всё выглядит так, что check
должен находиться где-то выше, где его и надо тестировать. И вот там можно мокать класс
Message, который находится на уровень ниже.

И здесь, на уровне Message, мы можем протестировать методы clearText и appendText,
возможно, мокая какие-то классы ниже.

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

P.P.S. Заметил, что речь идёт о моках. Я по возможности следую такому правилу: сначала
протестируй непосредственно, если не выходит, и нельзя перепроектировать, задействуй
стаб, если и так не выходит, задействуй мок. Таким образом простые валидирующие или
трансформирующие методы, которые работают с объектами стандартной библиотеки (строки,
даты, регулярные выражения) перетекают в классы-утилиты, где и тестируются.


Ответ 3

У вашего класса не полный API. Добавьте метод getText и тестируйте поведение комплексно.

Как вариант можно использовать следующие тесты:


На чистом объекте, убедиться, что check возвращает false.
На чистом объекте, убедиться, что getText возвращает ''.
После setText метод check возвращает true.
После setText метод getText возвращает правильную строку.
После повторного setText метод getText возвращает правильную строку.




Небольшое отступление:

Некоторые методы очень трудно тестировать в изоляции друг от друга и это нормально.
Типичный пример -- контейнер:

class Container {
    private $data;

    public function get(key) {/* ... */}

    public function set(key, value) {/* ... */}
}


Методы get и set просто нельзя протестировать отдельно друг от друга. В таких случаях
имеет смысл тестировать поведение объекта (либо группы методов).

Кроме того, правильнее подходить к методам как к маленьким "черным ящикам". Тесты
ни в коем случае не должны зависеть от реализации метода. В вашем примере, стоит вам
заменить вызов check на что-то еще и ваши тесты сломаются, хотя поведение метода setText
останется прежним.

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