Трофимов Никита #239

WoeromProg · 2023-11-28T12:43:27Z

@VladSavitskiy

…в NumberValidator

desolver · 2023-11-28T17:09:34Z

cs/HomeExercises/NumberValidatorTests.cs

 using System.Text.RegularExpressions;
 using FluentAssertions;
 using NUnit.Framework;
+using NUnit.Framework.Internal;

 namespace HomeExercises
 {
 	public class NumberValidatorTests
 	{
 		[Test]


Если есть TestCase, то [Test] писать не нужно, это почти синонимы. Тут и во всех тестах ниже

desolver · 2023-11-28T17:19:04Z

cs/HomeExercises/NumberValidatorTests.cs


 namespace HomeExercises
 {
 	public class NumberValidatorTests
 	{
 		[Test]
-		public void Test()
+		[TestCase(-1, 2, true, TestName = "Creation_ShouldThrowArgumentException_WhenNegativePrecision")]


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

Базовая часть названия уже есть в названии метода, дублировать её просто не имеет смысла.

Почему так? Читая тест кейсы в инспекторе, мы и так понимаем, что находимся внутри метода Creation_ShouldThrowArgumentException, поэтому дублирование только создает лишнюю когнитивную нагрузку. Также при переименовании метода, TestName автоматом не обновится, если это не сделает IDE

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

Поправить нужно тут и во всех тестах ниже

desolver · 2023-11-28T17:20:42Z

cs/HomeExercises/NumberValidatorTests.cs


 namespace HomeExercises
 {
 	public class NumberValidatorTests
 	{
 		[Test]
-		public void Test()
+		[TestCase(-1, 2, true, TestName = "Creation_ShouldThrowArgumentException_WhenNegativePrecision")]
+		[TestCase(0,0,true, TestName = "Creation_ShouldThrowArgumentException_WhenPrecisionIsZero")]


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

desolver · 2023-11-28T17:21:08Z

cs/HomeExercises/NumberValidatorTests.cs

+		[TestCase(0,0,true, TestName = "Creation_ShouldThrowArgumentException_WhenPrecisionIsZero")]
+		[TestCase(3,-1,true, TestName = "Creation_ShouldThrowArgumentException_WhenNegativeScale")]
+		[TestCase(1,2,true, TestName = "Creation_ShouldThrowArgumentException_WhenScaleIsGreaterThanPrecision")]
+		public void Creation_ShouldThrowArgumentException(int precision, int scale, bool onlyPositive)


Для чего в тесте аргумент onlyPositive, если он всегда равен true?

desolver · 2023-11-28T17:23:03Z

cs/HomeExercises/NumberValidatorTests.cs

+		[TestCase(-1, 2, true, TestName = "Creation_ShouldThrowArgumentException_WhenNegativePrecision")]
+		[TestCase(0,0,true, TestName = "Creation_ShouldThrowArgumentException_WhenPrecisionIsZero")]
+		[TestCase(3,-1,true, TestName = "Creation_ShouldThrowArgumentException_WhenNegativeScale")]
+		[TestCase(1,2,true, TestName = "Creation_ShouldThrowArgumentException_WhenScaleIsGreaterThanPrecision")]


Не хватает ещё одного граничного, очень похожего на этот ))

desolver · 2023-11-28T17:41:14Z

cs/HomeExercises/NumberValidatorTests.cs

 using System.Text.RegularExpressions;
 using FluentAssertions;
 using NUnit.Framework;
+using NUnit.Framework.Internal;

 namespace HomeExercises
 {
 	public class NumberValidatorTests


Во всех методах этого класса потерялась часть, означающая "при каком условии выполняется действие", то есть часть, начинающаяся на _When.... В случае с TestCase желательно обобщить все тесткейсы под одно выражение и записать его в название метода, а сами тесткейсы уже будут пояснять детали своими названиями

desolver · 2023-11-28T17:44:55Z

cs/HomeExercises/NumberValidatorTests.cs

+			validator.IsValidNumber(number).Should().BeFalse();
+		}
+		[Test]
+		[TestCase(17, 2, true, "0.0", TestName = "IsValidNumber_ReturnTrueWhenNumberWithFractionalPart")]


Точно ли здесь учтены все случаи корректной работы метода?

desolver · 2023-11-28T17:46:47Z

cs/HomeExercises/NumberValidatorTests.cs

+		public void IsValidNumber_True(int precision, int scale, bool onlyPositive, string number)
+		{
+			NumberValidator validator = new NumberValidator(precision, scale, onlyPositive);
+			validator.IsValidNumber(number).Should().BeTrue();
 		}
 	}



Понимаю, что этот файл создавал не ты, но быть может задумка автора как раз и была в том, чтобы с какими-то конвентами знакомить

В большом продакшене придется работать с тысячами файлов и классов, и всегда удобно, когда каждый класс, интерфейс, enum и тд лежит в своем собственном файле. Давай тут сделаем так же

Аналогично с соседним файлом

desolver · 2023-11-28T17:50:15Z

cs/HomeExercises/ObjectComparison.cs

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


Не могу почему-то написать комментарий парой строк выше)) Напишу тут - понятное ли название у теста?

desolver · 2023-11-28T17:53:03Z

cs/HomeExercises/ObjectComparison.cs

@@ -15,16 +15,9 @@ public void CheckCurrentTsar()
 			var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
 				new Person("Vasili III of Russia", 28, 170, 60, null));


Замени здесь Василия 3, на Василия 2 и прогони тест, ожидаемое ли это поведение?

Переписал тест с одним testCase Перенёс тесты в отдельные файлы Добавил тест в Creation_ShouldThrowArgumentException Убрал параметр OnlyPositive Изменил тест для проверки царя

desolver · 2023-11-30T12:13:06Z

cs/HomeExercises/NumberValidatorTests.cs

-			validator.IsValidNumber(number).Should().BeTrue();
-		}
-	}
-
 	public class NumberValidator


Файл все ещё называется NumberValidatorTests)

desolver · 2023-11-30T12:13:30Z

cs/HomeExercises/ObjectComparison.cs

-				new Person("Vasili III of Russia", 28, 170, 60, null));
-		}
-	}
-
 	public class Person


И тут название файла не совпадает с названием класса

desolver · 2023-11-30T12:16:31Z

cs/HomeExercises/tests/NumberValidator_Should.cs

+		[TestCase(3, 2,"0,", TestName ="Aren'tDigitsAfterCommas")]
+		[TestCase(3, 2,"0.0.0", TestName = "ThreeDigitsInNumberByTwoDots")]
+		[TestCase(3, 2,"0,0,0", TestName = "ThreeDigitsInNumberByTwoCommas")]


Снова форматирование уехало

desolver · 2023-11-30T12:18:17Z

cs/HomeExercises/tests/NumberValidator_Should.cs

+			creation
+				.Should() 
+				.NotThrow<Exception>();
+


Лишняя строка пустая

desolver · 2023-11-30T12:21:25Z

cs/HomeExercises/NumberValidatorTests.cs

-using FluentAssertions;
-using NUnit.Framework;
-using NUnit.Framework.Internal;
+


Тоже лишняя пустая строка

desolver · 2023-11-30T12:24:34Z

cs/HomeExercises/tests/NumberValidator_Should.cs

+		[TestCase(3, 2, "-1.23", TestName = "NumberNegativeInNumberOnlyPositive")]
+		[TestCase(17, 2, "0.000", TestName = "NumberScaleGreaterThanValidatorScale")]
+		[TestCase(17, 2, "a.sd", TestName = "IncorrectNumberFormatBecauseLettersArePresent")]
+		public void IsValidNumber_False(int precision, int scale, string number)


Все ещё нет глагола перед False и True тут и ниже, вспомни про порядок именований в тестах

Method_Should..._When...

Добавил по тесту с onlyPositive = true Доработал нейминг тестов

Отрефакторил NumberValidator и ObjectComparsion, добавил новые тесты …

9fda963

…в NumberValidator

desolver suggested changes Nov 28, 2023

View reviewed changes

desolver suggested changes Nov 30, 2023

View reviewed changes

WoeromProg added 2 commits November 30, 2023 20:01

Изменил имена файлов

6a3a718

Добавил по тесту с onlyPositive = true Доработал нейминг тестов

Изменил имена файлов без опечатки (ObjectComparison)

6cbad54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Трофимов Никита #239

Трофимов Никита #239

WoeromProg commented Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 28, 2023 •

edited

Loading

desolver Nov 28, 2023

desolver Nov 28, 2023

desolver Nov 30, 2023

desolver Nov 30, 2023

desolver Nov 30, 2023

desolver Nov 30, 2023

desolver Nov 30, 2023

desolver Nov 30, 2023

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

		@@ -15,16 +15,9 @@ public void CheckCurrentTsar()
		var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
		new Person("Vasili III of Russia", 28, 170, 60, null));

Трофимов Никита #239

Are you sure you want to change the base?

Трофимов Никита #239

Conversation

WoeromProg commented Nov 28, 2023

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

desolver Nov 28, 2023 • 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

desolver Nov 28, 2023 •

edited

Loading