Comments 29
Вроде как суть в подходе «мокиста» не просто проверить, что были совершены вызовы, но и проверить, что в эти вызовы было передано. Часто внутри используется несколько сервисов, одни что-то возвращают, чтобы передать результат другим сервисам. Именно это и надо проверить, если эти вызовы являются обязательными для гарантии правильной работы алгоритма. При разработке и рефакторинге можно забыть вызвать нужные методы, что не повлияет на работу алгоритма, например, запись в лог. Проблема «замены метода» в контексте юнит-тестов не является проблемой, так как если придётся переписывать тесты, это последнее что надо будет переписывать, и тесты надо будет переписывать отнюдь не из-за замены метода, а потому что поменялся тестируемый код. Использовать фейки только лишь в угоду более удобному рефакторингу, значит не понимать саму суть тестов, как я думаю.
Хочу уточнить правильно ли я Вас понял.
1) Вы утверждаете, что верифицируете сам факт вызова методов в купе со значениями передаваемых параметров.
2) Если кто-то создает классы, чтобы удобнее тестирвоать, то такой человек не понимает тестирование воообще.
Это то, что вы говорите?
Если в первом случае да, то мне интересно всегда ли Вы так пишете или только в некоторых случаях?
1) Я говорю о том, что алгоритм тестируется проверкой результата работы алгоритма. Верифицирование фактов вызова методов и переданных значений является проверкой реализации алгоритма в дополнение к проверке результата. Т.е. если в алгоритм были переданы определённые интерфейсы, значит они должны определённым образом использоваться, иначе их наличие становится лишним. Тесты проверяют, что это так.

2) Нет. Если человек пишет тесты так, как будто это код приложения, применяя различные принципы кодирования, в том числе DRY, то это не правильный подход. Тесты не должны быть абстрактными классами, чтобы максимально адаптироваться, совсем наоборот. Изменение и рефакторинг кода должно провоцировать пересмотр тестов, если они в этом случае ломаются — это хорошо. Подозрительно, если они не ломаются.

Я стараюсь максимально протестировать каждый аспект алгоритмов. Если мы подставляем интерфейс, значит мы должны его мокать. Стараюсь избегать фейк-объектов в разумных пределах, так как это усложняет тесты, делая их похожим на код. Надо смотреть код фейков, а они в свою очередь могут использовать другие фейки и т.д. Так быть не должно. Каждый отдельный тест должен быть по моему мнению изолирован от других тестов. Хотя соблазн «улучшить» код тестов велик. Код тестов должен быть простым и понятным, даже если он будет уродлив с точки зрения программного кода.
Понятно, и я полностью согласен. Даже если в терминах есть разница, я все равно не вижу сильной разницы в подходах. Тесты должны легко писаться, читаться, а создание новых сущностей помогать их читать, а не усложнять.
Я немного не уловил разницу в этих двух подходах. В обоих случаях создается некая заглушка. В первом случае заглушку генерит Moq, во втором вы пишите её руками. Если в коде что-то меняется так, что первый вариант нужно переписать, то с вероятностью близкой в 1, второй вариант тоже надо будет переписать. Так в чем же разница?
Разница действительно незначительная. Она в том, что тест не знает о внутренней реализации тестируемого метода. Это важно для легкости тестов и понятности когда. Далее вы проверяете fake объектом не конкретную реализацию, а следствие (эффект) вызванный работой тестируемого метода.

Лично я и тот и другой использую подход и не вижу ничего критичного в замене одного другим. Второй случай мне кадется более показательным для описания логики метода.
Необходимо протестировать работу метода Handle(). Вопрос стоит в том, чтобы убедиться, что методы DbCommands и MessagingLogger были вызваны.
[...]
[мокист] передал бы в конструктор класса моки соответствующих интерфейсов, а после проверил бы вызвались или нет соответствующие методы: SaveEvt(), Received() или InvalidEvent().
dbCommands.Verify(m => m.SaveEvt(It.IsAny<Event>(), Times.Once())
messagingLogger.Verify(m => m.Received(It.IsAny<EventPayload>(), Times.Once())
[...]
[немокист] создал бы fake objects и проверил бы совершилось ли событие в целом, а не вызов метода
[...]
А методы fake-objects выглядели бы так: public void SaveEvt(Event evt) => IsEventSaved = true;

Так вот.


Во-первых, в обоих случаях у вас используются стабы (а не моки или фейки). Это согласно следующему определению Фаулера:


Stubs provide canned answers to calls made during the test, usually not responding at all to anything outside what's programmed in for the test. Stubs may also record information about calls, such as an email gateway stub that remembers the messages it 'sent', or maybe only how many messages it 'sent'.

(по классификации Мезароса это Test Spy: "A Test Spy is an object that can act as an observation point for the indirect outputs of the SUT.")


А из того, что оно одинаково называется, можно догадаться, что в реальности это, на самом деле, один и тот же подход. И это, в общем-то, тоже вытекает из фразы Фаулера в той же статье


The classical TDD style [который Фаулер противопоставляет "мокистскому"] is to use real objects if possible and a double if it's awkward to use the real thing. So a classical TDDer would use a real warehouse and a double for the mail service.

В нашем случае оба объекта, на которых мы делаем верификацию — это те, которые "неудобно использовать", поэтому, согласно Фаулеру, представители обоих подходов будут здесь использовать test doubles (что у вас и произошло).


Реальная разница между двумя подходами в вашей статье состоит в том, что первый проверяет требования как они были сформулированы (напомню: "убедиться, что методы DbCommands и MessagingLogger были вызваны"), а второй проверяет некоторое допущение о нужном бизнес-требовании.


Если вы хотите, чтобы этот код был менее "мокистским" и более "классическим", вам надо переформулировать требования — например, так: "убедиться, что данные сохранены и сообщение отправлено", — затем переписать оба test double так, чтобы они максимально соответствовали оригиналу, но использовали легкодоступное хранилище (например, оба складывали полученное в очередь), а затем в качестве проверки анализировать это хранилище. Вот тогда — да, вы получите тестирование вносимого эффекта.


Потому что в чем основная ошибка (обоих) ваших тестов? Они не тестируют, что именно передано в метод — а это, очевидно, нужно для корректного теста. Когда вы начнете это делать, первый код изменится незначительно (вместо It.IsAny появится нормальный предикат), а вот второй придется менять радикально.

Test Doubles удобно рассматривать в разрезе ролей, которую они выполняют в тестах сайд-эффектов:
Stubs — предоставляют заданные значения для входов SUT (Indirect Inputs),
Mocks — дают возможность для верификации выходов SUT (Indirect Outputs),
Fake — реализуют штуки, которые в тесте не важны, но без которых SUT не работает (null-objects, dummy и т.п.).

В этом смысле оба варианта, предложенных в статье, предназначены для проверки Indirect Outputs и реализуют Mock. Первый использует программную генерацию (ad-hoc), второй написан лапками (hard coded).

Когда мне нужны детали, я предпочитаю классификацию Мезароса, в которой то, что описано в статье, называется test spy (и является частным случаем test stub), а мок — это то, что само проверяет indirect outputs.


Но в общем случае я вообще не вижу смысла в этом разделении, и предпочитаю использовать термин "мок" для любого test double вообще.

Есть смысл различать входы и выходы SUT: первое это то, что в тесте мы меняем, второе — то, что анализируем. Spy, как концепция, скорее про выходы.

Spy, как концепция, и есть про выходы. И… что? Я про это и написал изначально.

И… Звучит будто вы не согласны с тем, что мы в чем-то согласны. Если больше нет возражений, предлагаю эту ветку закончить. :)
Очень позновательно о терминах. Спасибо. Про требования именно так.
На счет этого дам пояснения:
Потому что в чем основная ошибка (обоих) ваших тестов? Они не тестируют, что именно передано в метод

На мой взгляд именно так должен выглядеть самый первый тест на данную логику. По мере накопления багов или в ходе предпродакшн тестирования появятся тесты более специфияные. Но для работы данной логики такие детали, как передача конкретного значения может быть упущена. Это сделает тесты легкими и легко пишущимися/поддерживающимися. Спустя 3 месяца там 100% будет и тест на параметры и на какие-то значения, но зачем сразу?
Спустя 3 месяца там 100% будет и тест на параметры и на какие-то значения, но зачем сразу?

Затем, что тест "что-то вообще передали" весьма бессмысленен. Обычно важно, что именно было передано (особенно если в хэндлере есть перепаковка/трансформация).


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

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

С идеалистической я бы сделал иначе.

Во-первых, стоит ответить на вопрос: зачем тестировать значения? Если это необходимо, то вероятно существует логика трансформации в тестируемом методе. Эту логику можно вытащить и тестировать отдельно. Т.о. используя данный компонет для трансформации (маппинга) вы будете уверены, что он выдает на выходе всегда правильный результат. Если нет, вы добавите еще один случай для проверки этого маппера.

Во-вторых, в случае использования компонента-маппера внутри тестируемого метода, вам уже не надо будет заботиться о конкретных значениях. Это следует из допущения, что маппер работает так, как надо, а логики трансформации в тестируемом методе нет.

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

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

Чтобы знать, что код делает то, что от него ожидается.


Эту логику можно вытащить и тестировать отдельно.

Вытащить куда? Зачем? Он у вас и так вытащен: Mapper.Map<Event>(payload);. Но вам же все равно надо протестировать что вы засовываете именно результат транформации, а не что-то другое.


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

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


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

Надо-надо. Смотрите выше.


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

Избыточно. У AutoMapper уже давно есть свой собственный интерфейс, который можно так же вбрасывать и мокать, и не нужны никакие врапперы.


И тогда я был бы уверен, что передав некое значение, я всегда в методах DbCommands получу ему соответствующее.

Нет, не будете. Смотрите выше.

Вы правы и я с вами полностью согласен. Так будет правильнее, хотя писать тесты будет сложнее. Я даже уверен, что они станут менее поддерживаемыми, т.к. придется больше прописывать различных нюансов, как значения передаваемых типов. Мой тест не покажет проблему, если вдруг между маппером и следующим возовом DbCommands параметр приравнять NULL.

В данном случае, я оставляю эту часть проверки ручному тестированию. Думаю, программист в здравом уме не будет просто так вставлять NULL и не проверять данную модификацию никак. А закрыть все абсолютно дыры программно все равно не получится. Поэтому тут вопрос о балансе затрачиваемых ресурсов и результата, который дают тесты. Чрезмерно сильно проверять код там, где это не надо нет необходимости. У нас есть голова, чтобы знать и помнить о том, как он работает. А тест должен продокументировать основной ход выполнения, за счет его проверки, и проверить различные краевые случаи.

Т.е. в целом я согласен, но против чрезмерно дотошного тестирования юнит-тестами.
Так будет правильнее, хотя писать тесты будет сложнее. Я даже уверен, что они станут менее поддерживаемыми, т.к. придется больше прописывать различных нюансов, как значения передаваемых типов

Нет и нет. Если использовать Moq, то там есть прекрасный метод с автовыводом типов (предположим, что commands — это то, что возвращает маппер):


dbCommands.Verify(m => m.SaveEvt(commands), Times.Once());
messagingLogger.Verify(m => m.Received(payload), Times.Once());

Кода стало меньше, чем с It.IsAny, и явные указания типов пропали.


Если же использовать самописный test double, то типы прописывать в любом случае (и в любом случае при нормальном рефакторинге они изменятся автоматически).


Мой тест не покажет проблему, если вдруг между маппером и следующим возовом DbCommands параметр приравнять NULL.

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


В данном случае, я оставляю эту часть проверки ручному тестированию.

У ручного тестирования есть (как минимум) одна крупная проблема: его нельзя запускать тогда, когда мне нужно, и столько, сколько мне нужно. Поэтому чем меньше в моей жизни ручного тестирования, тем эта жизнь спокойнее.


Думаю, программист в здравом уме не будет просто так вставлять NULL и не проверять данную модификацию никак.

Вы зря так думаете.


У нас есть голова, чтобы знать и помнить о том, как он [код] работает.

Спасибо, но нет. Я не хочу знать и помнить, как работает код во всей системе на сотни и тысячи KLOC.


Т.е. в целом я согласен, но против чрезмерно дотошного тестирования юнит-тестами.

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

Я восхищаюсь вашим уровнем погружения в данную область. Очень интересные и ценные комментарии.

Необходимо протестировать работу метода Handle(). Вопрос стоит в том, чтобы убедиться, что методы DbCommands и MessagingLogger были вызваны.

Требование к Indirect Outputs какое-то мутное. Если по аналогии сформулировать требование к выходу (Direct Output) самого метода Handle(), то получится что-то типа: «Необходимо протестировать работу метода Handle(). Вопрос стоит в том, чтобы убедиться, что метод возвращает результат.».

Тот факт, что мы просто должны дергать за ручки back door систем, не очень осмысленный, так же как мало смысла в проверке, что метод возвращает «неважно что». При верификации выходов SUT (Direct или Indirect) в первую очередь стоит проверять, что выполняются контракты по значениям на этих самых выходах. А для этого и требования должны формулироваться соответствующим образом.
Требование к Indirect Outputs какое-то мутное

Оно не мутное, оно, гм, заранее завязано на реализацию (т.е., если следовать определениям из самого поста — оно заранее "мокистское").


При верификации выходов SUT (Direct или Indirect) в первую очередь стоит проверять, что выполняются контракты по значениям на этих самых выходах.

Можете привести пример?

При тестировании SUT через API («front door») проверяются связи между значениями на прямых входах и значении на выходе (direct inputs и direct outputs). В данном случае при подаче на вход валидного значения Payload ожидается конкретный результат MessageHandlerResult.Success на выходе:
public void Should_save_valid_data()
{
    var result = handler.Handle(validPayload);
    Assert.Equal(MessageHandlerResult.Success, result);
}

Такого же уровня детализации стоит придерживаться и при формулировании требований в отношении Indirect Output при тестировании «back door»:
public void Should_log_to_messaging_events_on_saving_valid_data()
{
    var result = handler.Handle(validPayload);
    messagingLogger.Verify(m => m.Received(It.Is(validPayload)), Times.Once())
}
Такого же уровня детализации стоит придерживаться и при формулировании требований в отношении Indirect Output при тестировании «back door»:

Вот только никакого контракта тут нет. Вы проверяете, что вы запихнули в логгер то же, что передали в хэндлер.

Я так понимаю ответственность этого метода. А как бы вы описали контракт?

Ответственность метода — не то же самое, что его контракт.


(собственно, контракты не могут распространяеться на indirect inputs/outputs)

Если вы позволите контракту распространяться на indirect in/out, контракт станет зависимым от реализации (а в пределе — станет идентичным реализации). При этом смысл контракта как раз абстрагировать реализацию.


Контракт — это публичная информация о компоненте; например, "сохраняет событие в персистентное хранилище". То, что при этом компонент (а) логгирует это событие и (б) делает сохранение через три метода (добавить, сохранить, закоммитить транзакцию) — это детали реализации, которые потребителя не интересуют.


Я обычно представляю себе это так: если бы я вытащил из компонента интерфейс, то контракт — это тот набор условий, который я вправе ожидать от этого интерфейса вне зависимости от реализации.

Друзья, не надо ссориться. Пишите оба теста. Одни тегируйте как те, что проверяют публичный интерфейс, другие как те, что проверяют внутренний интерфейс.
Есть следующий код хендлера:
<…>
Необходимо протестировать работу метода
Надеюсь, я правильно понимаю, что буковки «TDD» в первую строку заметки попали случайно и отношения к ней не имеют?
Only those users with full accounts are able to leave comments. Log in, please.