Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Попов Захар #29

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
47 changes: 32 additions & 15 deletions Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
using NUnit.Framework;
using FluentAssertions;
using FluentAssertions.Equivalency;
using NUnit.Framework;
using NUnit.Framework.Legacy;

namespace HomeExercise.Tasks.ObjectComparison;

public class ObjectComparison
{
private Person actualTsar;

[SetUp]
[Description("Получение экземпляра текущего царя")]
public void SetUp()
{
actualTsar = TsarRegistry.GetCurrentTsar();
}
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved

[Test]
[Description("Проверка текущего царя")]
[Category("ToRefactor")]
public void CheckCurrentTsar()
{
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));

// Перепишите код на использование 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);

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);
actualTsar.Should().BeEquivalentTo(expectedTsar,
ExcludeIdFromPersonClass);
}

private EquivalencyAssertionOptions<Person> ExcludeIdFromPersonClass(
EquivalencyAssertionOptions<Person> congiguration)
=> congiguration
.Excluding(member =>
member.DeclaringType == typeof(Person)
&& member.Name.Equals(nameof(Person.Id)));
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved

[Test]
[Description("Альтернативное решение. Какие у него недостатки?")]
public void CheckCurrentTsar_WithCustomEquality()
Expand All @@ -34,7 +43,15 @@ public void CheckCurrentTsar_WithCustomEquality()
var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
new Person("Vasili III of Russia", 28, 170, 60, null));

// Какие недостатки у такого подхода?
/* Какие недостатки у такого подхода?

Основным недостатком данного подхода я считаю необходимость
изменения метода AreEqual при добавлении новых полей класса Person.
Например, если мы добавим в Person поле Gender(пол),
то нам нужно будет в AreEqual так же ручками добавить
проверку совпадения полов:
actual.Gender == expected.Gender.
*/
Comment on lines +56 to +64
Copy link

Choose a reason for hiding this comment

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

Может быть есть ещё недостатки?

Copy link
Author

@BlizPerfect BlizPerfect Nov 6, 2024

Choose a reason for hiding this comment

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

Предположу, что так же недостатком является то, что данный метод является рекурсивным и возможна ситуация, при которой цепочка из царей превысит глубину рекурсии. Знаю, что это одна из проблем питона - малая глубина рекурсии по умолчанию. Шарп вроде за таким не замечен. Так же будет забиваться стек вызовов методов. Будут накладные расходы на постановку/снятие контекста метода в/из стек вызовов.

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

Copy link

Choose a reason for hiding this comment

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

Действительно, для большой вложенности могут возникнуть проблемы с конечным стеком потока. Но лишино ли решение с Fluent Assertions такой проблемы? Там точно цикл внутри а не подобная рекурсия?

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

Copy link
Author

Choose a reason for hiding this comment

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

Если я всё правильно понял, то исходя из документации, решение с Fluent Assertions на самом деле так же рекурсивно. По умолчанию глубина рекурсии выставлена как 10.

The comparison is recursive by default. To avoid infinite recursion, Fluent Assertions will recurse up to 10 levels deep by default, but if you want to force it to go as deep as possible, use the AllowingInfiniteRecursion option.

По недостаткам решения, могу ещё предложить следующее:

  1. Нет конкретики, какое поле оказалось разным, просто сообщается о том, что цари различаются.
  2. Если мы передаём оба царя как null, тогда цари считаются равными. Равны ли цари тогда? Философвски как-то.

Copy link

Choose a reason for hiding this comment

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

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

Второй пункт не совсем понял, мы всегда ожидаем какого-то царя. Если ожидаем null, вероятно так необходимо. Например, если в базе данных мы не ожидаем увидеть такого-то царя, то он может быть null ;)

Copy link
Author

Choose a reason for hiding this comment

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

Про второй пункт, мне это просто кажется логически неверно - нам пришли вообще не цари, оба null. Тогда тест вернёт true. Не цари равны.
Наверное я тут больше копаю в ту сторону, что до AreEqual не должно вообще доходить null, это должно отлавливаться заранее.

Copy link

Choose a reason for hiding this comment

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

Про второй пункт, мне это просто кажется логически неверно - нам пришли вообще не цари, оба null. Тогда тест вернёт true. Не цари равны.

Но ведь в тестах у нас есть два значения:

  • actual - значение, которое мы получили из базы данных/внешнего API/метода
  • expected - константа в тесте, которую ожидалась увидеть.

В этом плане не понятно, что такое "пришли не цари". Так как один параметр контролируется тестом.

Если имелось ввиду, что вместо царя со значением null пришла королева со значением null, то это не допускается статической типизацией языка :)

А также вопрос, разве сейчас два null сравниваются по-другому через Fluent Assertions?

Copy link
Author

Choose a reason for hiding this comment

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

Да, что-то я действительно тут переусложнил - мне это свойственно!

ClassicAssert.True(AreEqual(actualTsar, expectedTsar));
}

Expand All @@ -49,4 +66,4 @@ private bool AreEqual(Person? actual, Person? expected)
&& actual.Weight == expected.Weight
&& AreEqual(actual.Parent, expected.Parent);
}
}
}
143 changes: 122 additions & 21 deletions Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,31 +1,132 @@

using NUnit.Framework;
using NUnit.Framework;
using NUnit.Framework.Legacy;
using static System.Formats.Asn1.AsnWriter;

namespace HomeExercise.Tasks.NumberValidator;

[TestFixture]
public class NumberValidatorTests
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved
{
[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)]
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved
[TestCase(-1)]
[TestCase(int.MinValue)]
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved
public void NumberValidator_ThrowsArgumentException_WhenPrecisionLessOrEqualThanZero(int precision)
{
Assert.Throws<ArgumentException>(() => new NumberValidator(0));
}
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved

[Test]
[TestCase(1, -1)]
[TestCase(1, int.MinValue)]
public void NumberValidator_ThrowsArgumentException_WhenScaleLessThanZero(int precision,
int scale)
{
Assert.Throws<ArgumentException>(() => new NumberValidator(precision, scale));
}

[Test]
[TestCase(1, 1)]
[TestCase(1, 2)]
public void NumberValidator_ThrowsArgumentException_WhenScaleMoreOrEqualThanPrecision(int precision,
int scale)
{
Assert.Throws<ArgumentException>(() => new NumberValidator(precision, scale));
}

[Test]
[TestCase(17, 2, true, null)]
[TestCase(17, 2, true, "")]
[TestCase(17, 2, true, " ")]
public void IsValidNumber_ReturnFalse_WhenGivenNullOrEmptyString(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == false);
}
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved

[Test]
[TestCase(17, 2, true, "0.0")]
[TestCase(17, 2, true, "0")]
[TestCase(4, 2, true, "+1.23")]
public void IsValidNumber_ReturnTrue_WhenGivenCorrectValues(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == true);
}

[Test]
[TestCase(17, 2, true, "0,0")]
public void IsValidNumber_ReturnTrue_WhenGivenCorrectValuesWithComma(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == true);
}

[Test]
[TestCase(17, 2, true, "a.sd")]
[TestCase(17, 2, true, "1..2")]
[TestCase(17, 2, true, "1.2.3")]
[TestCase(17, 2, true, "1.a")]
[TestCase(17, 2, true, "a.1")]
[TestCase(17, 2, true, "0.,0")]
public void IsValidNumber_ReturnFalse_WhenGivenNotANumberValues(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == false);
}

[Test]
[TestCase(3, 2, true, "00.00")]
[TestCase(3, 0, true, "0.000")]
[TestCase(3, 2, false, "-1.23")]
public void IsValidNumber_ReturnFalse_WhenPrecisionIsExceeded(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == false);
}

[Test]
[TestCase(17, 2, true, "0.000")]
public void IsValidNumber_ReturnFalse_WhenScaleIsExceeded(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == false);
}

[Test]
[TestCase(3, 2, true, "-0.00")]
[TestCase(3, 2, true, "-1.23")]
public void IsValidNumber_ReturnFalse_WhenInvalidNumberSign(int precision,
int scale, bool onlyPositive, string value)
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == false);
}

[Test]
[TestCase(3, 2, true, "+1.23")]
[TestCase(3, 2, true, "1.23")]
// Я думаю, что в данном задании подразумивается,
// что NumberValidator корректен.
// Тогда цитирую:
// "Формат числового значения указывается в виде N(m.к),
// где m – максимальное количество знаков в числе,
// включая знак (для отрицательного числа),".
// В моём понимании,исходя из документации в IsValidNumber,
// тест со значением "+1.23" должен
// возвращать true, так как +1.23 это положительное число,
// а знак включаться в длину должен только у отрицательного числа.
// Я не так понял?
public void IsValidNumber_ReturnTrue_WhenGivenCorrectPositiveValue(int precision,
int scale, bool onlyPositive, string value)
{
var validator = new NumberValidator(precision, scale, onlyPositive);
Assert.That(validator.IsValidNumber(value) == true);
BlizPerfect marked this conversation as resolved.
Show resolved Hide resolved
}
}