Брозовский Максим #12

BMV989 · 2024-11-04T04:55:07Z

No description provided.

HELLoWorlD01100

У нас все тесты(кроме тестов на конструктор) можно разделить на два вида благодаря тому, что у нас возвращается bool:

С валидным значением
С невалидным значением

Мб так и поступим, вместо дублирования?

HELLoWorlD01100 · 2024-11-04T15:07:57Z

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

@@ -36,6 +30,9 @@ public void CheckCurrentTsar_WithCustomEquality()

        // Какие недостатки у такого подхода? 
        ClassicAssert.True(AreEqual(actualTsar, expectedTsar));
+        // Нужно писать сам предикат AreEqual (лишний код в котором можно ошибиться)
+        // + его надо поддерживать (при расширении класса Person нужно менять логику AreEqual)
+        // В AreEqual нет Assert, значит при падении теста не будет информации о том как упал тест (причина падения)


Рассмотри еще кейс, когда у царя поле parent будет указывать на уже встречающегося царя в родословной. Нам же ничего не мешает так сделать :)

Случай несколько вырожден, но формально возможен.
В таком случае возникнет бесконечная рекурсия.
Рассмотрим простой случай: есть actualTsar и пусть actualTsar.Parent.Parent = actualTsar.
Тогда при сравнении AreEqual(actualTsar, actualTsar) мы будем сравнивать их Parent, но у них тоже есть Parent, а значит их мы тоже будем сравнивать и т. д.
Таким образом мы получим бесконечную последовательность вызовов AreEqual для каждого Parent.

А почему вырожден?
Ты не смотри на это как в жизни реально есть, у тебя написан код, который такое допускает.
И т.к мы не можем менять с тобой сурсы, давай обложимся на уровне тестов хотя бы

Окей согласен

HELLoWorlD01100 · 2024-11-04T15:17:26Z

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


 namespace HomeExercise.Tasks.NumberValidator;

 [TestFixture]
 public class NumberValidatorTests
 {
    [Test]
-    public void Test()
+    public void NumberValidator_ThrowsArgumentException_WhenPrecisionIsNegative()


Проверяется единственный набор аргументов.

Тесты тебе нужны, чтобы при изменении кода отловить какое-то нарушение поведение. Хочется, чтобы тест отражал все варианты.

HELLoWorlD01100 · 2024-11-04T15:20:10Z

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

    }
+
+    [Test]
+    public void NumberValidator_ThrowsArgumentException_WhenScaleLessThanZero()


То же самое, что выше

HELLoWorlD01100 · 2024-11-04T15:20:55Z

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

    }
+
+    [Test]
+    public void NumberValidator_ThrowsArgumentException_WhenScaleLessThanZero()


Нет проверки на пограничные кейсы

HELLoWorlD01100 · 2024-11-04T15:22:58Z

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

+    }
+
+    [Test]
+    public void NumberValidator_ThrowsArgumentException_WhenScaleGreaterOrEqualThanPrecision()


Один тест содержит сразу два теста. Давай разделим

HELLoWorlD01100 · 2024-11-04T15:24:00Z

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

+    }
+
+    [Test]
+    public void NumberValidator_DoesNotThrow_WhenScaleLessThanPrecision()


А если работает все только когда разница между ними 1?

HELLoWorlD01100 · 2024-11-04T15:30:10Z

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

+    [TestCase("-99", 3, false)]
+    [TestCase("9999999", 7, true)]
+    [TestCase("-999999999", 10, false)] 
+    public void IsValidNumber_ShouldBeTrue_WhenValueIsInteger(string value, int precision, bool onlyPositive) => 


А если Int переполнен?

А если scale тоже позадавать?

А если с нуля начинается?
В общем еще много тестов можно написать :)

Арабские цифры мы проверили, а другие будем проверять?

HELLoWorlD01100 · 2024-11-04T15:39:03Z

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

+    [TestCase("-300", 3, false)]
+    [TestCase("300", 2, false)]
+    [TestCase("-1234567890", 10, false)]
+    public void IsValidNumber_ShouldBeFalse_WhenValueIsInteger(string value, int precision, bool onlyPositive) =>


Условие(WhenValueInInteger) должно отражать почему тест работает.
Если я открою тесты и посмотрю почему они упали, я увижу такую картину:

IsValidNumber_ShouldBeTrue_WhenValueIsInteger

IsValidNumber_ShouldBeFalse_WhenValueIsInteger

Условие одно, результаты разные. Придется лезть явно в код и читать почему тесты падают, из описания я мало что пойму

HELLoWorlD01100 · 2024-11-04T15:45:05Z

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

+    [TestCase("-1234567890.1234567890", 21, 10, false)]
+    [TestCase("0.0", 2, 1,false)]
+    [TestCase("300.52", 5, 2, true)]
+    public void IsValidNumber_ShouldBeTrue_WhenValueIsFloatNumber(string value, int precision, int scale, 


А еще запятую можно ставить вместо точки

HELLoWorlD01100 · 2024-11-04T15:47:27Z

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

+    [TestCase("+300.52")]
+    [TestCase("-999.99")]
+    [TestCase("-101")]
+    public void IsValidNumber_ShouldBeTrue_WhenValueHasSign(string value) => TestShouldBeTrue(value);


А мы же отрицательные проверили в кейсаъ IsValidNumber_ShouldBeTrue_WhenValueIsInteger

-- added some edge cases for constructor -- rework test schema for IsValidNumber -- added more testcases

HELLoWorlD01100 · 2024-11-06T01:08:51Z

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

@@ -36,6 +30,9 @@ public void CheckCurrentTsar_WithCustomEquality()

        // Какие недостатки у такого подхода? 
        ClassicAssert.True(AreEqual(actualTsar, expectedTsar));
+        // Нужно писать сам предикат AreEqual (лишний код в котором можно ошибиться)
+        // + его надо поддерживать (при расширении класса Person нужно менять логику AreEqual)
+        // В AreEqual нет Assert, значит при падении теста не будет информации о том как упал тест (причина падения)


А почему вырожден?
Ты не смотри на это как в жизни реально есть, у тебя написан код, который такое допускает.
И т.к мы не можем менять с тобой сурсы, давай обложимся на уровне тестов хотя бы

HELLoWorlD01100 · 2024-11-06T01:10:02Z

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

+    [TestCase(-1, 0, PrecisionErrorMessage, Description = "precision < 0")]
+    [TestCase(-10, 1, PrecisionErrorMessage, Description = "precision < 0")]
+    [TestCase(0, 0, PrecisionErrorMessage, Description = "precision == 0")]
+    [TestCase(0, 2, PrecisionErrorMessage, Description = "precision == 0")]
+    [TestCase(5, -1, ScaleErrorMessage, Description = "scale < 0")]
+    [TestCase(5, -6, ScaleErrorMessage, Description = "scale < 0")]
+    [TestCase(1, 2, ScaleErrorMessage, Description = "scale > precision")]
+    [TestCase(1, 10, ScaleErrorMessage, Description = "scale > precision")]
+    [TestCase(1, 1, ScaleErrorMessage, Description = "scale == precision")]
+    [TestCase(2, 2, ScaleErrorMessage, Description = "scale == precision")]


Там прям TestName вроде есть

HELLoWorlD01100 · 2024-11-06T01:11:59Z

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

+    [TestCase(17, 10)]
+    [TestCase(3, 2)]
+    [TestCase(1, 0)]
+    [TestCase(6, 5)]
+    [TestCase(300, 52)]
+    public void NumberValidator_DoesNotThrow_WithValidParams(int precision, int scale)
+    {
+        Action act = () => new NumberValidator(precision, scale);
+        act.Should().NotThrow();


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

HELLoWorlD01100 · 2024-11-06T01:15:36Z

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

+    [TestCase("", 17, 10, false)]
+    [TestCase(null, 17, 10, false)]
+    [TestCase("II", 17, 10, false)]
+    [TestCase("III", 17, 10, false)]
+    [TestCase("VI", 17, 10, false)]
+    [TestCase("X", 17, 10, false)]
+    [TestCase("seven.eleven", 17, 10, false)]
+    [TestCase("five/two", 17, 10, false)]
+    [TestCase("11.11.11", 17, 10, false)]
+    [TestCase("11.22,33", 17, 10, false)]
+    [TestCase("55,22,33", 17, 10, false)]
+    [TestCase("1\\2", 17, 10, false)]
+    [TestCase("-100", 4, 0, true)]
+    [TestCase("-0.00", 3, 2, true)]
+    [TestCase("+1,23", 3, 2, true)]
+    [TestCase("1.123", 4, 2, true)]
+    [TestCase("0.000", 5, 2, true)]
+    [TestCase("a.sd", 17, 10, false)]
+    [TestCase("52", 1, 0, true)]
+    [TestCase("-300", 3, 0, false)]
+    [TestCase("300", 2, 0, false)]
+    [TestCase("-1234567890", 10, 0, false)]
+    [TestCase("-1.23", 3, 2, false)]
+    [TestCase("-1234567890.1234567890", 20, 10, false)]
+    [TestCase("100", 2, 1, true)]
+    [TestCase("X,Y", 3, 2, true)]
+    [TestCase("1\n.5", 3, 2, true)]
+    [TestCase("+-15", 3, 2, false)]
+    [TestCase("300_000", 17, 10, false)]
+    [TestCase("52 000", 17, 10, false)]
+    [TestCase("1/2", 17, 10, false)]
+    [TestCase("12 . 22", 17, 10, false)]
+    [TestCase("12 , 22", 17, 10, false)]


Давай докинем информации о тесте? Если что-то упадет, то придется аргументы разбирать

Например, кейс [TestCase("-300", 4, 0, false)] из другого тестового метода будет сложно понять по аргументам, придется лезть в реализацию и специфику.

HELLoWorlD01100 · 2024-11-06T01:30:06Z

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

+    [TestCase("+1.23", 17, 10, false)]
+    [TestCase("+3.52", 15, 10, false)]


А чем эти тесты отличаются?

HELLoWorlD01100 · 2024-11-06T01:33:48Z

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

+    [TestCase("100", 2, 1, true)]
+    [TestCase("X,Y", 3, 2, true)]
+    [TestCase("1\n.5", 3, 2, true)]
+    [TestCase("+-15", 3, 2, false)]


А если два одинаковых знака?

HELLoWorlD01100 · 2024-11-06T01:37:00Z

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

+                .Excluding(p => p.Id)
+                .Excluding(p => p.Parent.Id));


А p.Parent.Parent.Id мы не будем учитывать? :)

Мы в другом треде говорили про бесконечную рекурсию. А тут какое поведение будет?

В моем старом решение нет, потому что там если будет Parent.Parent, то их Id могут не совпасть и тогда мы упадем с ошибкой что Id не одинаковы (поэтому Id надо исключать у всех), но если их Id и Parent.Id совпадут, то да, будет бесконечная рекурсия

Я писал тест исходя из actualTsar и expectedTsar, поэтому я не стал брать рекуретное обобщение

Но так действительно лучше

но оно не поможет с рекурсией(

оно и не должно, впрочем

HELLoWorlD01100 · 2024-11-06T01:39:31Z

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

+    [TestCase("II", 17, 10, false)]
+    [TestCase("III", 17, 10, false)]
+    [TestCase("VI", 17, 10, false)]
+    [TestCase("X", 17, 10, false)]


Кроме римских есть еще различные цифры, и числа из них, на удивление, будут валидными :)

HELLoWorlD01100 · 2024-11-06T01:39:48Z

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

+    [TestCase("-1234567890.1234567890", 20, 10, false)]
+    [TestCase("100", 2, 1, true)]
+    [TestCase("X,Y", 3, 2, true)]
+    [TestCase("1\n.5", 3, 2, true)]


А если неэкранируемый на конце? или в начале?

Лучше похожие кейсы рядом писать, чтобы контекст постоянно не переключать(например,100 и 300)

HELLoWorlD01100 · 2024-11-06T01:42:32Z

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

+    [TestCase("55,22,33", 17, 10, false)]
+    [TestCase("1\\2", 17, 10, false)]
+    [TestCase("-100", 4, 0, true)]
+    [TestCase("-0.00", 3, 2, true)]


Интересный же кейс) Почему положительного кейса нету такого?

HELLoWorlD01100 · 2024-11-06T22:59:14Z

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

+    [TestCase("2", 1, 0, true, TestName = "2 should be valid number")]
+    [TestCase("52", 2, 1, true, TestName = "52 should be valid number")]
+    [TestCase("-300", 4, 0, false, TestName = "-300 should be valid number")]
+    [TestCase("+300", 4, 0, true, TestName = "+300 should be valid number")]
+    [TestCase("-99", 3, 2, false, TestName = "-99 should be valid number")]
+    [TestCase("9999999", 7, 1, false, TestName = "9999999 should be valid number")]
+    [TestCase("2147483649", 10, 1, false, TestName = "Int overflow should be valid number")]
+    [TestCase("-1234567890", 11, 8, false, TestName = "-1234567890 should be valid number")]
+    [TestCase("0101010", 20, 12, false, TestName = "0101010 should be valid number")]
+    [TestCase("01110.01010", 10, 5, true, TestName = "01110.01010 should be valid number")]
+    [TestCase("01110,01010", 10, 5, true, TestName = "01110,01010 should be valid number")]
+    [TestCase("+01110,01010", 11, 5, true, TestName = "+01110,01010 should be valid number")]
+    [TestCase("-1234567890.1234567890", 21, 10, false, TestName = "-1234567890.1234567890 should be valid number")]
+    [TestCase("+0.0", 3, 1, false, TestName = "+0.0 should be valid number")]
+    [TestCase("0.000", 4, 3, true, TestName = "0.000 should be valid number")]
+    [TestCase("300.52", 5, 2, true, TestName = "300.52 should be valid number")]
+    [TestCase("+3.52", 15, 10, false, TestName = "+3.52 should be valid number")]
+    [TestCase("+300,52", 17, 10, false, TestName = "+300,52 should be valid number")]
+    [TestCase("1,23", 17, 10, false, TestName = "1,23 should be valid number")]
+    [TestCase("+100", 4, 1, true, TestName = "+100 should be valid number")]
+    [TestCase("100\n", 4, 2, true, TestName = "100\n should be valid number")]


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

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

В случае с 100\n у меня вообще кажется в TestName'е при запуске тестов по итогу будет просто 100?) И как понять что было не так?

Это можно было бы назвать типа: When \n at the end of a number

HELLoWorlD01100 · 2024-11-06T23:00:19Z

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

+    [TestCase("", 17, 10, false, TestName = "empty string should be invalid number")]
+    [TestCase(null, 17, 10, false, TestName = "null should be invalid number")]
+    [TestCase("II", 17, 10, false, TestName = "II should be invalid number")]
+    [TestCase("III", 17, 10, false, TestName = "III should be invalid number")]
+    [TestCase("VI", 17, 10, false, TestName = "VI should be invalid number")]
+    [TestCase("X", 17, 10, false, TestName = "X should be invalid number")]
+    [TestCase("ఎనమద", 4, 0 , true, TestName = "ఎనమద should be invalid number")]
+    [TestCase("తమమద", 4, 0 , true, TestName = "తమమద should be invalid number")]
+    [TestCase("0xFF", 4, 0, true, TestName = "0xFF should be invalid number")]
+    [TestCase("0x1A", 4, 0, true, TestName = "0x1A should be invalid number")]
+    [TestCase("0o75", 4, 0, true, TestName = "0o75 should be invalid number")]
+    [TestCase("0o44", 4, 0, true, TestName = "0o44 should be invalid number")]
+    [TestCase("0b1010", 6, 0, true, TestName = "0b1010 should be invalid number")]
+    [TestCase("0b01110.01010", 10, 5, true, TestName = "0b01110.01010 should be invalid number")]
+    [TestCase("seven.eleven", 17, 10, false, TestName = "seven.eleven should be invalid number")]
+    [TestCase("five/two", 17, 10, false, TestName = "five/two should be invalid number")]
+    [TestCase("11.11.11", 17, 10, false, TestName = "11.11.11 should be invalid number")]
+    [TestCase("+127.000.001", 17, 10, false, TestName = "+127.000.001 should be invalid number")]
+    [TestCase("11.22,33", 17, 10, false, TestName = "11.22.33 should be invalid number")]
+    [TestCase("55,22,33", 17, 10, false, TestName = "55.22.33 should be invalid number")]
+    [TestCase("+1\\2", 17, 10, false, TestName = "+1\\2 should be invalid number")]
+    [TestCase("X,Y", 3, 2, true, TestName = "X,Y should be invalid number")]
+    [TestCase("1\n.5", 3, 2, true, TestName = "1\n.5 should be invalid number")]
+    [TestCase("\n2.4", 3, 2, true, TestName = "\n2.4 should be invalid number")]
+    [TestCase("+-15", 3, 2, false, TestName = "+-15 should be invalid number")]
+    [TestCase("--10", 4, 2, false, TestName = "--10 should be invalid number")]
+    [TestCase("++1", 3, 0, true, TestName = "++1 should be invalid number")]
+    [TestCase("300_000", 17, 10, false, TestName = "300_000 should be invalid number")]
+    [TestCase("52 000", 17, 10, false, TestName = "52 000 should be invalid number")]
+    [TestCase("1/2", 17, 10, false, TestName = "1/2 should be invalid number")]
+    [TestCase("12 . 22", 17, 10, false, TestName = "12 . 22 should be invalid number")]
+    [TestCase("12 , 22", 17, 10, false, TestName = "12 , 22 should be invalid number")]
+    [TestCase("a.sd", 17, 10, false, TestName = "a.sd should be invalid number")]
+    [TestCase("-100", 4, 0, true, TestName = "-100 should be invalid number because onlyPositive is true")]
+    [TestCase("+1,23", 3, 2, true, TestName = "+1.23 should be invalid number because precision < 4")]
+    [TestCase("1.123", 4, 2, true, TestName = "1.123 should be invalid number because scale < 3")]
+    [TestCase("0.00", 5, 1, true, TestName = "0.000 should be invalid number because scale < 2")]
+    [TestCase("52", 1, 0, true, TestName = "52 should be invalid number because precision < 2")]
+    [TestCase("-300", 3, 0, false, TestName = "-300 should be invalid number because precision < 4")]
+    [TestCase("300", 2, 0, false, TestName = "300 should be invalid number because precision < 3")] 
+    [TestCase("+100", 3, 0, false, TestName = "+100 should be invalid number because precision < 4")] 
+    [TestCase("-1.23", 3, 2, false, TestName = "-1.23 should be invalid number because precision < 4")]
+    [TestCase("-1234567890.1234567890", 20, 10, false, TestName = "-1234567890.1234567890 should be invalid number because precision < 21")]


Тут тоже: мне недостаточно просто знать аргументов, мне важно знать какой сценарий покрывает тест

Homework is done

1c6f559

HELLoWorlD01100 suggested changes Nov 4, 2024

View reviewed changes

Refactor: NumberValidatorTests

e0e6f80

-- added some edge cases for constructor -- rework test schema for IsValidNumber -- added more testcases

BMV989 requested a review from HELLoWorlD01100 November 5, 2024 06:44

BMV989 added 2 commits November 5, 2024 13:26

Remove duplicate tests

232b0cb

small change in one test

3440c65

HELLoWorlD01100 suggested changes Nov 6, 2024

View reviewed changes

BMV989 added 3 commits November 6, 2024 10:24

dealing with exclude all persons' ids

bb6c13c

another refactoring of NumberValidatorTests

d942723

small mistake in case's TestName

f85a528

BMV989 requested a review from HELLoWorlD01100 November 6, 2024 07:39

BMV989 added 2 commits November 6, 2024 17:40

small refactoring of NumberValidatorTests

0b113c5

another small mistake in one of testcases

ff9fdc5

HELLoWorlD01100 suggested changes Nov 6, 2024

View reviewed changes

BMV989 added 4 commits November 7, 2024 07:57

Fixing testcases' naming

46d831d

small mistake in testcase

cdf2d70

another pack of mistakes have been fixed

ef2b03f

add some remark in one of the tests

4f1f011

BMV989 requested a review from HELLoWorlD01100 November 7, 2024 03:08

BMV989 added 3 commits November 7, 2024 08:13

yet another pack of mistakes

537ba36

small mistake

8f4d1f4

still some mistakes

4d634a9

		[TestCase("+1.23", 17, 10, false)]
		[TestCase("+3.52", 15, 10, false)]

Брозовский Максим #12

Are you sure you want to change the base?

Брозовский Максим #12

Conversation

BMV989 commented Nov 4, 2024

HELLoWorlD01100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BMV989 Nov 5, 2024 • edited Loading

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

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

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

BMV989 Nov 6, 2024 • edited Loading

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

BMV989 Nov 5, 2024 •

edited

Loading

BMV989 Nov 6, 2024 •

edited

Loading