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

Галичев Артем #13

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 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
37 changes: 25 additions & 12 deletions Testing/Basic/Homework/1. ObjectComparison/ObjectComparison.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using NUnit.Framework;
using FluentAssertions;

Choose a reason for hiding this comment

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

Привет) давай сначала общие рекомендации:

  1. Именование коммитов) Имена должны быть емкими (и желательно на английском языке, как стандарте в ИТ индустрии) у меня в github desktop'e они даже не влезают в строчку имени и уезжают в описание

  2. Содержание коммитов должно быть атомарным. Один коммит - одно логическое изменение. Например, в коммите NumberValidatorTests. Написал недостающие тесты, разделил разные тестовые случаи на разные тестовые методы. Создал класс TestNumberValidatorBuilder для удобного создания класса NumberValidator ты уже сам видишь, что завел много различных независимых изменений, которые просто завернул в один коммит и отправил. Рекомендую стараться разделять независимое. Зачем это нужно? Чтобы по истории коммитов, понимать, что произошло. Или уметь ревертить только "плохие" части. Или уметь их черри-пикать. И тд

Прямо сейчас что-либо делать с коммитами не надо. Это рекомендации на будущее

Choose a reason for hiding this comment

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

Также могу порекомендовать отмечать мои сообщения какой-нибудь эмодзи, чтобы отмечать проработанное и не запутаться перед отправкой на следующую итерацию ревью)

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

namespace HomeExercise.Tasks.ObjectComparison;
Expand All @@ -13,17 +14,8 @@ public void CheckCurrentTsar()

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.IsEqual(expectedTsar);

Choose a reason for hiding this comment

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

Можешь рассказать, почему вынес в экстеншн эту проверку?

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.

Понял, корректно)
Могу только немного со стороны накинуть, что пока использование единственное и сама проверка не супер сложная (всего шесть строк, когнитивная нагрузка низкая).ю то можно оставлять прямо в тесте. А вот при дублировании или усложнении проверки - да, вынести в метод или экстешн хорошее повышение читаемости)

}

[Test]
Expand All @@ -35,6 +27,14 @@ public void CheckCurrentTsar_WithCustomEquality()
new Person("Vasili III of Russia", 28, 170, 60, null));

// Какие недостатки у такого подхода?
/*
* 1) Если у класса изменять поля (удалять или добавлять), то также придется изменять

This comment was marked as resolved.

* логику проверки на соответствие сущностей. В случае с удалением поля программа вообще не скомпилируется
* 2) Возможна бесконечная рекурсия
* 3) При падении теста не поймем в чем была причина. Ожидался True, а получили False, вот и вся инфа(
* 4) Использование FluentAssertion ускоряет понимание логики секции проверки, за счет использования
* method chaining. В данном тесте вникнуть в логику проверки теста сложнее, чем в первом.
*/
ClassicAssert.True(AreEqual(actualTsar, expectedTsar));
}

Expand All @@ -50,3 +50,16 @@ private bool AreEqual(Person? actual, Person? expected)
&& AreEqual(actual.Parent, expected.Parent);
}
}

internal static class ObjectComparisonAssertExtensions
{
public static void IsEqual(this Person actualTsar, Person expectedTsar)
{
actualTsar.Should().BeEquivalentTo(
expectedTsar,
options => options
.Excluding(x => x.Name == nameof(Person.Id) && x.DeclaringType == typeof(Person))
.AllowingInfiniteRecursion()
.IgnoringCyclicReferences());
}
}
68 changes: 49 additions & 19 deletions Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs
Original file line number Diff line number Diff line change
@@ -1,31 +1,61 @@

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

namespace HomeExercise.Tasks.NumberValidator;

[TestFixture]
public class NumberValidatorTests
{
[TestCase(-1, 5, TestName = "Should_Throw_ArgumentException_If_Precision_Is_Negative")]
[TestCase(5, -1, TestName = "Should_Throw_ArgumentException_If_Scale_Is_Negative")]
[TestCase(2, 5, TestName = "Should_Throw_ArgumentException_If_Scale_Greater_Precision")]
[TestCase(2, 2, TestName = "Should_Throw_ArgumentException_If_Scale_Equal_Precision")]
public void Should_Throw_Exception(int precision, int scale)

This comment was marked as resolved.

{
var act = () => new NumberValidator(precision, scale);

This comment was marked as resolved.


act.Should().Throw<ArgumentException>();
}

[Test]
public void Test()
public void Should_DoesNotThrow_Exception_If_Scale_And_Precision_Is_Correct()
{
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));
var act = () => new NumberValidator(10, 5);

act.Should().NotThrow<ArgumentException>();

This comment was marked as resolved.

}

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(null, TestName = "IsValidNumber_Should_Return_False_If_Value_Is_Null")]
[TestCase(" ", TestName = "IsValidNumber_Should_Return_False_If_Value_Is_Empty")]
[TestCase("2.a1", TestName = "IsValidNumber_Should_Return_False_If_Value_Contains_Letter")]
[TestCase("0.", TestName = "IsValidNumber_Should_Return_False_If_Value_Contains_Separator_Without_FracPart")]
[TestCase(",0", TestName = "IsValidNumber_Should_Return_False_If_Value_Without_IntPart")]
[TestCase("0*0", TestName = "IsValidNumber_Should_Return_False_If_Value_Separator_Incorrect")]
[TestCase("!0,0", TestName = "IsValidNumber_Should_Return_False_If_Value_Sign_Incorrect")]
[TestCase("1,234", TestName = "IsValidNumber_Should_Return_False_If_Value_FracPart_More_Than_Scale")]
[TestCase("11111", TestName = "IsValidNumber_Should_Return_False_If_Value_IntPart_More_Than_Precision")]
[TestCase("225,32", TestName = "IsValidNumber_Should_Return_False_If_Value_With_FracPart_More_Than_Precision")]
[TestCase("+14,23", TestName = "IsValidNumber_Should_Return_False_If_Value_With_Sign_More_Than_Precision")]
[TestCase("-0", TestName = "IsValidNumber_Should_Return_False_If_OnlyPositive_True_And_Value_Contains_Minus")]
public void IsValidNumber_Should_Return_False(string value)
{
var numberValidator = new NumberValidator(4, 2, true);

var actual = numberValidator.IsValidNumber(value);

actual.Should().BeFalse();

This comment was marked as resolved.

}

[TestCase("-1,4", false, TestName = "IsValidNumber_Should_Return_True_If_OnlyPositive_False_And_Value_Contains_Minus")]
[TestCase("1", TestName = "IsValidNumber_Should_Return_True_If_Value_IntPart")]
[TestCase("+1,444", TestName = "IsValidNumber_Should_Return_True_If_Value_Contains_FracPart_And_Sign")]
[TestCase("1,444", TestName = "IsValidNumber_Should_Return_True_If_Value_Contains_FracPart")]
public void IsValidNumber_Should_Return_True(string value, bool onlyPositive = true)
{
var numberValidator = new NumberValidator(10, 5, onlyPositive);

var actual = numberValidator.IsValidNumber(value);

actual.Should().BeTrue();
}
}