Леонтьев Дмитрий #19

Dary0302 · 2024-11-04T17:41:29Z

No description provided.

Task complete

Number validator task

Buskervil

Посмотрел пока только NumberValidator

Buskervil · 2024-11-05T13:51:02Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+        bool onlyPositive,
+        string expectedResultValue)
+    {
+        return new NumberValidator(precision, scale, onlyPositive).IsValidNumber(expectedResultValue);


Здесь ассерт забыл. Должно быть что-то типа Should().BeTrue() или тому подобное, в зависимости от того, что ты хочешь проверить.

Buskervil · 2024-11-05T14:02:38Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    [TestCase(3, 1, false, "-1,1", ExpectedResult = true, TestName = "precision 3, scale 1, not only positive, result -1,1")]
+    [TestCase(2, 0, true, "+1", ExpectedResult = true, TestName = "precision 2, scale 0, only positive, result +1")]
+    [TestCase(3, 1, true, "+1.1", ExpectedResult = true, TestName = "precision 3, scale 1, only positive, result +1.1")]
+    [TestCase(3, 1, true, "+1,1", ExpectedResult = true, TestName = "precision 3, scale 1, only positive, result +1,1")]


Я бы предложил сгруппировать тест-кейсы по категориям и отобразить это на уровне методов (разные методы сделать для разных проверок) и на уровне названий (TestName отражает суть проверки, а не перечисляет входные параметры). У тебя так сделано для тестов на конструктор.

Так будет более структурировано, и читать такие тесты будет гораздо легче 🙂

Buskervil · 2024-11-05T14:11:31Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+
+    [TestCase(-1, TestName = "negative precision")]
+    [TestCase(0, TestName = "zero precision")]
+    public void NumberValidator_IncorrectPrecision_AfterCreatingIncorrectValidator(int precision, int scale = 0)


У тестов на конструктор имена не очень удались. Лучше что-нибудь типа
Contructor_WithIncorrectPrecision_Throws (по принципу Что_ПриКакихУсловиях_КакойРезультат)

Buskervil · 2024-11-05T15:08:26Z

Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs

+                options
+                    .AllowingInfiniteRecursion()
+                    .Excluding(o => o.Id)
+                    .Excluding(o => o.Parent.Id));


ага, а если будет 3 поколения, будет работать?

Buskervil · 2024-11-05T15:10:20Z

Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs

+        // При изменении класса Person придётся изменять метод AreEqual,
+        // т.е. если мы добавим поле в Person, то придётся добавить проверку на него в этом методе.
+        // При провале теста мы не получаем конкретной информации из-за чего тест не прошёл,
+        // т.к. метод AreEqual возвращает bool и скажет нам только Success или Failed.


Ага, все так

all tasks corrected

Buskervil

Со структурой теперь лучше. Можно обсудить количество тесткейсов.

длина чисел:

целая часть меньше/больше/равна допустимому, дробная меньше/больше/равна (для полного покрытия можно попарно скомбинировать)
длина чисел со знаком

знак числа:

onlyPositive true/false и варианты +/-/отсутствие знака (попарно скомбинировать)

нечисловая строка/не соответствующая по формату

пустая (у тебя есть)
null (у тебя есть)
как пробелы обрабатываются (внутри, на краях)
как буквы обрабатываются
... другие эксперименты с форматом строки

Общая суть: берешь один из проверяемых параметров и комбинаторикой перебираешь все возможные кейсы.

Buskervil · 2024-11-06T02:13:15Z

Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs

+            .BeEquivalentTo(expectedTsar, options =>
+                options
+                    .AllowingInfiniteRecursion()
+                    .Excluding(o => o.DeclaringType == typeof(Person) && o.Name == nameof(Person.Id)));


Теперь окей. При сравнении объектового графа еще иногда полезно игнорировать циклические ссылки. Но в данном случае это не обязательно, тут по бизнеслогике цари в династии повторяться не должны вроде как)

Buskervil · 2024-11-06T02:31:28Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+
+    [TestCase(".0", TestName = "intPart is missing")]
+    [TestCase("0.", TestName = "fracPart part is missing")]
+    public void IsValidNumber_MatchIsNotSuccess_ReturnsFalse(string expectedResultValue)


Вот тут имя концептуально неверное). Ты должен проверять бизнес-кейсы, а не догадываться, что в реализации есть некий Matсh по регулярке, который ничего не найдет.

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

Buskervil · 2024-11-06T02:35:52Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    }
+
+    [TestCase(2, 0, true, "+1", TestName = "number with a plus")]
+    public void IsValidNumber_CorrectPlus_ReturnsTrue(


С названиями теперь в целом хорошо. Совсем минорная шутка: на мой вкус не хватает какого-нибудь предлога во второй части, чтобы это было не просто Что_Условия_Результат, а Что_ПриКакихУсловиях_Результат. В тестах на конструктор ты используешь With, а ниже предлог потерялся.

Buskervil · 2024-11-06T02:39:30Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

-        ClassicAssert.IsFalse(new NumberValidator(17, 2, true).IsValidNumber("0.000"));
-        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("-1.23"));
-        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("a.sd"));
+    private static NumberValidator GetCorrectValidator(bool onlyPositive = false) => new(2, 1, onlyPositive);


На мой взгляд, в таких маленьких тестах это не нужно. Обычно тест должен быть максимально наглядным и это нормально, если в 10 методах подряд написано new NumberValidator(), зато сразу видно с какими параметрами. В тестах, где блок Arrange становится слошком громозким, более оправдано что-то выносить вовне.

Buskervil · 2024-11-06T02:47:15Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    }
+
+    [TestCase(1, 0, true, "0", TestName = "one digit")]
+    [TestCase(3, 0, true, "123", TestName = "three digits")]


У тебя тесткейсы где-то описывают полный набор параметор, а где-то нет. Можно выбрать один из приоритетов.
Все максимально перед глазами: пишем все параметры
Пишем только то, что сейчас важно: тогда такие параметры как onlyPositive и может быть размерности можно унести внутрь метода.

В общем, хорошо когда однообразно. Сделай как нравится, главное чтобы одинаково было.

Остались странные тесты
IsValidNumber_WithCorrectMinus_ReturnsTrue и IsValidNumber_WithCorrectPlus_ReturnsTrue, которые выбиваются из единообразия

New tests added

Buskervil

В целом, молодец)

Buskervil · 2024-11-06T12:44:32Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    [TestCase(2, 0, true, "+1", TestName = "number with a plus")]
+    [TestCase(3, 1, true, "+1.2", TestName = "number with a plus and separator dot")]
+    [TestCase(3, 1, true, "+1,2", TestName = "number with a plus and separator comma")]
+    public void IsValidNumber_WithCorrectPlus_ReturnsTrue(


что именно проверяет этот тест?
ниже аналогичный IsValidNumber_WithCorrectMinus_ReturnsTrue тоже не понятен

То что именно плюсы и минусы корректно обрабатываются при подсчете intPart

Buskervil · 2024-11-06T13:15:03Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    }
+
+    [TestCase(1, 0, true, "0", TestName = "one digit")]
+    [TestCase(3, 0, true, "123", TestName = "three digits")]


Остались странные тесты
IsValidNumber_WithCorrectMinus_ReturnsTrue и IsValidNumber_WithCorrectPlus_ReturnsTrue, которые выбиваются из единообразия

Buskervil · 2024-11-06T13:18:09Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    [TestCase(3, 0, "123", TestName = "three digits")]
+    [TestCase(3, 1, "12.3", TestName = "three digits with one number after the dot")]
+    [TestCase(3, 2, "1.23", TestName = "three digits with two number after the dot")]
+    public void IsValidNumber_WithCorrectNumberOfDigits_ReturnsTrue(


А в чем разница с IsValidNumber_WithCorrectPrecision_ReturnsTrue?

Идея была в том, чтобы в IsValidNumber_WithCorrectPrecision_ReturnsTrue проверять именно Precision без scale, а в IsValidNumber_WithCorrectNumberOfDigits_ReturnsTrue как бы комбинированная проверка

Buskervil · 2024-11-06T13:22:57Z

Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs

+    [TestCase(1, 0, "0", TestName = "one digit")]
+    [TestCase(3, 0, "123", TestName = "three digits")]
+    [TestCase(3, 1, "12.3", TestName = "three digits with one number after the dot")]
+    [TestCase(3, 2, "1.23", TestName = "three digits with two number after the dot")]


Писать в названии количество цифр бессмысленно. Тесты - это документация, ты начинаешь утверждать, что если число из 3х цифр, то метод вернет true. Здесь лучше что-то типа: "number of digits match precision". В общем-то у тебя везде все названо хорошо, подгляди в соседний метод.

Dary0302 and others added 6 commits November 4, 2024 17:07

Refactored the CheckCurrentTsar method

5e91213

Merge pull request #1 from Dary0302/ObjectComparisonTask

321e64c

Task complete

Merge branch 'ObjectComparisonTask'

138ba86

Add tests for NumberValidator creation

25512ae

implemented all tests and add testCase for all tests

a494c2c

Merge pull request #2 from Dary0302/NumberValidatorTask

c25ecda

Number validator task

Buskervil reviewed Nov 5, 2024

View reviewed changes

Dary0302 and others added 2 commits November 6, 2024 00:34

all tasks corrected

383e965

Merge pull request #3 from Dary0302/correction-tasks

dfebc84

all tasks corrected

Buskervil reviewed Nov 6, 2024

View reviewed changes

Dary0302 and others added 2 commits November 6, 2024 16:24

New tests added

4ab39c2

Merge pull request #4 from Dary0302/correction-tasks

b71dc00

New tests added

Buskervil reviewed Nov 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Леонтьев Дмитрий #19

Леонтьев Дмитрий #19

Dary0302 commented Nov 4, 2024

Buskervil left a comment

Buskervil Nov 5, 2024

Buskervil Nov 6, 2024

Buskervil Nov 5, 2024

Buskervil Nov 6, 2024

Buskervil Nov 5, 2024

Buskervil Nov 6, 2024

Buskervil Nov 5, 2024

Buskervil Nov 6, 2024

Buskervil Nov 5, 2024

Buskervil Nov 6, 2024

Buskervil left a comment

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil left a comment

Buskervil Nov 6, 2024

Dary0302 Nov 6, 2024

Buskervil Nov 6, 2024

Buskervil Nov 6, 2024

Dary0302 Nov 6, 2024

Buskervil Nov 6, 2024

Леонтьев Дмитрий #19

Are you sure you want to change the base?

Леонтьев Дмитрий #19

Conversation

Dary0302 commented Nov 4, 2024

Buskervil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Buskervil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Buskervil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment