Зубков Артём ДЗ №1 by artem7841 · Pull Request #55 · kontur-courses/shpora-testing

artem7841 · 2025-10-29T05:19:03Z

No description provided.

Yrwlcm

В целом сценарии для тестов написаны неплохо, только их надо прибрать и покрасивее написать.

Также нужно убрать все лишние файлы, которые закоммитил. И сейчас придираться не буду, т.к. первая домашка, но в будущих заданиях нужно стоит делить коммиты по смыслу и давать им говорящее название по содержанию.

По идее по регламенту если история коммитов плохая (непонятные названия, или в одном коммите все сделано), то домашка не проверяется, пока не поправишь

Yrwlcm · 2025-10-29T14:58:17Z

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

+        actualTsar.Should().BeEquivalentTo(expectedTsar, options => 
+            options
+                .Excluding(p => p.Id) 
+                .Excluding(p => p.Parent.Id)
+        );


В целом решение правильное. Но что если цари будут отличаться внуками, или правнуками и т.д? Всех через .Parent.Parent.Parent... не переберёшь) С этим может помочь другая перегрузка метода Excluding, которая принимает IMemberInfo и шарповый оператор nameof

Yrwlcm · 2025-10-29T15:02:59Z

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

+        // Тест с FluentAssertions имеет следующие приемущества: 
+        // 1) Автоматически расширяется при добавлении новых свойств в класс Person
+        // 2) Лучшая читаемость кода
+        // 3) При  ошибке видно сразу каке свойтво не совпало, например если изменить возраст в expectedTsar вывод будет содержать "Expected field actualTsar.Age to be 53, but found 54." 
+        // а при Assert "Assert.That(actual, Is.EqualTo(expected)) Expected: 54 But was:  53" - не понятно какое именно свойтво провалило тест 


Тут все верно, но у кастмного решения есть еще проблема с рекурсией, там она потенциально бесконечная. У FluentAssertions есть ограничение по глубине рекурсии

Yrwlcm · 2025-10-29T15:03:55Z

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

        Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, true));
        Assert.DoesNotThrow(() => new NumberValidator(1, 0, true));
        Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, false));
-        Assert.DoesNotThrow(() => new NumberValidator(1, 0, true));
+        Assert.DoesNotThrow(() => new NumberValidator(1, 0, true)); // Дублирует 2 проверку

-        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
+        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0")); // Дублирует 3 проверку
        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0"));
        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("00.00"));
        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("-0.00"));
-        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
+        ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0")); // Дублирует 3 проверку
        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("+0.00"));
        ClassicAssert.IsTrue(new NumberValidator(4, 2, true).IsValidNumber("+1.23"));
        ClassicAssert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("+1.23"));
        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"));


Старые тесты стоит удалить, мы же их в новые переделали

Yrwlcm · 2025-10-29T15:08:37Z

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

+// Было изменено:
+// 1) Тесты были разделены на логические группы для лучшей читаемости
+// 2) Были добалены сообщения к каждому тесту чтобы из стек-трейса было понятно на каких данных тест не работает
+// 3) Использован Assert.Multiple() для того чтобы одна упавшая проверка не блокировала прохождение остальных проверок
+// 4) Удалены дублирующиеся тесты
+// 5) Добавлены тесты проверяющие: пограничные значения на scale и precision, параметр onlyPositive=false, пустую строку и null, запятую в качетве разделителя,
+// некоректные форматы чисел (2 точки; число начинается с точки; без дробной части, но с точкой), отрицательный ноль.


Спасибо, что подсветил по какой логике делал, но целом такие комменты, даже в домашках, лучше не писать :) Допустимо в случае, когда как в первой части задания явно просили обосновать

Твои изменения видны с помощью гита. А если приходится объяснять свой код комментами, в 90% случаев что-то пошло не так и код надо отрефакторить

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

Yrwlcm · 2025-10-29T15:13:26Z

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

+public class NumberValidatorTestsFix
+{
+    [Test]
+    public void КонструкторКорректноВалидируетПараметры()


Писать названия в коде не на английском - плохой тон, есть переводчики и нейросети, если не получается придумать как назвать. И еще у вас в лекции (в презентации) есть про конвенты именования тестов их тоже нужно посмотреть и поправить все названия тестов

Yrwlcm · 2025-10-29T15:24:33Z

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

+    {
+        var validator = new NumberValidator(17, 2, true);
+
+        Assert.Multiple(() =>


Все Assert.Multiple стоит заменить на [TestCase]

Yrwlcm · 2025-10-29T15:29:02Z

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

+            Assert.That(strictValidator.IsValidNumber("0.12"), Is.True, "Граница scale");
+
+            // Превышение лимитов
+            Assert.That(strictValidator.IsValidNumber("12.34"), Is.False, "Превышение precision");


Тест который проверяет превышение precision уже был, этот нужно удалить

Yrwlcm · 2025-10-29T15:31:46Z

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

+    }
+
+    [Test]
+    public void IsValidNumberКорректноРаботаетСРежимомOnlyPositive()


Очень странная группировка тестов, название говорит, что СРежимомOnlyPositive а внутри 3 из 4 тестов anyNum 🤔

Yrwlcm · 2025-10-29T15:33:41Z

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

+            Assert.That(validator.IsValidNumber("12..34"), Is.False, "Две точки");
+            Assert.That(validator.IsValidNumber(".123"), Is.False, "Начинается с точки");
+            Assert.That(validator.IsValidNumber("123."), Is.False, "Оканчивается точкой");


// 5) Добавлены тесты проверяющие: пограничные значения на scale и precision, параметр onlyPositive=false, пустую строку и null, запятую в качетве разделителя,

Где-то потерялся тест на запятую)

Yrwlcm · 2025-10-29T15:40:05Z

Testing/Testing.sln.DotSettings.user

Вот это точно лишний файл, который не стоит коммитить, как и все файлы из .idea/

…нтацией

Yrwlcm

Сейчас ты тоже ударился в другую крайность, коммит почти на каждое изменение. У нас тут 2 файлика, а уже 12 коммитов, так тоже делать не стоит)

С опытом придет понимание/привычка как это правильней разбивать. Например в этой задачке с моей точки зрения идеально можно было разбить на коммиты так:

Перевел CheckCurrentTsar на FluentAssertions
Прибрался в тестах NumberValidatorTests
Правки после ревью (либо разделить его на два коммита, правки CheckCurrentTsar и правки NumberValidatorTests)

Yrwlcm · 2025-10-30T08:53:05Z

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

+        actualTsar.Should().BeEquivalentTo(expectedTsar, options => 
+            options
+                .Excluding(p => p.Id) 
+                .Excluding((IMemberInfo p) => 
+                    p.Name == "Id" && 
+                    p.Path.Contains("Parent"))


Что-то ты перемудрил) Нам же достаточно через IMemberInfo исключить поле Id по названию, а будет оно в изначальном Person или во вложенном, уже не важно.

В итоге у нас будет один Excluding и без проверки на Parent в пути. И еще воспользуйся nameof, так зашивать строчки, тоже не очень хорошо)

Yrwlcm · 2025-10-30T08:53:59Z

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

+
 [TestFixture]
-public class NumberValidatorTests
+public class NumberValidatorTestsFix


Fix точно лишнее в названии, у нас ведь это всё те же тесты

Yrwlcm · 2025-10-30T08:55:14Z

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

+    [TestCase(-1, 2)]
+    [TestCase(5, -1)]
+    [TestCase(5, 6)]


Тут не хватает названия для теста, почему именно эти параметры должны выкидывать исключение, также как в других тестах, Отрицательная точность и т.п.

Yrwlcm · 2025-10-30T08:58:56Z

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

+    [TestCase("123.4", "Превышение precision в целой части")]
+    public void IsValidNumber_WithExceededPrecisionAndScale_ReturnsFalse(string number, string description)
+    {
+        var strictValidator = new NumberValidator(3, 2);
+
+        strictValidator.IsValidNumber(number).Should().BeFalse(description);
+    }


У нас уже есть похожие тесты в группе IsValidNumber_WithInvalidNumberFormats_ReturnsFalse перенеси этот туда

Yrwlcm · 2025-10-30T08:59:20Z

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

+    public void IsValidNumber_WithOnlyPositiveTrue_RejectsNegativeNumbers()
+    {
+        var positiveOnly  = new NumberValidator(5, 2, true);
+
+        positiveOnly.IsValidNumber("-1.23").Should().BeFalse("Только положительные: отрицательное число");
+    }


Такой тест уже есть

Yrwlcm · 2025-10-30T09:01:41Z

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

+        anyNum.IsValidNumber("+1.23").Should().BeTrue("Положительное число с плюсом");
+        anyNum.IsValidNumber("1.23").Should().BeTrue("Положительное число без плюсом");


Эти два сценария уже тестируются в IsValidNumber_WithValidNumberFormats_ReturnsTrue. И перепиши на [TestCase]

Yrwlcm · 2025-10-30T09:03:30Z

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

+    [TestCase("12,34", "Запятая в качестве разделителя")]
+    public void IsValidNumber_WithCommaAsSeparator_ReturnsTrue(string number, string description)
+    {
+        var validator = new NumberValidator(10, 2);
+
+        validator.IsValidNumber(number).Should().BeTrue(description);


У нас же есть группа тестов про валидные форматы IsValidNumber_WithValidNumberFormats_ReturnsTrue, чем этот от них отличается что мы его в отдельную группу вынесли?)

Yrwlcm · 2025-10-30T14:07:34Z

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

+    [TestCase("1.23", "Максимальная precision и scale")]
+    [TestCase("12.3", "Граница precision")]
+    [TestCase("0.12", "Граница scale")]
+    [TestCase("1,23", "Запятая в качестве разделителя")]


Этот тест с запятой все таки должен лежать в группе WithValidNumberFormats потому что по смыслу как раз туда подходит, а не в WithValidPrecisionAndScale

homework done

bd903db

Yrwlcm reviewed Oct 29, 2025

View reviewed changes

artem7841 added 11 commits October 29, 2025 22:27

исправлен тест CheckCurrentTsarFluentAssertions для вложенных Parent

788579a

удалены старые тесты

0d22773

тесты переименованы на английский, и исправлены в соответсвии с презе…

47dae77

…нтацией

удалены лишнее комменты, тесты сгруппированны

81a3eeb

добавлены TestCase, все тесты переписаны на FluentAssertions

f4ce550

удален дублирующий тест для конструктора

3df44ce

удален второй тест на превышение preсision

b939226

тест для режима OnlyPositive разделен на 2

f67c7f2

добавлен тест на запятую

5611bcb

Удаление лишних файлов из репозитория

7a696cb

Удаление sln.DotSettings.user из репозитория

7c94a52

Yrwlcm reviewed Oct 30, 2025

View reviewed changes

artem7841 added 2 commits October 30, 2025 14:33

правки в ObjectComparison

28c71cb

правки в NumberValidatorTests

6996e46

Yrwlcm reviewed Oct 30, 2025

View reviewed changes

		anyNum.IsValidNumber("+1.23").Should().BeTrue("Положительное число с плюсом");
		anyNum.IsValidNumber("1.23").Should().BeTrue("Положительное число без плюсом");

Conversation

artem7841 commented Oct 29, 2025

Uh oh!

Yrwlcm left a comment

Choose a reason for hiding this comment

Uh oh!

Yrwlcm Oct 29, 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!

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!

Yrwlcm left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yrwlcm Oct 29, 2025 •

edited

Loading