Шестопалов Андрей #18

Virtical · 2024-11-04T16:12:46Z

No description provided.

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

+        var ex = Assert.Throws<ArgumentException>(action,
+            $"Failed: Expected ArgumentException for precision = {precision}");
+
+        Assert.That(ex.Message, Is.EqualTo("precision must be a positive number"),


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

+    {
+        TestDelegate action = () => new NumberValidator(precision);
+
+        var ex = Assert.Throws<ArgumentException>(action,


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

+    [TestCase(-100)]
+    [TestCase(-9999)]
+    [Category("InitializationTests")]
+    public void Constructor_ScaleIsNegative_ThrowArgumentException(int scale)


shiyois · 2024-11-06T08:37:20Z

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

+    [Category("InitializationTests")]
+    public void Constructor_InvalidArguments_ThrowArgumentException(int precision, int scale, string expectedMessage)
+    {
+        Action action = () => new NumberValidator(precision, scale);


Action -> var. И по смыслу это Func так как возвращает результат.
Предпочтительно использовать ключевое слово var. Явно указывать тип излишне, компилятор умеет определять тип динамически. Так же это позволит сократить код и сделать его более читаемым

shiyois · 2024-11-06T08:38:21Z

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

+    [TestCase(1, 2, "scale must be a non-negative number less or equal than precision", 
+        TestName = "Constructor_PrecisionIsGreaterThanPrecision_ThrowArgumentException (precision = 1, scale = 2)")]
+    [Category("InitializationTests")]
+    public void Constructor_InvalidArguments_ThrowArgumentException(int precision, int scale, string expectedMessage)


Проверять сообщение ошибки в тесте не самая лучшая практика. Тест становиться не устойчивым к законным изменениям в NumberValidator. При каждом изменении сообщения придется править тест

shiyois · 2024-11-06T08:43:47Z

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


 namespace HomeExercise.Tasks.NumberValidator;

 [TestFixture]
 public class NumberValidatorTests
 {
-    [Test]
-    public void Test()
+    #region ConstructorTests


Предлагаю убрать использование #region.
Это вызывает затруднение при навигации, лишние клики
Большинство Ide поддерживают скрытие блоков кода, если в этом есть необходимость.
И на практике использование region указывает на плохо декомпозированный код.
Альтернативой может послужить рефакторинг кода, переиспользуемые методы, вынесение в отдельный или partial класс

shiyois · 2024-11-06T08:54:32Z

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

+    [TestCase(5, 2, "+21",
+        TestName = "IsValidNumber_IntegerWithSign_ScaleIsGreaterThanZero_ReturnsTrue (precision = 5, scale = 2, onlyPositive = false, valueForValidation = \"+37\")")]
+    [Category("IntegersTests")]
+    public void IsValidNumber_IntegerWithinPrecisionAndScale_ReturnTrue(int precision, int scale, string valueForValidation)


Тест дублирует логику теста ниже. За исключением onlyPositive в конструкторе NumberValidator.
Предлагаю передавать значение onlyPositive в TestCase

shiyois · 2024-11-06T08:55:52Z

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

+    [Category("IntegersTests")]
+    public void IsValidNumber_IntegerWithinPrecisionAndScale_ReturnTrue(int precision, int scale, string valueForValidation)
+    {
+        var validator = new NumberValidator(precision, scale, false);


В текущей реализации излишне явно проставлять onlyPositive false. Оно и так определено как false по умолчанию в конструкторе

shiyois · 2024-11-06T09:33:06Z

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

+
+    #region IntegersTests
+    [TestCase(1, 0, "0", 
+        TestName = "IsValidNumber_IntegerWithoutSign_LengthEqualsPrecision_ReturnsTrue (precision = 1, scale = 0, onlyPositive = false, valueForValidation = \"0\")")]


Указывать параметры тест-кейса еще и в нейминге - излишнее дублирование. Это затрудняет читаемость теста и смещает фокус с самого главного - что проверяет тест. Если тест падает, то разработчик пойдет чинить и увидит сигнатуру теста

shiyois · 2024-11-06T09:36:39Z

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

+        var validator = new NumberValidator(precision, scale, false);
+        var isValid = validator.IsValidNumber(valueForValidation);
+
+        isValid.Should().BeTrue();


Дублирование кода метода ниже за исключением ожидаемого результата. Предлагаю передавать ожидаемый результат в параметрах теста

shiyois · 2024-11-06T09:39:02Z

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

+        TestName = "IsValidNumber_InputIsWhitespace_ReturnFalse (precision = 5, scale = 2, onlyPositive = false, valueForValidation = \"    \")")]
+    [TestCase(5, 2, "\t", 
+        TestName = "IsValidNumber_InputIsEscapeSequences_ReturnFalse (precision = 5, scale = 2, onlyPositive = false, valueForValidation = \"\\t\")")]
+


Лишние пробелы между атрибутами

shiyois · 2024-11-06T09:41:10Z

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

+    [TestCase(5, 2, "636.", 
+        TestName = "IsValidNumber_DecimalWithoutFraction_ScaleIsPositive_ReturnsFalse (precision = 5, scale = 2, onlyPositive = false, valueForValidation = \"636.\")")]
+
+    [Category("InvalidInputsTests")]


Расскажешь для чего используется атрибут Category?

Как написано в интернете, через него можно запускать отдельные группы тестов

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

Согласен, в данном случае это не использую.
В первой задаче (ObjectComparison) на тестовом методе был атрибут [Category("ToRefactor")]. Я тоже не нашёл где бы он использовался, но посчитал, что раз он использован в показательном примере первой задачи, то можно добавить этот атрибут и в моих методах.

shiyois · 2024-11-06T09:45:17Z

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

+    [TestCase(1, 2, "scale must be a non-negative number less or equal than precision", 
+        TestName = "Constructor_PrecisionIsGreaterThanPrecision_ThrowArgumentException (precision = 1, scale = 2)")]
+    [Category("InitializationTests")]
+    public void Constructor_InvalidArguments_ThrowArgumentException(int precision, int scale, string expectedMessage)


Предлагаю отрефакторить имена тестов
Метод назвать ThrowException_When, а тесты PrecisionEqualsScale и т.д
Повысим читаемость, уберём дублирование

shiyois · 2024-11-06T09:46:11Z

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

+    [TestCase(5, 5, "scale must be a non-negative number less or equal than precision", 
+        TestName = "Constructor_PrecisionEqualsScale_ThrowArgumentException (precision = 5, scale = 5)")]
+    [TestCase(1, 2, "scale must be a non-negative number less or equal than precision", 
+        TestName = "Constructor_PrecisionIsGreaterThanPrecision_ThrowArgumentException (precision = 1, scale = 2)")]


PrecisionIsGreaterThanPrecision - опечатка?

Virtical added 2 commits November 4, 2024 14:27

ObjectComparison task has been solved

8a9c996

NumberValidator task has been solved

f62673c

shiyois reviewed Nov 5, 2024

View reviewed changes

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

{

TestDelegate action = () => new NumberValidator(precision);

var ex = Assert.Throws<ArgumentException>(action,

This comment was marked as resolved.

Sign in to view

shiyois reviewed Nov 5, 2024

View reviewed changes

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

[TestCase(-100)]

[TestCase(-9999)]

[Category("InitializationTests")]

public void Constructor_ScaleIsNegative_ThrowArgumentException(int scale)

This comment was marked as resolved.

Sign in to view

Corrections after mentor review

47755b4

shiyois reviewed Nov 6, 2024

View reviewed changes

Virtical added 2 commits November 6, 2024 15:19

Corrections after mentor review №2

7877e6d

Merge branch 'kontur-courses:master' into master

b22b0ac

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Шестопалов Андрей #18

Шестопалов Андрей #18

Virtical commented Nov 4, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

Virtical Nov 6, 2024

shiyois Nov 6, 2024

Virtical Nov 6, 2024

shiyois Nov 6, 2024

shiyois Nov 6, 2024

Virtical Nov 6, 2024

Шестопалов Андрей #18

Are you sure you want to change the base?

Шестопалов Андрей #18

Conversation

Virtical commented Nov 4, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

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

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