Code Review - 5-й этап (приёмочные тесты)

[sergey-shambir]

Я смотрел ветку feature/currency, коммит 768e53a6fe3a21007ce3b5df4db517eb64a22e60

Список замечаний

1. При запуске команды dotnet build я вижу предупреждения компилятора, а при запуске команды dotnet test я вижу ошибки в Unit-тестах
   • перед коммитом лучше прогонять dotnet clean и затем dotnet test, чтобы убедиться в отсутствии проблем
2. В тестах используются ключевые слова "Пусть/Когда/Тогда" — в классе CbrSteps лучше для префиксов в именах методов применять те же слова
   • пример: вместо ДопустимЯИмпортировалДанныеИзФайлаЗаДату лучше писать ПустьЯИмпортировалДанныеИзФайлаЗаДату
3. В тесте для фраз следует использовать соответствующее время по таблице ниже, а сами фразы оформлять в стиле, понятном эксперту предметной области или QA
   • можно совершать стилистические ошибки — например, многократные повторения нужны, чтобы различать похожие шаги в разных тестах, например, "тогда будут видны инвойсы" и "тогда будут видны контрагенты"
   • пример дефекта: фраза Тогда курсы имеют дату "2008-08-26"
   • лучше написать: "Тогда за дату 2008-08-26 появятся курсы валют:" (а на следующей строке таблица)
4. Для проверки курсов валют лучше передавать данные в шаг в виде таблицы
   • для этого в Reqnroll есть класс Table
   • см. https://docs.reqnroll.net/latest/automation/datatable-helpers.html

Почему таблицы

Таблица позволит писать кратко и ёмко — ближе к языку экспертов предметной области

• при этом легко проверяемы как количество записей о курсах валют за определённую дату, так конкретные значения
• советую использовать горизонтальную таблицу с данными — т.е. каждая колонка обозначает один атрибут, а каждая строка обозначает одну запись (объект)
  • в AccountingGateway, напротив, используются вертикальные таблицы с данными, потому что там объекты имеют много атрибутов и горизонтальную таблицу было бы трудно читать

Время для глаголов на разных шагах

Тип шага	Время	Пример фразы	Особенности
Пусть	Прошедшее	Пусть импортировали инвойсы	Сообщаем о предусловиях до совершения действия
Когда	Настоящее	Когда импортируем инвойсы	Совершаем действие (обычно команда, реже — запрос к API на чтение)
Тогда	Будущее	Тогда будут сохранены инвойсы	Проверяем наблюдаемое состояние (идеально) или состояние в базе данных (допустимо)

[sergey-shambir]

1. Модульные тесты всё ещё падают на моём компьютере
   • для исправления следует явно передавать локаль вторым параметром в метод decimal.Parse
   • правильная локаль: CultureInfo.GetCultureInfo("ru-RU")
2. Не хватает интеграционного теста, который загрузит данные за две разные даты и запросит их последовательно
   • нет ничего плохого в том, чтобы в тесте дважды повторялся Когда-Тогда; зато будет уверенность, что сервис может хранить курсы валют за различные даты и при импорте они не конфликтуют
3. Предлагаю добавить в gitignore генерируемые при сборке файлы *.feature.cs, и удалить их из git: git rm -f --cached *.feature.cs