Комментарии 41
Смущает нарушение коммутативности в DSL в неочевидных местах:
var order = Create.Order
.Dated(3.May(2019))
.With(Pizza.Pepperoni.CountOf(1).For(500))
.With(Pizza.Margarita.CountOf(1).For(650))
.Please();
Вернет не то же самое, что
var order = Create.Order
.With(Pizza.Pepperoni.CountOf(1).For(500))
.With(Pizza.Margarita.CountOf(1).For(650))
.Dated(3.May(2019))
.Please();
Или такое поведение было задумано изначально?
Новые тесты становится писать легко. Не нужно 2 часа настраивать себя на юнит-тестирование, просто берешь и пишешь.
Можно просто брать и писать, как будто билдеры уже есть, а потом пусть студия создаст недостающие методы.
Не совсем понятно нафиг все это надо, если есть AutoFixture.
Я считаю, что самых лучших результатов можно добиться, если использовать AutoFixture вместе с кастомным DSL, а не вместо него, но это за скоупом статьи.
Не совсем понимаю в чем соль доморощенного DSL, зачем время тратить на классы строители? Я бы вас понял если бы вы писали в эпоху Java6 на ней самой. Классы строители как шаблон изжили себя в среде .NET.
Я думаю, это скорее зависит от языка, чем от фреймворка.
В случае С# property и collection initializers устраняют некоторую потребность в строителях, но все равно есть потребность в бизнес-специфичных алгоритмах типа "создай заказ готовый для исполнения" когда можно убрать под капот какие-то детали создания, не важные для конкретного теста.
Например, тут я бы убрал дату внутрь билдера:
var order = Create.Order
.With(Pizza.Pepperoni.CountOf(1).For(500))
.With(Pizza.Margarita.CountOf(1).For(650))
.Please();
var service = Create.PizzeriaService.Please();
service.AcceptOrder(order);
service.VerifyReserveIngredientsWasCalledWith(order);
Но, внезапно выясняется, что property и collection initializers работают только при создании объекта.
Т.е. написать new Order{ Date = Today } можно, а CreateOrder(){Date = Today} нельзя.
Для записей есть proposal
CreateOrder() with {Date = Today}
Вы привели достаточно простой пример строителя, но в настоящей жизни нам пришлось бы иметь дело со строителем на 3 или 4 десятка строк больше. Иметь такое чудо в коде теста, на мой взгляд, не приемлемо. Тест это не только тест, но и живая документация. Так что смысла в доморощенных строителях не вижу, тем более что с AutoFixture все можно персонализировать и если это делать то в отдельном вспомогательном методе.
// Arrange
var order = CreateDualOrder();
var service = CreatePizzeriaService();
// Act
service.AcceptOrder(order);
// Assert
service.VerifyReserveIngredientsWasCalledWith(order);
В книжке Art of unit testing, предлагается скрывать детали не относящиеся к конкретному тесту и показывать связь между исходными данными и результатами. В получившемся результате скрыто почти все (из кода теста непонятно, что и сколько должно быть зарезервировано). Фактически и CreateDualOrder и VerifyReserveIngredientsWasCalledWith внутри используют какие-то константы но как они друг с другом связаны, неясно.
Я бы написал так:
var ingridient = CreateIngridient();
var product = CreateProduct().WithIngridient(ingridient, AmountOfIngridientInProduct).Please();
var order = CreateOrder().WithLine(product, AmountOfProductInOrderLine).Please();
var service = CreateService();
service.AcceptOrder(order);
Assert.Equals(AmountOfProductInOrderLine * AmountOfIngridientInProduct, service.GetReservedAmount(ingridient));
AutoFixture не пользовался, интересно, как там с воспроизводимостью падений. Например, в данном случае вполне возможно, что результат будет работать с точностью до округлений (каковые будет проверять другой тест).
По мне так все равно куча ненужного шума:
CreateProduct().WithIngridient(ingridient, AmountOfIngridientInProduct).Please();
Лучше
CreateProduct(ingridient: "olive", amount: 10);
В книжке Art of unit testing ...
Кстати в той же книге предлагается награждать описанием проверки типа:
Assert.Equals(AmountOfProductInOrderLine * AmountOfIngridientInProduct, service.GetReservedAmount(ingridient), "Ибо так потребно");
Не совсем уверен что service.GetReservedAmount(ingridient)
и AmountOfProductInOrderLine * AmountOfIngridientInProduct
уместны в секции Assert, по книге аrt of unit testing.
Лучше
Возможно — но тогда придется все сочетания поддерживать в одном методе.
Кстати в той же книге предлагается награждать описанием проверки типа
С моей точки зрения, когда в тесте ровно один ассерт и название достаточно говорящее, это не надо. Кстати.
Не совсем уверен что service.GetReservedAmount(ingridient) и AmountOfProductInOrderLine * AmountOfIngridientInProduct уместны в секции Assert, по книге аrt of unit testing.
Почему?
Возможно — но тогда придется все сочетания поддерживать в одном методе.
Ну это уж вам решать как структурировать код, можно и не в одном методе.
С моей точки зрения, когда в тесте ровно один ассерт и название достаточно говорящее, это не надо.
Здесь страдает не читаемость кода теста, а ошибки в ходе прогона тестов.
Почему?
Ну хотя бы потому что:
Секция 8.3.4 Separating asserts from actions
For the sake of readability, avoid writing the assert line and the method call in the same statement.
Ну это уж вам решать как структурировать код, можно и не в одном методе.
Я имел ввиду, что у вас метод уже создает что-то — соответственно сочетать два таких метода вместе не удастся. Метод не умеет модифицировать уже готовый ордер, например.
Здесь страдает не читаемость кода теста, а ошибки в ходе прогона тестов.
Я тоже про них.
Обычно названия метода-теста выводятся при ошибках, так, что если мы видим что тест проверяет зарезервированность И ошибка в том, что количество отличается от заданного можно сказать что проверяется зарезервированное количество.
Секция 8.3.4 Separating asserts from actions
Я думаю, имелось ввиду action — это то, что мы проверяем, а assert — то, как мы проверяем. В моем коде action это AcceptOrder. Я посмотрел несколько примеров из книжки, там используются выражения и вызовы методов в assert.
Я имел ввиду, что у вас метод уже создает что-то — соответственно сочетать два таких метода вместе не удастся. Метод не умеет модифицировать уже готовый ордер, например.
Да блин возможности по композиции в .NET почти безграничны, зачем вам метод createAll.
Обычно названия метода-теста выводятся при ошибках…
А ну да обрубок вроде Method_ShouldWorkAsExpected_WhenGetsRightValue
заменяет полноценную фразу на человеческом языке(xUnit не в счет)
Я думаю, имелось ввиду action
Да но ваш service
это объект тестирования, так что это сбивает с толку.
Да блин возможности по композиции в .NET почти безграничны, зачем вам метод createAll.
ну вот вы сделали CreateProduct(ingridient: "olive", amount: 10) потом в другом месте сделали CreateSpecialProduct() а потом с третьем месте понадобилось CreateSpecialProduct(ingridient: "olive", amount: 10) — как вы их скомпозируете? Придется рефакторить.
А ну да обрубок вроде
Method_ShouldWorkAsExpected_WhenGetsRightValue — так это и есть фраза на человеческом языке с точностью до написания. Мы ж все равно ее пишем, чем нам написать еще раз облегчает дело?
Да но ваш service это объект тестирования, так что это сбивает с толку.
Как сбивает? Я же обращение к методу с целью проверки результатов поместил внутрь assert — наоборот, в этом случае Act отделен он Assert если вынести GetReservedAmount в отдельную переменную, придется писать комментарий
// Assert
перед GetReservedAmount. Нет?
Теперь больше конкретики:
1) Написав такой DSL вы тестируете в итоге работу DSL в первую очередь, а не систему.
2) Тесты позволяют в том числе под другим углом посмотреть на удобство кода и открытость его для изменений и тестирования, DSL — уменьшает этот момент.
3) Для лучшей читаемости есть комментарии и cucumber(в том числе для .net)
4) Debug такого теста может быть очень увлекательным, DSL имеет свойство становиться сложнее со временем.
- Это не так.
- То, что удобно использовать при разработке бизнес-логики не обязано быть удобным для тестов и DSL как раз помогает сгладить это несоответствие.
- Мы стараемся писать комментарии только в самых крайних случаях. А использовать BDD фреймворки в юнит-тестах мне представляется оверхедом посильнее DSL.
- Дебаг любого теста со сложным сетапом не самое приятное занятие. Согласен, DSL явно не поможет его упростить, но и не сильно усложнит.
Где здесь месяц, а где день? Если не помнишь конструктор наизусть — не очевидно. А если код смотрят люди из разных стран с разными локалями (например, смотрит аналитик из США), то может быть совсем печально.
Год, месяц, день. Как в формате ISO 8601, в JavaScript, в SQL, в Java. И потом, это часть базовой библиотеки, и с датами приходится сталкиваться достаточно часто, чтобы порядок аргументов в конструкторе запомнился. А вот в 4.May(2019) — это как раз локальный российский формат, людям из разных стран с разными локалями он не поможет.
WhenAcceptOrder_AddIsCalled
и WhenAcceptOrder_ReserveIngredientsIsCalled
, конструирование его излишне и не помогает в достижении цели теста. Тут достаточно мока order
.2) Не понятно, зачем нужен
PizzeriaServiceTestable
? Для доступа к protected
методам? А стали они protected
только ради тестов? Если вам нужно верифаить protected/private
методы, возможно, вы что-то делаете не так.Не понял, почему не помогает? Ордер — это, кстати, не мок, а тестовые данные.
Конкретно здесь Testable нужен только для того, чтобы завернуть в себя ассёрты на моках и создание самих моков. Protected методов тут нет.
Но, например для рефакторинга легаси можно его использовать и для вызова protected. Пользы будет больше чем зла.
Если тест не прошёл, значит «что-то не так»?
- либо какая-то (неизвестно какая) задейстованная функция была как-то изменена (например, Create.Order перестал принимать две пиццы подряд в этом формате). Т.е. так-то всё везде работает (переписали уже), нужно теперь ещё тесты переписывать.
- либо у вас поменялось меню и Pizza.Pepperoni или Pizza.Margarita теперь не существуют или называются по-другому
- либо в самом деле «что-то не так»
Если тест прошёл, значит «всё как надо»? Нет:
- проверяется только факт вызова Add и ReserveIngredients, а не результат их работы. То есть внутри этих функций можно хоть весь код удалить и всё будет продолжать тестироваться на ура
- проверяется только на этом конкретном заказе. Т.е. WhenAcceptOrder_ReserveIngredientsIsCalled() выглядит обманчиво — звучит так, как будто при любом приёме заказа должны (и будут) резервироваться ингредиенты. И даже если сейчас это так, то через год у вас в товарах могут добавиться электронные книги. Т.е. тест будет зелёный, а логика не проверена.
Не лучше ли, когда тест просто проверяет, что «при заказе пиццы Пепперони зарезервировано 150 г. сыра»? При этом Пепперони заменяется на «номер один в текущем меню» а 150 и «сыр» на соответсвующее количество и ингредиент из БД для этого номера. И без разницы, вызовом каких функций и как это происходит.
Можно ли накосячить при написании DSL? Да, разумеется.
Можно ли накосячить в сложном сетапе юнит-теста? Да, разумеется.
Разница только в том, что в первом случае косяк придется править только в одном месте, а во втором во всех тестах, использующих этот сетап.
По поводу того, что проверяется только вызов метода: это называется "тест на поведение". О том, что следует писать тесты на состояние, а не на поведение я упоминал в статье.
Ну и в целом, если юнит-тесты проходят, это еще далеко не значит, что у вас всё хорошо. Зато если они падают, значит всё точно плохо :)
Если тест не прошёл, значит «что-то не так»?
Вы же не знаете, какое приложение человек пишет. В данном случае, понятно, что человек пишет компилятор c#. С-но, он написал на c# код с вызовом определенных методов, и теперь проверяет, что данный код, будучи скомпилированным и запущенным, действительно вызывает эти методы.
Просто название для теста придумано неудачное, вот и все.
Кстати, в чем проблема была написать просто:
[Fact]
public void AcceptOrder_Successful()
{
var order = new Order(DateTime.Now);
service.AcceptOrder(order);
orderRepositoryMock.Verify(r => r.Add(order), Times.Once);
ingredientsRepositoryMock.Verify(r => r.ReserveIngredients(order), Times.Once);
}
Или ф-я AcceptOrder занимается валидацией заказа и не пропустит такой заказ? Так надо тогда вынести валидацию в отдельную ф-ю и мокнуть ее, чтобы она сделала valid на order.
И валидацию проверять отдельным тестом самой ф-и валидации, т.к. иначе п.2 (сфокусированность) нарушено.
public class AuthenticateShould
{
[Fact]
public async Task ReturnSuccess_WhenTokenCorrect()
{
var actual = await Authenticate("correct_token");
actual.Result.Should().Be(AuthenticationResult.Success);
}
[Fact]
public async Task ProlongateSessionForHour_WhenTokenCorrect()
{
currentMomentSetup.Returns(new DateTime(2019, 01, 01, 10, 05, 20));
await Authenticate("correct_token");
sessionRepository.Verify(r => r.RefreshSession(It.Is(userId), It.Is(sessionId), It.Is(new DateTime(2019, 01, 01, 11, 05, 20)), Times.Once);
sessionRepository.VerifyNoOtherCalls();
}
[Fact]
public async Task UpdateUserActivity_WhenTokenCorrect()
{
currentMomentSetup.Returns(new DateTime(2019, 02, 03));
await Authenticate("correct_token");
userRepository.Verify(r => r.Update(It.IsUpdateBuilder<User>().WithField(u => u.LastActivityDate, new DateTime(2019, 02, 03))), Times.Once);
userRepository.VerifyNoOtherCalls();
}
}
Вот мне интересно про VerifyNoOtherCalls — это не приводит к излишней хрупкости теста? Т.е. фактически мы проверяем не только что он делает что-то но и то что он не делает ничего еще — если в будущей появится новое требование мы должны будем переделывать все такие тесты. Что из этого получается на практике?
Еще хотелось бы увидеть общий подход к организации тестов — на какие куски делите, и какими инструментами отслеживаете структуру (сколько уровней категоризации и так далее и насколько устраивает то, что получилось)?
Если возникнет необходимость дважды вызывать репозиторий, то тогда конечно придется этот тест переписывать, но по-моему лучше держать архитектуру такой, чтобы этого не случалось, если есть возможность.
По поводу структуры — предпочитаю не повторять структуру неймспейсов из тестируемого проекта, вместо этого просто держу все в папках с названиями классов, а внутри делю по тестовым классам по методам или даже подметодам. В случае с примером выше, можно даже целый тестовый класс назвать AuthenticationService.AuthenticateWithCorrectTokenShould. Критерия по разделению строго нет, просто слежу за тем, чтобы тестовые классы не становились слишком жирными. Для этого еще отдельно обращаю внимание на неиспользуемые зависимости, стараюсь, чтобы их было поменьше. Это, как мне кажется, помогает придерживаться SRP, а в итоге вместо обычных для индустрии медиаторов типа CommentaryService, где сложена вся логика работы с комментариями, у меня есть CreateCommentaryService, DeleteCommentaryService, LikeCommentaryService и ReplyCommentaryService. Все классы строго отвечают за бизнес-процессы, очень легко покрываются тестами, которые даже не всегда надо разносить по разным файлам (как можно понять по названиям, в них редко бывает больше одного метода).
Пока вроде бы устраивает, что получается. В основном даже не в самих тестах, а в том, как меняется код, чтобы повысить свою тестабильность.
Итак, я хочу сделать так, чтобы в моём тестовом методе не было ни одного мока.
PizzeriaServiceTestable проверяет неявные выходные данные (indirect output) тестируемой системы. Фактически, это мок, просто специализированный.
Волшебная фея для юнит-тестов: DSL в C#