Глейзер Роман by RomanGleyzer · Pull Request #39 · kontur-courses/shpora-testing

RomanGleyzer · 2025-10-26T16:26:55Z

masssha1308 · 2025-10-27T09:12:53Z

+
 public class ObjectComparison
 {
    [Test]


Неинформативное название и описание. Не ясно что проверяется. Давай поправим

masssha1308 · 2025-10-27T09:21:35Z

@@ -14,16 +16,14 @@ public void CheckCurrentTsar()
        var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,


Давай явно укажем что значат эти параметры через named arguments, чтобы не приходилось проваливаться в конструктор Person чтобы разобраться что есть что

masssha1308 · 2025-10-27T09:24:21Z

-        ClassicAssert.AreEqual(expectedTsar.Parent.Age, actualTsar.Parent.Age);
-        ClassicAssert.AreEqual(expectedTsar.Parent.Height, actualTsar.Parent.Height);
-        ClassicAssert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent);
+        actualTsar.Should()


Давай поправим форматирование Fluent Assertions. Либо в одну строку, либо с правильными переносами для цепочки вызовов

masssha1308 · 2025-10-27T09:39:47Z

+        // Какие недостатки у такого подхода?
+        // Недостатки подхода:
+        // 1) Требуется ручное обновление метода сравнения при добавлении новых свойств в класс Person,
+        // 2) Тест менее информативен при падении. Он просто укажет, что объекты не равны, не показывая какие именно свойства не совпали.


Могут ли быть проблемы с рекурсией?

Могут ли быть проблемы с рекурсией?

Да, попробовал сделать так, чтобы у обоих сравниваемых объектов родитель указывал на самого себя. В итоге метод сравнения убежал в бесконечную рекурсию:

masssha1308 · 2025-10-27T12:19:58Z

-        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"));
+    [TestCase(-1, 2, true, TestName = "Исключение, когда precision отрицательный")]


Много где testName избыточен, т.к. очевиден из переданных аргументов

masssha1308 · 2025-10-27T12:22:31Z

+    [TestCase(5, -1, true, TestName = "Исключение, когда scale отрицательный")]
+    [TestCase(1, 2, true, TestName = "Исключение, когда scale больше precision")]
+    [TestCase(1, 1, true, TestName = "Исключение исключение, когда scale равен precision")]
+    public void Ctor_Should_Throw_On_Invalid_Arguments(int precision, int scale, bool onlyPositive)


Лучше разбить на отдельные тесты по категориям (InvalidPrecision, InvalidScale и т.д.) для улучшения читабельности

masssha1308 · 2025-10-27T12:26:26Z

+    [TestCase(5, -1, true, TestName = "Исключение, когда scale отрицательный")]
+    [TestCase(1, 2, true, TestName = "Исключение, когда scale больше precision")]
+    [TestCase(1, 1, true, TestName = "Исключение исключение, когда scale равен precision")]
+    public void Ctor_Should_Throw_On_Invalid_Arguments(int precision, int scale, bool onlyPositive)


В нейминге лучше придерживаться шаблона Method_Scenario_ExpectedResult

masssha1308 · 2025-10-27T12:27:22Z

+    public void Ctor_Should_Throw_On_Invalid_Arguments(int precision, int scale, bool onlyPositive)
+    {
+        var act = () => new NumberValidator(precision, scale, onlyPositive);
+        act.Should().Throw<ArgumentException>();


Можно еще текст ошибки уточнить

masssha1308 · 2025-10-27T12:34:09Z

+    }
+
+    [TestCase(1, 0, true, TestName = "Не выбрасывает исключение на корректных элемент")]
+    public void Ctor_Should_Not_Throw_On_Valid_Arguments(int precision, int scale, bool onlyPositive)


можно переписать без testCase, т.к. он один

masssha1308 · 2025-10-27T12:36:00Z

+
+    [TestCase("", 17, 2, true, TestName = "Возвращает false, когда значение пустое")]
+    [TestCase(null, 17, 2, true, TestName = "Возвращает false, когда значение null")]
+    public void Should_Return_False_On_NullOrEmpty(string value, int precision, int scale, bool onlyPositive)


Лучше указывать имя метода в нейминге теста, т.к. в классе может быть несколько методов

masssha1308 · 2025-10-27T12:39:15Z

+    [TestCase("4_4", 11, 3, true, TestName = "Возвращает false, когда число содержит символ подчёркивания")]
+    [TestCase("abcd_4", 10, 5, true, TestName = "Возвращает false, когда строка содержит буквы и подчёркивание")]
+    [TestCase("--1", 12, 2, true, TestName = "Возвращает false, когда число содержит двойной минус")]
+    public void Should_Return_False_On_InvalidFormat(string value, int precision, int scale, bool onlyPositive)


Тоже лучше разбить по категориям

masssha1308 · 2025-10-27T12:45:55Z

+    public void IsValid_Respects_Scale(string input, int precision, int scale,
+        bool onlyPositive, bool expected)
+    {
+        var validator = new NumberValidator(10, scale, onlyPositive);


магическое число

masssha1308 · 2025-10-27T12:49:33Z

+    [TestCase(0, 2, true, TestName = "Исключение, когда precision равен нулю")]
+    [TestCase(5, -1, true, TestName = "Исключение, когда scale отрицательный")]
+    [TestCase(1, 2, true, TestName = "Исключение, когда scale больше precision")]
+    [TestCase(1, 1, true, TestName = "Исключение исключение, когда scale равен precision")]


Кое-где в TestName's есть очепятки, проверь, пожалуйста

ObjectComparison стал TsarComparison, добавил named arguments, поправил форматирование Fluent Assertions, выяснил, что могу быть проблемы с рекурсией

Убрал избыточный TestName, поправил опечатки, стал придерживаться нейминга Method_Scenario_ExpectedResult, добавил уточнения текстов ошибок, разбил тесты по категориям и избавился от магического числа

masssha1308 · 2025-10-30T05:45:20Z

+    #region Некорректная дробность
+
+    [TestCase(5, -1, true)]
+    public void Ctor_InvalidScale_Negative_ThrowsArgumentException(int precision,  int scale, bool onlyPositive)


Если TestCase один, то лучше не писать через TestCase, а написать просто тест

masssha1308 · 2025-10-30T05:47:47Z

+public class TsarComparison
 {
    [Test]
    [Description("Проверка текущего царя")]


категорию теста после рефакторинга можно убрать

RomanGleyzer added 2 commits October 26, 2025 10:59

Сравнение объектов

944cfb6

Рефакторинг тестов

f530fb8

masssha1308 reviewed Oct 27, 2025

View reviewed changes

RomanGleyzer added 2 commits October 27, 2025 19:49

Исправления в сравнении царей

e619528

ObjectComparison стал TsarComparison, добавил named arguments, поправил форматирование Fluent Assertions, выяснил, что могу быть проблемы с рекурсией

masssha1308 reviewed Oct 30, 2025

View reviewed changes

		@@ -14,16 +16,14 @@ public void CheckCurrentTsar()
		var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,

Uh oh!

Conversation

RomanGleyzer commented Oct 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masssha1308 Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

masssha1308 Oct 27, 2025 •

edited

Loading