Комментарии 34
А я всегда верила в Visual Studio :)
Вот именно поэтому статические вызовы и прочие синглетоны, — это зло.
Какой-то странный тест, честно говоря.
1. Три разных случая входных данных проверяются в рамках одного теста. Это противоречит базовым принципам построения юнит-тестов ("не более одного утверждения на тест"), т.к. мешает нормально проверять, что код работает правильно.
2. Не ясно, какие именно утверждения проверяются тестом. Будет ли он оставаться зелёным, если из метода PurchaseItem убрать код, который выбрасывает исключение? Что-то мне подсказывает, что в таком случае тест по-прежнему будет проходить. А значит, в нынешнем виде он некорректен.
В JUnit для проверки на выброс исключения используется параметр expected аннотации @Test. Наверняка в NUnit (или что это за фреймворк?) есть какие-то аналоги. Их и надо использовать.
3. Метод ProcessOrder() имеет сайд-эффекты? Обращается ли он к другим классам? Если да, то их тоже надо закрывать в тесте моками. Если же Order сохраняется внутри класса, то в тесте надо проверить, что содержимое объекта изменилось после вызова PurchaseItem.
В целом за стремление использовать юнит-тесты и за желание рассказать об этом остальным можно лишь похвалить, но я не хотел бы, чтобы новички учились их писать по подобным образцам.
1. Три разных случая входных данных проверяются в рамках одного теста. Это противоречит базовым принципам построения юнит-тестов ("не более одного утверждения на тест"), т.к. мешает нормально проверять, что код работает правильно.
2. Не ясно, какие именно утверждения проверяются тестом. Будет ли он оставаться зелёным, если из метода PurchaseItem убрать код, который выбрасывает исключение? Что-то мне подсказывает, что в таком случае тест по-прежнему будет проходить. А значит, в нынешнем виде он некорректен.
В JUnit для проверки на выброс исключения используется параметр expected аннотации @Test. Наверняка в NUnit (или что это за фреймворк?) есть какие-то аналоги. Их и надо использовать.
3. Метод ProcessOrder() имеет сайд-эффекты? Обращается ли он к другим классам? Если да, то их тоже надо закрывать в тесте моками. Если же Order сохраняется внутри класса, то в тесте надо проверить, что содержимое объекта изменилось после вызова PurchaseItem.
В целом за стремление использовать юнит-тесты и за желание рассказать об этом остальным можно лишь похвалить, но я не хотел бы, чтобы новички учились их писать по подобным образцам.
3. Это MSTest, судя по всему.
Присоединяюсь. А где хоть один ассерт? Где название, по которому понятно, что именно свалилось, если тест красный (хотя КАК он может стать красным)? И где вообще тот самый «юнит» — мы тестируем условие времени работы, или правильность идентификатора ордера, или логику покупки?
Ну и совсем о наболевшем:
>> Запустив этот юнит тест мы опять получим значение Code Coverage 100% вне зависимости от того, какое на самом деле текущее время.
Ну не говорит этот ваш coverage ничего о качестве тестирования. Использовать как индикатор — да, и то не как сиюмитный показатель, а в динамике; маленькое покрытие — да, подозрительно, надо смотреть почему. Но нельзя его как самоцель ставить, иначе получаем тесты пустых конструкторов, автопропертей и просто то самое тестирование бранчей без понимания, что именно надо бы протестрировать на самом деле.
Ну и совсем о наболевшем:
>> Запустив этот юнит тест мы опять получим значение Code Coverage 100% вне зависимости от того, какое на самом деле текущее время.
Ну не говорит этот ваш coverage ничего о качестве тестирования. Использовать как индикатор — да, и то не как сиюмитный показатель, а в динамике; маленькое покрытие — да, подозрительно, надо смотреть почему. Но нельзя его как самоцель ставить, иначе получаем тесты пустых конструкторов, автопропертей и просто то самое тестирование бранчей без понимания, что именно надо бы протестрировать на самом деле.
Боюсь что я совершил ошибку, убрав все ассерты и прочие штуки, а так же не сделал три кейса для каждого случая. Сделано это было с единственной целью повысить — читабельность примера.
Еще раз повторюсь, цель статьи показать как можно залезть внутрь поведения казалось бы совсем неуправляемых вещей что можно попасть внутрь условия if (Date.Now....).
Если code coverage ничего не говорит о качестве тестирования, какую бы вы метрику предложили?
Еще раз повторюсь, цель статьи показать как можно залезть внутрь поведения казалось бы совсем неуправляемых вещей что можно попасть внутрь условия if (Date.Now....).
Если code coverage ничего не говорит о качестве тестирования, какую бы вы метрику предложили?
ошибку никогда не поздно исправить!
представьте, что ваш пост прочитает новичок, который еще ничего не знает о unit-тестах. или знает, но очень мало. чему его научит такой пример? он только запутается.
представьте, что ваш пост прочитает новичок, который еще ничего не знает о unit-тестах. или знает, но очень мало. чему его научит такой пример? он только запутается.
Поправил.
Уже лучше! Ещё бы тестовые методы назвать так, чтобы было лучше видно, что именно они проверяют. Например:
Выражение намерения теста в его названии — это мощный приём. Он позволяет лучше сфокусироваться на задаче при написании теста. А тот, кто будет читать код этого теста, будет лучше понимать, какие данные в тесте более важны, а какие менее. Всё это позволяет сфокусировать внимание на существенных в данном контексте вещах, что улучшает читабельность и понятность кода.
PurchaseItemTest1 -> shouldNotProcessOrderWithInvalidId
PurchaseItemTest2 -> shouldProcessValidOrder
PurchaseItemTest3 -> shouldNotProcessOrderAtInvalidTime
Выражение намерения теста в его названии — это мощный приём. Он позволяет лучше сфокусироваться на задаче при написании теста. А тот, кто будет читать код этого теста, будет лучше понимать, какие данные в тесте более важны, а какие менее. Всё это позволяет сфокусировать внимание на существенных в данном контексте вещах, что улучшает читабельность и понятность кода.
Возможно, стоит подобрать иной пример, который, оставаясь простым, является в то же самое время честным юниттестом, с ассертами и удачным названием.
Про метрики — почти все они становятся весьма бесполезными, когда от команды начинают требовать исполнения каких-то цифр, потому что «в этой метрике это число всегда должно быть 37 или меньше». В этой статье у меня на протяжении всего чтения было ощущение, что автор гонится за мифическими ста процентами покрытия. Для меня любая метрика — это не более чем индикатор того, что и как происходит.
С точки зрения оценки качества тестирования одна из показательных метрик — это грубо говоря количество багов, найденных в протестированном коде «на следующем цикле», т.е. для программистов и их юнит тестов — сколько багов нашли тестеры, для тестеров — сколько багов нашли пользователи в продакшене и т.д.
Простой вычислялки, которая автоматическим образом скажет, что да как, тупо нету. На одну строчку с регэскпом часто надо написать десятки тестов, а на десятки строк кода которые в принципе просто создают кучу объектов и передают их в метод нет смысла писать и одного теста.
Про метрики — почти все они становятся весьма бесполезными, когда от команды начинают требовать исполнения каких-то цифр, потому что «в этой метрике это число всегда должно быть 37 или меньше». В этой статье у меня на протяжении всего чтения было ощущение, что автор гонится за мифическими ста процентами покрытия. Для меня любая метрика — это не более чем индикатор того, что и как происходит.
С точки зрения оценки качества тестирования одна из показательных метрик — это грубо говоря количество багов, найденных в протестированном коде «на следующем цикле», т.е. для программистов и их юнит тестов — сколько багов нашли тестеры, для тестеров — сколько багов нашли пользователи в продакшене и т.д.
Простой вычислялки, которая автоматическим образом скажет, что да как, тупо нету. На одну строчку с регэскпом часто надо написать десятки тестов, а на десятки строк кода которые в принципе просто создают кучу объектов и передают их в метод нет смысла писать и одного теста.
Представьте что там не Exceptions. Просто другие вызовы чего нибудь.
А где в первом случае Assert на брошенное исключение?
И, кстати, что за идея бросать Exception, а не ArgumentException с комментарием?
И, кстати, что за идея бросать Exception, а не ArgumentException с комментарием?
Сразу вспоминается:
public Calendar getTomorrow() {
Thread.sleep(1000*60*60*24);
return Calendar.getInstance();
}
Извините :)
public Calendar getTomorrow() {
Thread.sleep(1000*60*60*24);
return Calendar.getInstance();
}
Извините :)
Красиво, но «You may use, copy, reproduce, and distribute this Software for any non-commercial purpose,
subject to the restrictions in this MSR-LA.».
Пока не будет релиза дальше игрушек это исспользовать нельзя.
subject to the restrictions in this MSR-LA.».
Пока не будет релиза дальше игрушек это исспользовать нельзя.
А в каких сторонних фреймворках реализованы подобные возможности стаббинга? Isolator?
Мне почему-то кажется, что было уже довольно много статей по Moles: habrahabr.ru/search/?q=moles
К примеру для начинающего есть habrahabr.ru/blogs/net/98571/ а там внизу ссылки даже с видео.
Или вот более расширенная, но вполне доступная для начинаюшего статья: habrahabr.ru/blogs/net/101408/
А если по субжу, то после некоторого практического опыта использования Moles, я пришел к выводу, что это отличный костыль для legacy-кода.
Намного чаще если есть что-то «нетестируемое», значит есть проблема в архитектуре.
К примеру для начинающего есть habrahabr.ru/blogs/net/98571/ а там внизу ссылки даже с видео.
Или вот более расширенная, но вполне доступная для начинаюшего статья: habrahabr.ru/blogs/net/101408/
А если по субжу, то после некоторого практического опыта использования Moles, я пришел к выводу, что это отличный костыль для legacy-кода.
Намного чаще если есть что-то «нетестируемое», значит есть проблема в архитектуре.
ну почему устаревшее, возьмём DirectoryServices.AccountManagement.
Мы сейчас написали обёртки с подменой реализации fake/real, а с Moles теоретически могло быть всё попроще.
Но учитывая что это ещё какаято 0.9* версия то в production такое не возьму, придётся ждать релиза, судя по последней активности ждать уже недолго.
Мы сейчас написали обёртки с подменой реализации fake/real, а с Moles теоретически могло быть всё попроще.
Но учитывая что это ещё какаято 0.9* версия то в production такое не возьму, придётся ждать релиза, судя по последней активности ждать уже недолго.
Надо ли создавать тесты для юнит тестов
Если вы не уверены в своем unit-тесте, если у вас появляется желание проверить правильность его работы — это плохой тест, его нужно переписать.
Unit-тесты должны быть максимально просты, чтобы для уверенности в их правильности хватало, грубо говоря, взгляда и запуска, то есть должна быть круговая порука — 1) тесты тестируют ваш код; 2) глядя на ваш код и тесты, вы понимаете, что тесты верны. тесты и код тестируют друг друга.
А почему вызов DateTime идет внутри функции, мне кажется в этом и есть загвоздка того что вы исполняете танец с бубнами чтобы написать Unit-тестирование :-)
Думаю вопрос решается если передавать извне DateTime :-)
Думаю вопрос решается если передавать извне DateTime :-)
Спасибо за пост, очень интересная тулза, где-то может оказаться полезной. Но мне кажется, увлекаться такими тулзами не стоит. Надо понимать, что писать тестируемый (и поддерживаемый, ликвидный) код они не помогают, а скорее наоборот. Вот этот мануал (State of the Art Testability) можно покурить на досуге (я это делаю периодически), объясняет основные принципы на конкретном примере
А можно поподробнее про «специальным образом подготовленную заглушку для сборки MSCorLib»?
Функция которая зависит от времени суток (погоды на марсе) — сама по себе должна насторожить. Вы правильно делаете, что насторожились, но переделываете/советуете неправильно.
А надо ИМХО вынести логику связанную с временем за рамки функции PurchaseItem(), потому что этот метод должен покупать (и зависисеть только от параметров), иначе вы должны были ее назвать PurchaseItemBetween6PMAnd9PM(), да и то при этом лучше было бы передавать время параметром PurchaseItemBetween6PMAnd9PM(time currentTime).
Я допускаю что это должно быть не так в API (но там свои методы, а ля контроллер), но в модели то всё должно быть четко и без магии (чтобы с магией не проверять).
А надо ИМХО вынести логику связанную с временем за рамки функции PurchaseItem(), потому что этот метод должен покупать (и зависисеть только от параметров), иначе вы должны были ее назвать PurchaseItemBetween6PMAnd9PM(), да и то при этом лучше было бы передавать время параметром PurchaseItemBetween6PMAnd9PM(time currentTime).
Я допускаю что это должно быть не так в API (но там свои методы, а ля контроллер), но в модели то всё должно быть четко и без магии (чтобы с магией не проверять).
Друзья, статья не про архитектуру и паттерны проектировоания. И я отметил в тексте что по одним из корректных методов обхода таких «острых» углов является использование Visitor.
Конечно, по хорошему тесты и должны направлять разработчика создавать код который не содержит таких решений. Но в жизни всякое бывает, и в каких то случаях Moles позволяет решить эти проблемы.
Конечно, по хорошему тесты и должны направлять разработчика создавать код который не содержит таких решений. Но в жизни всякое бывает, и в каких то случаях Moles позволяет решить эти проблемы.
Спустя время некоторые картинки в твоей статье пропали. Перезалей, пожалуйста.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Юнит-тесты: Как протестировать то, что не тестируется