Conversation
| [TestCase(-1, 2, true)] | ||
| [TestCase(-1, 2, false)] | ||
| [TestCase(1, 0, false, false)] | ||
| public void CheckParametersValidation( |
There was a problem hiding this comment.
Название тестов должно говорить само за себя. Когда происходит объединение всего в один тест с указанием флагов true/false читаемость в разы падает. Думаю, здесь нужно разделить на отдельно успешные и неуспешные сценарии
| [TestCase("-123", false, 4)] | ||
| [TestCase("-123", true, 4, 0, false)] | ||
| [TestCase("123", true, 3)] | ||
| public void CheckNumberValidation( |
There was a problem hiding this comment.
А чем этот тест отличается от того, что было изначально? Из-за такого обилия кейсов не понятно, что именно проверяется и какая логика закладывается в проверку
| [TestCase("-123", false, 4)] | ||
| [TestCase("-123", true, 4, 0, false)] | ||
| [TestCase("123", true, 3)] | ||
| public void CheckNumberValidation( |
There was a problem hiding this comment.
Название теста должно отражать тот объект, который подвергается тестированию. В данном случае это должно быть IsValidNumber_ShouldBe???_When???
| && AreEqual(actual.Parent, expected.Parent); | ||
| } | ||
|
|
||
| private void CheckTsarFields(Person? actual, Person? expected, int genNumber) |
There was a problem hiding this comment.
Обсудили в личке, что такой подход не отличается от предложенного изначально решения и имеет все те же недостатки. Договорились изучить FluentAssertions и реализовать сравнение, удобное в поддержке и расширении в будущем
| [TestCase(-1)] | ||
| [TestCase(1, -1)] | ||
| [TestCase(1, 2)] | ||
| [TestCase(-1, 2, true)] |
There was a problem hiding this comment.
Этот тест проверяет сразу два условия и по сути повторяет уже существующие
| [TestCase("-0.00", false, 3, 2)] | ||
| [TestCase("-0.00", false, 4, 2)] |
There was a problem hiding this comment.
Значение по умолчанию сводит на нет всю пользу тестов. Смотришь на них и не понятно, что меняется и почему они оба должны быть неуспешными.
| [TestCase("-0.00", false, 3, 2)] | ||
| [TestCase("-0.00", false, 4, 2)] | ||
| [TestCase("-0.00", true, 4, 2, false)] | ||
| [TestCase("+0.00", false, 3, 2)] |
There was a problem hiding this comment.
Этот тест не дублирует уже существующий?
| [TestCase("-0.00", true, 4, 2, false)] | ||
| [TestCase("+0.00", false, 3, 2)] | ||
| [TestCase("+1.23", true, 4, 2)] | ||
| [TestCase("+1.23", false, 3, 2)] |
There was a problem hiding this comment.
Этот тест не дублирует уже существующий?
| [TestCase("+0.00", false, 3, 2)] | ||
| [TestCase("+1.23", true, 4, 2)] | ||
| [TestCase("+1.23", false, 3, 2)] | ||
| [TestCase("-1.23", false, 3, 2)] |
There was a problem hiding this comment.
Этот тест не дублирует уже существующий?
| [TestCase("-123", false, 4)] | ||
| [TestCase("-123", true, 4, 0, false)] | ||
| [TestCase("123", true, 3)] | ||
| public void CheckNumberValidation( |
There was a problem hiding this comment.
Не хватает тестов на проверку символов в числе и перед ним
| Assert.Fail("The verification depth has reached its maximum value."); | ||
| } | ||
| actualTsar.Should().BeEquivalentTo(expectedTsar, options => options | ||
| .Excluding(info => info.Path.EndsWith(nameof(Person.Id)))); |
There was a problem hiding this comment.
А как сработает это правило, если появится новое свойство с именем "GlobalId", которое должно совпадать?
| [TestCase(1, -1, false, "Negative number of digits in fractional part")] | ||
| [TestCase(1, 2, false, | ||
| "Number of digits in fractional part greater or equal to total number of digits")] | ||
| public void NumberValidatorConstructor_ShouldThrow_ArgumentException( |
There was a problem hiding this comment.
Достаточно сказать просто про конструктор, так как NumberValidator уже содержится в имени класса и отобразится в интерфейсе
| [TestCase(-1, 2, false)] | ||
| [TestCase(1, 0, false, false)] | ||
| public void CheckParametersValidation( | ||
| [TestCase(-1, 0, false, "Non-positive total number of digits")] |
There was a problem hiding this comment.
Это не просто NonPositive, это прямо negative. Всё-таки важный кейс
| [TestCase(-1, 0, false, "Non-positive total number of digits")] | ||
| [TestCase(1, -1, false, "Negative number of digits in fractional part")] | ||
| [TestCase(1, 2, false, | ||
| "Number of digits in fractional part greater or equal to total number of digits")] |
| bool shouldThrowException = true) | ||
| int scale, | ||
| bool onlyPositive, | ||
| string message) |
There was a problem hiding this comment.
Достаточно указывать ту же строку в качестве названия теста. Нет смысла тянуть отдельный параметр
There was a problem hiding this comment.
На самом деле я имел в виду Name у TestCase. Существующее решение засчитываю, оно имеет полное право на существование, но на будущее, достаточно прописать параметр Name у атрибута
| [TestCase("0.", 17, 2, true)] | ||
| [TestCase("a23", 17, 2, true)] | ||
| [TestCase("1e3", 17, 2, true)] | ||
| [TestCase("1.a23", 17, 2, true)] |
There was a problem hiding this comment.
Этот тест гарантированно не пройдёт по причине буквенных символов?
GlazProject
left a comment
GlazProject
left a comment
There was a problem hiding this comment.
Задание засчитываю, ставлю полный балл за решение
2 participants
@GlazProject