Бабин Георгий #240
Бабин Георгий #240
Conversation
| @@ -5,28 +5,54 @@ | |||
|
|
|||
| namespace HomeExercises | |||
| { | |||
| [TestFixture] | |||
There was a problem hiding this comment.
[TestFixture] можно не писать, если внутри класса есть [Test] или [TestCase], .net всё поймет)
| [TestCase(1, -1, false, TestName = "scale is negative")] | ||
| [TestCase(1, 2, false, TestName = "scale is greater than precision")] | ||
| [TestCase(1, 1, false, TestName = "scale is equals precision")] |
There was a problem hiding this comment.
Почему в последних трех тестах поменялся флаг onlyPositive? Разве мы можем гарантировать, что не он стал виновником падения при создании объекта?
Обычно в тесткейсе меняется какой-то один проверяемый или пара связанных параметров, тогда явно видна зависимость параметров от результатов работы кода. Сейчас же эта связь размазалась, т.к. в названии тесткейса фигурируют только scale и precision, а меняется ещё и onlyPositive
| [TestCase(3, 2, true, "+1..23", TestName = "value has incorrect format")] | ||
| [TestCase(17, 2, true, "0.000", TestName = "value's fraction part length is greater than scale")] | ||
| [TestCase(5, 2, true, "-0.00", TestName = "negative sign when onlyPositive is true")] | ||
| [TestCase(3, 2, true, "+0.00", TestName = "intPart and fractPart together is greater than precision")] |
There was a problem hiding this comment.
Давай подумаем, точно ли все тесткейсы здесь обработаны. Безусловно, ты уже обработал основные, но есть так же и некоторые граничные и совсем неадекватные вещи)) Никогда не знаешь, чего можно ожидать от пользователя)
cs/HomeExercises/ObjectComparison.cs
Outdated
| } | ||
|
|
||
| [Test] | ||
| [Description("Альтернативное решение. Какие у него недостатки?")] | ||
| /* 1. Непонятно из названия, что тест проверяет и какого результата ожидает. |
cs/HomeExercises/ObjectComparison.cs
Outdated
| @@ -7,28 +7,33 @@ public class ObjectComparison | |||
| { | |||
| [Test] | |||
| [Description("Проверка текущего царя")] | |||
| [Category("ToRefactor")] | |||
| public void CheckCurrentTsar() | |||
There was a problem hiding this comment.
Ты написал, что нейминг соседнего теста не блещет читаемостью и понятностью, а что скажешь насчет этого?)
| } | ||
|
|
||
| [TestCase(3, 2, true, null, TestName = "value is null")] | ||
| [TestCase(3, 2, true, "", TestName = "value is empty")] |
There was a problem hiding this comment.
Зачастую null, пустая строка и ещё пробел идут паровозиком
| @@ -5,28 +5,54 @@ | |||
|
|
|||
| namespace HomeExercises | |||
There was a problem hiding this comment.
Понимаю, что этот файл создавал не ты, но быть может задумка автора как раз и была в том, чтобы с какими-то конвентами знакомить
В большом продакшене придется работать с тысячами файлов и классов, и всегда удобно, когда каждый класс, интерфейс, enum и тд лежит в своем собственном файле. Давай тут сделаем так же
Аналогично с соседним файлом
| a.Should().Throw<ArgumentException>(); | ||
| } | ||
|
|
||
| [TestCase(1, 0, true, TestName = "with all arguments")] |
There was a problem hiding this comment.
Нужен ли [TestCase], когда тесткейс всего один?
| { | ||
| public class NumberValidatorTests | ||
| { | ||
| [TestCase(0, 2, false, TestName = "precision is zero")] |
There was a problem hiding this comment.
По факту onlyPositive сейчас не используется, т.к. никак не влияет на результаты работы теста и всегда false
| [Test] | ||
| public void Constructor_Success_WithThreeArguments() | ||
| { | ||
| Action a = () => { new NumberValidator(1, 0, false); }; | ||
| a.Should().NotThrow(); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Constructor_Success_WithTwoArguments() | ||
| { | ||
| Action a = () => { new NumberValidator(1, 0); }; | ||
| a.Should().NotThrow(); | ||
| } | ||
|
|
||
| [Test] | ||
| public void Constructor_Success_WithOneArgument() | ||
| { | ||
| Action a = () => { new NumberValidator(1); }; | ||
| a.Should().NotThrow(); | ||
| } |
There was a problem hiding this comment.
Второй и третий аргумент совпадает с аргументами по умолчанию - те же значения, поэтому нет разницы - вызовешь ты конструктор явно с тремя аргументами, как в первом тесте, либо с одним, как в последнем, результат один и тот же.
Поэтому тесты можно схлопнуть в один с [TestCase], если хочешь поиграться с аргументами, либо просто схлопнуть под один [Test]
| [TestCase(17, 2, true, "0.000", TestName = "value's fraction part length is greater than scale")] | ||
| [TestCase(5, 2, true, "-0.00", TestName = "negative sign when onlyPositive is true")] | ||
| [TestCase(3, 2, true, "+0.00", TestName = "intPart and fractPart together is greater than precision")] | ||
| public void IsValidNumber_ReturnsFalse_OnIncorrectArguments(int precision, int scale, bool onlyPositive, string value) |
There was a problem hiding this comment.
А если разрешены только положительные, а передаем отрицательное?
@desolver