13.05.2019 13:51
ypastushenko 35 4800 Источник
Как часто бывало так, что написав рабочий юнит-тест, ты смотришь на его код, а он… плохой? И ты такой думаешь: «Это же тест, оставлю так…». Нет, %username%, так оставлять не надо. Тесты — это значимая часть системы, которая обеспечивает поддерживаемость кода, и очень важно, чтобы эта часть также была поддерживаемой. К несчастью, у нас не так много способов обеспечить это (не будем же мы писать тесты на тесты), но парочка всё-таки есть.


В нашей школе разработчиков «Dodo DevSchool» мы выделяем в числе прочих такие критерии хорошего теста:

  • воспроизводимость: запуск тестов на одном и том же коде и входных данных всегда приводит к одному и тому же результату;
  • сфокусированность: должна быть только одна причина для падения теста;
  • понятность: ну тут и так понятно. :)

Как вам такой тест с точки зрения этих критериев?

[Fact]
public void AcceptOrder_Successful()
{
  var ingredient1 = new Ingredient("Ingredient1");
  var ingredient2 = new Ingredient("Ingredient2");
  var ingredient3 = new Ingredient("Ingredient3");
 
  var order = new Order(DateTime.Now);
 
  var product1 = new Product("Pizza1");
  product1.AddIngredient(ingredient1);
  product1.AddIngredient(ingredient2);
 
  var orderLine1 = new OrderLine(product1, 1, 500);
  order.AddLine(orderLine1);

  var product2 = new Product("Pizza2");
  product2.AddIngredient(ingredient1);
  product2.AddIngredient(ingredient3);
 
  var orderLine2 = new OrderLine(product2, 1, 650);
  order.AddLine(orderLine2);

  var orderRepositoryMock = new Mock<IOrderRepository>();
  var ingredientsRepositoryMock = new Mock<IIngredientRepository>();

  var service = new PizzeriaService(orderRepositoryMock.Object, ingredientsRepositoryMock.Object);
 
  service.AcceptOrder(order);
 
  orderRepositoryMock.Verify(r => r.Add(order), Times.Once);
  ingredientsRepositoryMock.Verify(r => r.ReserveIngredients(order), Times.Once);
}

По мне — очень плохо.

Он непонятный: я, например, не могу даже выделить блоки Arrange, Act и Assert.

Невоспроизводимый: используется свойство DateTime.Now. И наконец, он несфокусированный, т.к. имеет 2 причины падения: проверяются вызовы методов двух репозиториев.

Кроме того, хотя именование тестов выходит за рамки этой статьи, я всё же обращу внимание на название: при таком наборе отрицательных свойств сложно сформулировать его так, чтобы при взгляде на имя теста, сторонний человек сразу понял для чего этот тест вообще находится в проекте.
Если не получается лаконично назвать тест, значит с тестом что-то не так.
Раз уж тест непонятный, давайте расскажу что в нём происходит:

  1. Создаются ингредиенты.
  2. Из ингредиентов создаются продукты (пиццы).
  3. Из продуктов создается заказ.
  4. Создается сервис, для которого мокаются репозитории.
  5. Заказ передаётся методу AcceptOrder сервиса.
  6. Проверяется, что были вызваны методы Add и ReserveIngredients у соответствующих репозиториев.

Итак, как нам сделать этот тест лучше? Нужно попытаться оставить в теле теста только то, что по-настоящему важно. И для этого умные люди вроде Мартина Фаулера и Ребекки Парсонс придумали DSL (Domain Specific Language). Здесь я расскажу о паттернах DSL, которые мы в Додо используем для того, чтобы наши юнит-тесты были мягкими и шелковистыми, а разработчики чувствовали себя уверенно каждый день.

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

Вынесение ингредиентов (предопределённые доменные объекты)


Начнём с блока создания заказа. Заказ — это одна из центральных доменных сущностей. Было бы круто, если бы мы могли описывать заказ так, чтобы даже люди, которые не умеют писать код, но разбираются в доменной логике могли понять что за заказ мы создаём. Для этого, в первую очередь, нам нужно отказаться от использования абстрактных «Ingredient1» и «Pizza1» заменив их на реальные ингредиенты, пиццы и прочие доменные объекты.

Первый кандидат на оптимизацию — это ингредиенты. С ними всё просто: для них не нужно никакой кастомизации, только вызов конструктора. Достаточно вынести их в отдельный контейнер и назвать так, чтобы было понятно доменным экспертам:

public static class Ingredients
{
  public static readonly Ingredient Dough = new Ingredient("Dough");
  public static readonly Ingredient Pepperoni = new Ingredient("Pepperoni");
  public static readonly Ingredient Mozzarella = new Ingredient("Mozzarella");
}

Вместо совершенно невменяемых Ingredient1, Ingredient2 и Ingredient3 мы получили Тесто, Пепперони и Моцареллу.
Используйте предопределение доменных объектов для часто использующихся доменных сущностей.

Builder для продуктов


Следующая доменная сущность это продукты. С ними всё немного сложнее: каждый продукт состоит из нескольких ингредиентов и нам придётся добавить их в продукт перед использованием.

Здесь нам пригодится старый добрый паттерн Builder. Вот как выглядит моя версия билдера для продукта:

public class ProductBuilder
{
  private Product _product;

  public ProductBuilder(string name)
  {
     _product = new Product(name);
  }

  public ProductBuilder Containing(Ingredient ingredient)
  {
     _product.AddIngredient(ingredient);
     return this;
  }

  public Product Please()
  {
     return _product;
  }
}

Он состоит из параметризованного конструктора, кастомизирующего метода Containing и терминального метода Please. Если не любите любезничать с кодом, то можно заменить Please на Now. Билдер скрывает сложные конструкторы и вызовы методов, настраивающих объект. Код становится чище и понятнее. По-хорошему билдер должен упрощать создание объекта настолько, чтобы код был понятен доменному эксперту. Особенно стоит использовать билдер для объектов, которые требуют настройки перед началом работы.

Билдер продукта позволит создавать конструкции вроде:

var pepperoni = new ProductBuilder("Pepperoni")
  .Containing(Ingredients.Dough)
  .Containing(Ingredients.Pepperoni)
  .Please();

Билдеры помогают создавать объекты, которым нужна настройка. Рассмотрите возможность создания билдера даже если настройка состоит из одной строчки.

Объект ObjectMother


Несмотря на то, что создание продукта стало намного приличнее, конструктор new ProductBuilder всё еще выглядит довольно уродливо. Починим это с помощью паттерна ObjectMother (Father).

Паттерн простой как 5 копеек: создаём статический класс и собираем в него все билдеры.

public static class Create
{
  public static ProductBuilder Product(string name) => new ProductBuilder(name);
}

Теперь можно писать так:

var pepperoni = Create.Product("Pepperoni")
  .Containing(Ingredients.Dough)
  .Containing(Ingredients.Pepperoni)
  .Please();

ObjectMother придуман для декларативного создания объектов. Кроме того он помогает вводить в домен новых разработчиков, т.к. при написании слова Create IDE сама подскажет что можно создать в этом домене.

В нашем коде ObjectMother иногда называют не Create, а Given. Оба варианта мне нравятся. Если у вас есть какие-то еще идеи — поделитесь в комментариях.
Для декларативного создания объектов используйте ObjectMother. Код станет чище, а новым разработчикам будет проще вникать в домен.

Вынесение продуктов


Стало сильно лучше, но продуктам ещё есть куда расти. Продуктов у нас ограниченное количество и их можно подобно ингредиентам собрать в отдельном классе и не инициализировать для каждого теста:

public static class Pizza
{
  public static Product Pepperoni => Create.Product("Pepperoni")
     .Containing(Ingredients.Dough)
     .Containing(Ingredients.Pepperoni)
     .Please();
 
  public static Product Margarita => Create.Product("Margarita")
     .Containing(Ingredients.Dough)
     .Containing(Ingredients.Mozzarella)
     .Please();
}

Здесь я назвал контейнер не Products, а Pizza. Такое название помогает читать тест. Например, оно помогает снять вопросы типа «А Pepperoni — это пицца или колбаска?».
Старайтесь использовать реальные доменные объекты, а не заменители вроде Product1.

Билдер для заказа (пример с обратной стороны)


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

var order = Create.Order
  .Dated(DateTime.Now)
  .With(Pizza.Pepperoni.CountOf(1).For(500))
  .With(Pizza.Margarita.CountOf(1).For(650))
  .Please();

Как мы можем этого добиться? Нам, очевидно, понадобится билдеры для заказа и строки заказа. С билдером для заказа всё кристально ясно. Вот он:

public class OrderBuilder
{
  private DateTime _date;
  private readonly List<OrderLine> _lines = new List<OrderLine>();
  
  public OrderBuilder Dated(DateTime date)
  {
    _date = date;
    return this;
  }

  public OrderBuilder With(OrderLine orderLine)
  {
    _lines.Add(orderLine);
    return this;
  }

  public Order Please()
  {
    var order = new Order(_date);
    foreach (var line in _lines)
    {
      order.AddLine(line);
    }
    
    return order;
  }
}

А вот с OrderLine ситуация поинтереснее: во-первых, здесь не вызывается терминальный метод Please, а во-вторых, доступ к билдеру предоставляет не статический Create и не конструктор самого билдера. Первую проблему мы решим с помощью implicit operator и наш билдер будет выглядеть так:

public class OrderLineBuilder
{
  private Product _product;
  private decimal _count;
  private decimal _price;
 
  public OrderLineBuilder Of(decimal count, Product product)
  {
     _product = product;
     _count = count;
     return this;
  }

  public OrderLineBuilder For(decimal price)
  {
     _price = price;
     return this;
  }

  public static implicit operator OrderLine(OrderLineBuilder b)
  {
     return new OrderLine(b._product, b._count, b._price);
  }
}

Со второй нам поможет разобраться Extension-метод для класса Product:

public static class ProductExtensions
{
  public static OrderLineBuilder CountOf(this Product product, decimal count)
  {
     return Create.OrderLine.Of(count, product)
  }
}

Вообще Extension-методы — это большие друзья DSL. Они могут из совершенно адской логики сделать декларативное понятное описание.
Используйте extension-методы. Просто используйте их. :)
Сделав все эти действия мы получили вот такой код теста:

[Fact]
public void AcceptOrder_Successful()
{
  var order = Create.Order
     .Dated(DateTime.Now)
     .With(Pizza.Pepperoni.CountOf(1).For(500))
     .With(Pizza.Margarita.CountOf(1).For(650))
     .Please();
 
  var orderRepositoryMock = new Mock<IOrderRepository>();
  var ingredientsRepositoryMock = new Mock<IIngredientRepository>();

  var service = new PizzeriaService(orderRepositoryMock.Object, ingredientsRepositoryMock.Object);

  service.AcceptOrder(order);
 
  orderRepositoryMock.Verify(r => r.Add(order), Times.Once);
  ingredientsRepositoryMock.Verify(r => r.ReserveIngredients(order), Times.Once);
}

Здесь мы применили подход, который мы называем «Волшебная фея». Это когда ты сначала пишешь неработающий код так как тебе хотелось бы его видеть, а потом пытаешься завернуть то, что написал, в DSL. Так действовать очень полезно — иногда ты и сам не представляешь на что способен C#.
Представьте, что прилетела волшебная фея и разрешила вам писать код так, как хочется, а потом попробуйте обернуть написанное в DSL.

Создание сервиса (паттерн Testable)


С заказом теперь всё более-менее неплохо. Настало время разобраться с моками репозиториев. Здесь стоит сказать, что сам по себе тест, который мы рассматриваем — это тест на поведение. Тесты на поведение сильно связаны с реализацией методов и, если есть возможность не писать такие тесты, то лучше этого не делать. Однако, иногда они бывают полезны, а временами, без них вообще не обойтись. Следующая техника помогает писать именно тесты на поведение и если вы вдруг понимаете, что хотите воспользоваться ей, то сначала задумайтесь, нельзя ли переписать тесты таким образом, чтобы они проверяли состояние, а не поведение.

Итак, я хочу сделать так, чтобы в моём тестовом методе не было ни одного мока. Для этого я создам обёртку для PizzeriaService, в которой инкапсулирую всю логику, которая проверяет вызовы методов:

public class PizzeriaServiceTestable : PizzeriaService
{
  private readonly Mock<IOrderRepository> _orderRepositoryMock;
  private readonly Mock<IIngredientRepository> _ingredientRepositoryMock;

  public PizzeriaServiceTestable(Mock<IOrderRepository> orderRepositoryMock, Mock<IIngredientRepository> ingredientRepositoryMock)
     : base(orderRepositoryMock.Object, ingredientRepositoryMock.Object)
  {
     _orderRepositoryMock = orderRepositoryMock;
     _ingredientRepositoryMock = ingredientRepositoryMock;
  }

  public void VerifyAddWasCalledWith(Order order)
  {
     _orderRepositoryMock.Verify(r => r.Add(order), Times.Once);
  }

  public void VerifyReserveIngredientsWasCalledWith(Order order)
  {
     _ingredientRepositoryMock.Verify(r => r.ReserveIngredients(order), Times.Once);

  }
}

Этот класс позволит нам проверять вызовы методов, но нам ещё нужно как-то его создать. Для этого воспользуемся уже знакомым нам билдером:

public class PizzeriaServiceBuilder
{
  public PizzeriaServiceTestable Please()
  {
     var orderRepositoryMock = new Mock<IOrderRepository>();
     var ingredientsRepositoryMock = new Mock<IIngredientRepository>();

     return new PizzeriaServiceTestable(orderRepositoryMock, ingredientsRepositoryMock);
  }
}

На текущий момент наш тестовый метод выглядит так:

[Fact]
public void AcceptOrder_Successful()
{
  var order = Create.Order
     .Dated(DateTime.Now)
     .With(Pizza.Pepperoni.CountOf(1).For(500))
     .With(Pizza.Margarita.CountOf(1).For(650))
     .Please();
 
  var service = Create.PizzeriaService.Please();

  service.AcceptOrder(order);
 
  service.VerifyAddWasCalledWith(order);
  service.VerifyReserveIngredientsWasCalledWith(order);
}

Проверки вызовов методов — не единственное для чего может использоваться Testable-класс. Вот, например, здесь наш Дима Павлов использует его для сложного рефакторинга легаси-кода.
Testable способен спасти положение в самых сложных случаях. Для тестов на поведение он помогает обернуть уродливые проверки вызовов в красивые методы.
На этом знаменательном моменте мы закончили разбираться с понятностью теста. Осталось сделать его воспроизводимым и сфокусированным.

Воспроизводимость (Literal Extension)


Паттерн Literal Extension не имеет прямого отношения к воспроизводимости, но поможет нам именно с ней. Наша проблема на текущий момент в том, что мы используем дату DateTime.Now в качестве даты заказа. Если вдруг начиная с какой-то даты, логика приёма заказа изменится, то в нашей бизнес-логике мы должны будем хотя бы какое-то время поддерживать 2 логики принятия заказа, разделяя их проверкой вроде if (order.Date > edgeDate). В этом случае у нашего теста есть шанс упасть при переходе системной даты через граничную. Да, мы быстро это пофиксим, и даже сделаем из одного теста два: один будет проверять логику до граничной даты, а другой после. Тем не менее таких ситуаций лучше избегать и сразу делать все входные данные постоянными.

«Причём же здесь DSL?» — спросите вы. Дело в том, что даты в тестах удобно вводить через Extension-методы, например 3.May(2019). Такая форма записи будет понятна не только разработчикам, но и бизнесу. Для этого нужно всего лишь создать такой статический класс

public static class DateConstructionExtensions
{
   public static DateTime May(this int day, int year) => new DateTime(year, 5, day);
}

Естественно, даты — не единственное, для чего можно использовать этот паттерн. Например, если бы мы вводили количество ингредиентов в составе продуктов, то могли бы написать что-то вроде 42.Grams("flour").
Количественные объекты и даты удобно создавать через уже знакомые extension-методы.

Сфокусированность


Почему важно делать тесты сфокусированными? Дело в том, что сфокусированные тесты легче поддерживать, а поддерживать их еще как придется. Например, их приходится менять при изменении кода и удалять при выпиливании старых фич. Если тесты не сфокусированные, то при изменении логики нужно будет разобраться в больших тестах, и выпилить из них куски проверяемой функциональности. Если же тесты сфокусированные и их названия понятны, то нужно просто удалить устаревшие тесты и написать новые. Если в тестах есть хороший DSL, то это вообще не проблема.

Итак, после того, как мы закончили писать DSL, у нас появилась возможность сделать этот тест сфокусированным, разделив его на 2 теста:

[Fact]
public void WhenAcceptOrder_AddIsCalled()
{
  var order = Create.Order
     .Dated(3.May(2019))
     .With(Pizza.Pepperoni.CountOf(1).For(500))
     .With(Pizza.Margarita.CountOf(1).For(650))
     .Please();
 
  var service = Create.PizzeriaService.Please();

  service.AcceptOrder(order);
 
  service.VerifyAddWasCalledWith(order);
}

[Fact]
public void WhenAcceptOrder_ReserveIngredientsIsCalled()
{
  var order = Create.Order
     .Dated(3.May(2019))
     .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);
}

Оба теста получились короткими, понятными, воспроизводимыми и сфокусированными.

Обратите внимание, что теперь названия тестов отражают цель, ради которой они были написаны и теперь любой разработчик, зашедший в мой проект, поймет зачем был написан каждый из тестов и что в этом тесте происходит.
Сфокусированность тестов делает их поддерживаемыми. Хороший тест обязан быть сфокусированным.
И вот, я уже слышу как вы кричите мне «Юра, ты что охренел? Мы написали миллион кода только для того, чтобы сделать пару тестов красивенькими?». Да, именно так. Пока у нас всего пара тестов, имеет смысл вложиться в DSL и сделать эти тесты понятными. Один раз написав DSL, ты получаешь кучу плюшек:

  • Новые тесты становится писать легко. Не нужно 2 часа настраивать себя на юнит-тестирование, просто берешь и пишешь.
  • Тесты становятся понятными и читаемыми. Любой разработчик взглянув на тест понимает, зачем он был написан и что проверяет.
  • Снижается порог вхождения в тесты (а может быть и в домен) для новых разработчиков. Например, через ObjectMother можно легко разобраться какие объекты можно создать в домене.
  • Ну и наконец, с тестами просто приятно работать, и, как следствие, код становится более поддерживаемым.

Исходный код примера и тесты доступны здесь.

Комментарии (35)


  1. Quilin
    13.05.2019 18:29
    #20147514
    +1

    И как приятно видеть такой пост от додопиццы после тех сказочных статей о найме к вам. Это очень здорово!


  1. UnclShura
    13.05.2019 22:00
    #20148310
    +2

    Вообще говоря, паттерн builder и рождаемый им DSL штука спорная. Я вообще считаю builder антипаттерном потому, что он заставляет помнить ещё и птичий язык DSL, который уникален для данного билдера. Взять хоть пример из статьи — это же очевидно что. Please() надо вызывать в конце цепочки With, а не перед. И другого билдера с Please нет и не будет.


    1. ypastushenko Автор
      14.05.2019 09:53
      #20149626

      Я думаю, тут нет однозначного ответа использовать или нет. Я считаю, что использование билдеров несёт больше плюсов чем минусов. Более того, с минусами сам ни разу не сталкивался. Возможно у вас другой опыт и это интересно.
      Расскажите плз, как вам мешала необходимость изучения структуры билдеров?


  1. Atreides07
    14.05.2019 01:03
    #20148822
    +1

    Смущает нарушение коммутативности в 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();


    Или такое поведение было задумано изначально?


    1. ypastushenko Автор
      14.05.2019 01:18
      #20148856
      +1

      Справедливое замечание. Спасибо, поправил.


  1. ApeCoder
    14.05.2019 09:11
    #20149462
    +1

    Новые тесты становится писать легко. Не нужно 2 часа настраивать себя на юнит-тестирование, просто берешь и пишешь.

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


    1. ypastushenko Автор
      14.05.2019 09:30
      #20149520

      Да, так и надо. Всё, кроме extension'ов студия и райдер смогут создать.


  1. vba
    14.05.2019 11:33
    #20150166

    Не совсем понятно нафиг все это надо, если есть AutoFixture.


    1. ypastushenko Автор
      14.05.2019 12:48
      #20150824

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


      1. vba
        14.05.2019 14:41
        #20151568

        Не совсем понимаю в чем соль доморощенного DSL, зачем время тратить на классы строители? Я бы вас понял если бы вы писали в эпоху Java6 на ней самой. Классы строители как шаблон изжили себя в среде .NET.


        1. ApeCoder
          14.05.2019 22:27
          #20153544

          Я думаю, это скорее зависит от языка, чем от фреймворка.
          В случае С# 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}


          1. ypastushenko Автор
            15.05.2019 10:11
            #20154692

            Да, рекорды это круто. Скорее бы хотя бы в работу взяли)


          1. vba
            15.05.2019 10:17
            #20154732

            Вы привели достаточно простой пример строителя, но в настоящей жизни нам пришлось бы иметь дело со строителем на 3 или 4 десятка строк больше. Иметь такое чудо в коде теста, на мой взгляд, не приемлемо. Тест это не только тест, но и живая документация. Так что смысла в доморощенных строителях не вижу, тем более что с AutoFixture все можно персонализировать и если это делать то в отдельном вспомогательном методе. 


              // Arrange
  var order = CreateDualOrder();  
  var service = CreatePizzeriaService();
  // Act
  service.AcceptOrder(order);
  // Assert
  service.VerifyReserveIngredientsWasCalledWith(order);


            1. ApeCoder
              15.05.2019 15:03
              #20156360

              В книжке 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 не пользовался, интересно, как там с воспроизводимостью падений. Например, в данном случае вполне возможно, что результат будет работать с точностью до округлений (каковые будет проверять другой тест).


              1. vba
                15.05.2019 15:23
                #20156496

                По мне так все равно куча ненужного шума:


                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.


                1. ApeCoder
                  15.05.2019 15:42
                  #20156626

                  Лучше

                  Возможно — но тогда придется все сочетания поддерживать в одном методе.


                  Кстати в той же книге предлагается награждать описанием проверки типа

                  С моей точки зрения, когда в тесте ровно один ассерт и название достаточно говорящее, это не надо. Кстати.


                  Не совсем уверен что service.GetReservedAmount(ingridient) и AmountOfProductInOrderLine * AmountOfIngridientInProduct уместны в секции Assert, по книге аrt of unit testing.

                  Почему?


                  1. vba
                    15.05.2019 16:31
                    #20156900

                    Возможно — но тогда придется все сочетания поддерживать в одном методе.

                    Ну это уж вам решать как структурировать код, можно и не в одном методе.


                    С моей точки зрения, когда в тесте ровно один ассерт и название достаточно говорящее, это не надо.

                    Здесь страдает не читаемость кода теста, а ошибки в ходе прогона тестов.


                    Почему?

                    Ну хотя бы потому что:
                    Секция 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.


                    1. ApeCoder
                      15.05.2019 17:05
                      #20157108

                      Ну это уж вам решать как структурировать код, можно и не в одном методе.

                      Я имел ввиду, что у вас метод уже создает что-то — соответственно сочетать два таких метода вместе не удастся. Метод не умеет модифицировать уже готовый ордер, например.


                      Здесь страдает не читаемость кода теста, а ошибки в ходе прогона тестов.

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


                      Секция 8.3.4 Separating asserts from actions

                      Я думаю, имелось ввиду action — это то, что мы проверяем, а assert — то, как мы проверяем. В моем коде action это AcceptOrder. Я посмотрел несколько примеров из книжки, там используются выражения и вызовы методов в assert.


                      1. vba
                        15.05.2019 17:36
                        #20157264

                        Я имел ввиду, что у вас метод уже создает что-то — соответственно сочетать два таких метода вместе не удастся. Метод не умеет модифицировать уже готовый ордер, например.

                        Да блин возможности по композиции в .NET почти безграничны, зачем вам метод createAll.


                        Обычно названия метода-теста выводятся при ошибках…

                        А ну да обрубок вроде Method_ShouldWorkAsExpected_WhenGetsRightValue заменяет полноценную фразу на человеческом языке(xUnit не в счет)


                        Я думаю, имелось ввиду action

                        Да но ваш service это объект тестирования, так что это сбивает с толку.


                        1. ApeCoder
                          15.05.2019 22:19
                          #20158386

                          Да блин возможности по композиции в .NET почти безграничны, зачем вам метод createAll.

                          ну вот вы сделали CreateProduct(ingridient: "olive", amount: 10) потом в другом месте сделали CreateSpecialProduct() а потом с третьем месте понадобилось CreateSpecialProduct(ingridient: "olive", amount: 10) — как вы их скомпозируете? Придется рефакторить.


                          А ну да обрубок вроде

                          Method_ShouldWorkAsExpected_WhenGetsRightValue — так это и есть фраза на человеческом языке с точностью до написания. Мы ж все равно ее пишем, чем нам написать еще раз облегчает дело?


                          Да но ваш service это объект тестирования, так что это сбивает с толку.

                          Как сбивает? Я же обращение к методу с целью проверки результатов поместил внутрь assert — наоборот, в этом случае Act отделен он Assert если вынести GetReservedAmount в отдельную переменную, придется писать комментарий
                          // Assert
                          перед GetReservedAmount. Нет?


  1. Gorniv
    14.05.2019 11:35
    #20150180

    Вы, конечно, извините, но это фигня какая-то. Первое что приходит в голову — кто-то перечитал паттернов, в особенности билдера.
    Теперь больше конкретики:
    1) Написав такой DSL вы тестируете в итоге работу DSL в первую очередь, а не систему.
    2) Тесты позволяют в том числе под другим углом посмотреть на удобство кода и открытость его для изменений и тестирования, DSL — уменьшает этот момент.
    3) Для лучшей читаемости есть комментарии и cucumber(в том числе для .net)
    4) Debug такого теста может быть очень увлекательным, DSL имеет свойство становиться сложнее со временем.


    1. ypastushenko Автор
      14.05.2019 12:58
      #20150904

      1. Это не так.
      2. То, что удобно использовать при разработке бизнес-логики не обязано быть удобным для тестов и DSL как раз помогает сгладить это несоответствие.
      3. Мы стараемся писать комментарии только в самых крайних случаях. А использовать BDD фреймворки в юнит-тестах мне представляется оверхедом посильнее DSL.
      4. Дебаг любого теста со сложным сетапом не самое приятное занятие. Согласен, DSL явно не поможет его упростить, но и не сильно усложнит.


    1. OREZ
      14.05.2019 13:04
      #20150958

      deleted


  1. Ketovdk
    14.05.2019 12:59
    #20150910
    +1

    по-моему, структуры в духе 4.May(2019) — это уже перебор. Они не имеют никакого преимущества перед new DateTime(2019, 5, 4). Если хотите, чтобы читалось без IDE и людьми из бизнеса — можно new DateTime(year:2019, month:5, day:4). А если вместо 5 хотите май — можно объявить константу. Просто extension-методы так и названы, потому-что они должны расширать функциональность определенного класса. Ну или хотя-бы создавать обертку, а вот пихать себя в DateTime — это определенно не функциональность класса int. Более того, существует возможность в попытке создать эту дату записать ее не как 4.May(2019), а 2019.May(4) и все сломать.


    1. ypastushenko Автор
      14.05.2019 13:00
      #20150918

      У нас в команде тоже нет консенсуса по этому вопросу) В статье оно приведено для ознакомления. Кому-то нравится, кому-то нет.


    1. BoresExpress
      14.05.2019 13:19
      #20151060

      new DateTime(2019, 5, 4)
      Где здесь месяц, а где день? Если не помнишь конструктор наизусть — не очевидно. А если код смотрят люди из разных стран с разными локалями (например, смотрит аналитик из США), то может быть совсем печально.


      1. Ketovdk
        14.05.2019 13:27
        #20151112

        но в 4.May(2019) тоже самое. Ну и специально для этого я добавил версию DateTime(year:2019, month:5, day:4)


        1. BoresExpress
          14.05.2019 17:29
          #20152412

          Ну, тащемто, в 4.May(2019) очевидно где месяц :)


          1. Ketovdk
            14.05.2019 17:45
            #20152462

            простите, имел в виду не день и месяц, а день и год в момент написания int.May(int)


          1. KvanTTT
            15.05.2019 04:07
            #20154094

            4, а может 2019?


  1. OREZ
    14.05.2019 13:06
    #20150966

    1) Builder не нужен в WhenAcceptOrder_AddIsCalled и WhenAcceptOrder_ReserveIngredientsIsCalled, конструирование его излишне и не помогает в достижении цели теста. Тут достаточно мока order.
    2) Не понятно, зачем нужен PizzeriaServiceTestable? Для доступа к protected методам? А стали они protected только ради тестов? Если вам нужно верифаить protected/private методы, возможно, вы что-то делаете не так.


    1. ypastushenko Автор
      14.05.2019 18:39
      #20152728

      1. Не понял, почему не помогает? Ордер — это, кстати, не мок, а тестовые данные.


      2. Конкретно здесь Testable нужен только для того, чтобы завернуть в себя ассёрты на моках и создание самих моков. Protected методов тут нет.



      Но, например для рефакторинга легаси можно его использовать и для вызова protected. Пользы будет больше чем зла.


  1. i86com
    14.05.2019 15:47
    #20151888

    То ли пример неудачный, то ли я чего не понял, но тесты выглядят слишком синтетическими. То есть — а что, собственно, они должны сигнализировать?
    Если тест не прошёл, значит «что-то не так»?

    • либо какая-то (неизвестно какая) задейстованная функция была как-то изменена (например, Create.Order перестал принимать две пиццы подряд в этом формате). Т.е. так-то всё везде работает (переписали уже), нужно теперь ещё тесты переписывать.
    • либо у вас поменялось меню и Pizza.Pepperoni или Pizza.Margarita теперь не существуют или называются по-другому
    • либо в самом деле «что-то не так»

    Если тест прошёл, значит «всё как надо»? Нет:
    • проверяется только факт вызова Add и ReserveIngredients, а не результат их работы. То есть внутри этих функций можно хоть весь код удалить и всё будет продолжать тестироваться на ура
    • проверяется только на этом конкретном заказе. Т.е. WhenAcceptOrder_ReserveIngredientsIsCalled() выглядит обманчиво — звучит так, как будто при любом приёме заказа должны (и будут) резервироваться ингредиенты. И даже если сейчас это так, то через год у вас в товарах могут добавиться электронные книги. Т.е. тест будет зелёный, а логика не проверена.


    Не лучше ли, когда тест просто проверяет, что «при заказе пиццы Пепперони зарезервировано 150 г. сыра»? При этом Пепперони заменяется на «номер один в текущем меню» а 150 и «сыр» на соответсвующее количество и ингредиент из БД для этого номера. И без разницы, вызовом каких функций и как это происходит.


    1. ypastushenko Автор
      14.05.2019 18:34
      #20152698

      Можно ли накосячить при написании DSL? Да, разумеется.
      Можно ли накосячить в сложном сетапе юнит-теста? Да, разумеется.
      Разница только в том, что в первом случае косяк придется править только в одном месте, а во втором во всех тестах, использующих этот сетап.


      По поводу того, что проверяется только вызов метода: это называется "тест на поведение". О том, что следует писать тесты на состояние, а не на поведение я упоминал в статье.


      Ну и в целом, если юнит-тесты проходят, это еще далеко не значит, что у вас всё хорошо. Зато если они падают, значит всё точно плохо :)


    1. Druu
      16.05.2019 20:15
      #20163072

      Если тест не прошёл, значит «что-то не так»?

      Вы же не знаете, какое приложение человек пишет. В данном случае, понятно, что человек пишет компилятор 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 (сфокусированность) нарушено.