Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 73 additions & 18 deletions Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs
Original file line number Diff line number Diff line change
@@ -1,38 +1,58 @@
using NUnit.Framework;
using FluentAssertions;
using FluentAssertions.Equivalency;
using NUnit.Framework;
using NUnit.Framework.Legacy;

namespace HomeExercise.Tasks.ObjectComparison;

public class ObjectComparison
{
[Test]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Оставлю комментарий здесь, т.к. github не дает мне выбрать неизмененные строки кода
Обычно имена тестов следуют некоторым правилам (соглашениям). В разных компаниях (да и командах внутри компании) они могут различаться. Например, один из способов именования такой: <ClassName>_<ShouldBe...>_[When...]_Test.
В данном случае можно ограничиться добавлением суффикса _Test в конец имени тестов. Так мы явно выделим эти методы от "обычных" методов.

К слову, почти такой же способ ты использовал в NumberValidatorTests

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

поменял названия, которые совсем не подходили по структуре
обновил названия, которые не полностью совпадали со структурой

[Description("Проверка текущего царя")]
[Category("ToRefactor")]
public void CheckCurrentTsar()
public void TsarRegistry_ShouldReturnExpectedCurrentTsar_Test()
{
var actualTsar = TsarRegistry.GetCurrentTsar();
var expectedTsar = GetExpectedCurrentTsar();

var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
new Person("Vasili III of Russia", 28, 170, 60, null));
actualTsar.Should().BeEquivalentTo(expectedTsar, config => config
.Excluding((IMemberInfo info) => info.Name == "Id")
.IgnoringCyclicReferences());

// Перепишите код на использование Fluent Assertions.
ClassicAssert.AreEqual(actualTsar.Name, expectedTsar.Name);
ClassicAssert.AreEqual(actualTsar.Age, expectedTsar.Age);
ClassicAssert.AreEqual(actualTsar.Height, expectedTsar.Height);
ClassicAssert.AreEqual(actualTsar.Weight, expectedTsar.Weight);
/*
Заменяем на Should().BeEquivalentTo() из Fluent Assertions.
BeEquivalentTo проверяет два объекта необязательно одного класса на совпадение одноименных полей,
на совпадение полей объектов, которые могут находиться в качестве ссылок в полях сравниваемых объектов
(как в случае с Parent внутри класса Person)
Здесь не хватит цепочки Should().BeEquivalentTo(), ведь метод сравнивает все поля
(в том числе поля объектов, которые лежат в полях в виде ссылки сравниваемого объекта),
и здесь нам мешают Id, ведь в тесте мы создаем новые объекты класса, Id у объектов разные
Мы работаем с настройками данного assertion-метода, исключая из проверки поля с айдишниками
Считаю, что лучше исключать конкретные поля, даже если их несколько, не используя EndsWith, чтобы
не возникало проблем с тем, что чудом может появится поле, которое оканчивается на Id и его необходимо
включать в проверку, а с EndsWith мы его пропустим.
Если actualTsar == null || expectedTsar == null, то выдает false, как и предполагается.

ClassicAssert.AreEqual(expectedTsar.Parent!.Name, actualTsar.Parent!.Name);
ClassicAssert.AreEqual(expectedTsar.Parent.Age, actualTsar.Parent.Age);
ClassicAssert.AreEqual(expectedTsar.Parent.Height, actualTsar.Parent.Height);
ClassicAssert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent);
Уточнения:
1. Если мы захотим создать класс, похожий на Person и сравнить, то проверка пройдет, но необходимо
осознавать, что если в actual будет какое-то поле, которого нет в expected, то по проверке объекты не
будут "эквивалентны", а если наоборот, в expected найдутся все поля actual, но expected будет содержать
лишние поля, то тест будет пройден успешно, поэтому разные классы лучше не использовать, такое "расширение"
может ломать логику теста и выдавать некорректные результаты без внесения изменений
2. Такое построение теста дает полную возможность добавлять практически любые поля без внесения
изменений в тест, если мы остаемся в рамках одно и того же класса, ведь метод сам пробегается по всем полям
без уточнений в его параметрах или в тесте в целом. Исключение и проблема - поля, которые схожи с Id,
то есть являются какой-то "внутрянкой" и используются исключительно при работе с объектами класса в коде,
при их появлении тест начнет падать, но хотя бы этим падением даст о себе знать и разработчик сможет
быстро добавить лишний .Excluding() в тест
*/
}

[Test]
[Description("Альтернативное решение. Какие у него недостатки?")]
public void CheckCurrentTsar_WithCustomEquality()
public void TsarRegistry_ShouldReturnExpectedCurrentTsar_WithCustomEquality_Test()
{
var actualTsar = TsarRegistry.GetCurrentTsar();
var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
new Person("Vasili III of Russia", 28, 170, 60, null));
var expectedTsar = GetExpectedCurrentTsar();

// Какие недостатки у такого подхода?
ClassicAssert.True(AreEqual(actualTsar, expectedTsar));
Expand All @@ -49,4 +69,39 @@ private bool AreEqual(Person? actual, Person? expected)
&& actual.Weight == expected.Weight
&& AreEqual(actual.Parent, expected.Parent);
}
}

private static Person GetExpectedCurrentTsar()
{
var expectedTsarParent = new Person("Vasili III of Russia", 28, 170, 60, null);
return new Person("Ivan IV The Terrible", 54, 170, 70, expectedTsarParent);
}

/*
Недостатки альтернативного решения:

1. Альтернативное решение не позволяет спокойно расширять функциональность класса, а точнее добавлять новые поля
в класс, объекты которого мы сравниваем.
Мы в любом случае должны вносить изменения в AreEqual, который при всем при этом лежит внутри файла с тестами,
то есть он не является методом класса, объекты которого мы сравниваем. Разработчик мог бы сразу изменить
подобный метод сравнения при появлении новых полей, если бы он использовался в рамках работы с классом вне тестов
В данном случае тест упадет при появлении новых полей, если разработчик не знает о внутрянках тестов или не лезет
туда вообще, хотя появление новых полей (исключая что-то по типу Id) никак не должно влиять на исход работы теста

2. Альтернативное решение при запуске теста и появлении какого-то несовпадения не дает никаких подробностей
по тому, что вообще произошло. При использовании булевого утверждения увидим, что Expected: True, But was: False
Данная информация не помогает проблему решить, а использование Fluent Assertions дает подробную информацию

3. Читаемость: в целом очень легко понять, что делает данный тест, не заглядывая в AreEqual.
Однако, если мы хотим вникнуть в подробности, что конкретно происходит в тесте, то мы опускаем глаза и видим
не очень большой но неприятный отрывок кода, в котором необходимо разобраться. Тут даже используется рекурсия,
чтобы проверять объект по ссылке на родителя (рекурсия имеет свойство вызывать переполнение стека, чисто
теоретически это может произойти и здесь, если человек чудом сам себе родитель, нет проверки на
циклические ссылки). Относительно "моего" решения разница безусловно есть, но назвать читаемость явной
проблемой этого решения нельзя, слегка приятнуто за уши

Небольшой плюс альтернативного решения:
Оно будет быстрее использования BeEquivalentTo(), по информации из интернета метод из Fluent Assertions
использует рефлексию и может быть в 10 раз медленее альтернативного решения, что в целом не так критично
исходя из сложности обоих методов, отсюда вытекает и небольшой минус выбранного мной решения.
*/
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public NumberValidator(int precision, int scale = 0, bool onlyPositive = false)
if (precision <= 0)
throw new ArgumentException("precision must be a positive number");
if (scale < 0 || scale >= precision)
throw new ArgumentException("precision must be a non-negative number less or equal than precision");
throw new ArgumentException("scale must be a non-negative number less or equal than precision");
numberRegex = new Regex(@"^([+-]?)(\d+)([.,](\d+))?$", RegexOptions.IgnoreCase);
}

Expand Down
108 changes: 85 additions & 23 deletions Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,31 +1,93 @@

using NUnit.Framework;
using NUnit.Framework.Legacy;
using NUnit.Framework;
using FluentAssertions;

namespace HomeExercise.Tasks.NumberValidator;

[TestFixture]
public class NumberValidatorTests
{
[Test]
public void Test()
{
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));

ClassicAssert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
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.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"));
[TestCase(0, 2, "positive", TestName = "Precision is zero")]
[TestCase(1, -1, "non-negative", TestName = "Scale is negative")]
[TestCase(1, 2, "less or equal", TestName = "Precision is less than scale")]
[TestCase(1, 1, "less or equal", TestName = "Precision equals scale")]
public void Constructor_ThrowsException_OnInvalidArguments_Test(int precision, int scale, string expectedMessage)
{
var constructor = () => new NumberValidator(precision, scale);
constructor.Should().Throw<ArgumentException>().WithMessage($"*{expectedMessage}*");
}

[TestCase(1, 0, TestName = "Precision and scale are minimum possible")]
[TestCase(2, 1, TestName = "Highest valid scale below precision")]
public void Constructor_DoesNotThrowException_OnBoundaryValidArguments_Test(int precision, int scale)
{
var constructor = () => new NumberValidator(precision, scale, true);
constructor.Should().NotThrow();
}
Comment on lines 19 to 25

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

С одной стороны, под рефакторингом понимается изменение кода без изменения его поведения. С этой точки зрения тесты действительно должны быть - ведь они были в исходной версии.
Но с другой стороны - какую пользу дает этот набор тестов сравнительно с тестами Constructor_ThrowsException_OnInvalidArguments?
Т.е. мы очевидно ожидаем, что конструктор не будет кидать исключение при корректных параметрах. Вместе с этим мы описали некорректные параметры в пессимистичном тесте Constructor_ThrowsException_OnInvalidArguments. Я не предлагаю снести эти тесты - этот комментарий тебе просто на обдумывание

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это хороший подход
Граничные случаи - достаточно частое место для ошибок


[TestCase("a.sd", TestName = "Input contains letters")]
[TestCase("", TestName = "Input is empty string")]
[TestCase(null, TestName = "Input is null")]
[TestCase("12+3", TestName = "Input contains invalid characters")]
public void Validator_ReturnsFalse_OnInvalidInputFormats_Test(string input)
{
var validator = new NumberValidator(5);
validator.IsValidNumber(input).Should().BeFalse();
}

[TestCase(1, 0, "0", TestName = "One zero")]
[TestCase(2, 1, "0.0", TestName = "One zero with fraction part")]
[TestCase(3, 1, "00.0", TestName = "Two leading zeros with fraction part")]
public void Validator_ReturnsTrue_WithProperZeros_Test(int precision, int scale, string input)
{
var validator = new NumberValidator(precision, scale);
validator.IsValidNumber(input).Should().BeTrue();
}

[TestCase(4, 0, "1234", TestName = "Simple integer")]
[TestCase(5, 2, "123.54", TestName = "Number with fraction part")]
public void Validator_ReturnsTrue_WithProperNumbers_Test(int precision, int scale, string input)
{
var validator = new NumberValidator(precision, scale);
validator.IsValidNumber(input).Should().BeTrue();
}

[TestCase("123", TestName = "Precision overflow with numerals")]
[TestCase("000", TestName = "Precision overflow with zeros")]
public void Validator_ReturnsFalse_WithPrecisionOverflow_Test(string input)
{
var validator = new NumberValidator(2);
validator.IsValidNumber(input).Should().BeFalse();
}

[TestCase("1.23456", TestName = "Scale overflow with numerals")]
[TestCase("0.00000", TestName = "Scale overflow with zeros")]
public void Validator_ReturnsFalse_WithScaleOverflow_Test(string input)
{
var validator = new NumberValidator(10, 4);
validator.IsValidNumber(input).Should().BeFalse();
}

[TestCase("-123", TestName = "Negative integer")]
[TestCase("-0", TestName = "Negative zero")]
public void Validator_ReturnsFalse_WithNegativeNumberWhenNotAllowed_Test(string input)
{
var validator = new NumberValidator(10, 0, true);
validator.IsValidNumber(input).Should().BeFalse();
}

[TestCase("-1.23", TestName = "Negative float")]
[TestCase("+1.23", TestName = "Positive float")]
public void Validator_ReturnsFalse_WithSignPrecisionOverflow_Test(string input)
{
var validator = new NumberValidator(3, 2);
validator.IsValidNumber(input).Should().BeFalse();
}

[TestCase("12,34", TestName = "Comma separator")]
[TestCase("12.34", TestName = "Dot separator")]
public void Validator_ReturnsTrue_WithCommaSeparator_Test(string input)
{
var validator = new NumberValidator(5, 2);
validator.IsValidNumber(input).Should().BeTrue();
}
}