-
Notifications
You must be signed in to change notification settings - Fork 282
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
Волков Кирилл #236
base: master
Are you sure you want to change the base?
Волков Кирилл #236
Changes from 2 commits
6ee66ab
995aded
1cd3701
3f84a0c
36fe5aa
75bbb8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
using System; | ||
using System.Collections; | ||
using FluentAssertions; | ||
using NUnit.Framework; | ||
|
||
namespace HomeExercises.Tests | ||
{ | ||
public class NumberValidator_Should | ||
{ | ||
#region ConstructorTestsSources | ||
|
||
private static IEnumerable IncorrectConstructorParamsTests() | ||
{ | ||
yield return new TestCaseData(-1, 2, true) | ||
.SetName("Constructor_ThrowsArgumentExceptionOnNegativePrecision"); | ||
yield return new TestCaseData(1, 2, true) | ||
.SetName("Constructor_ThrowsArgumentExceptionOnPrecisionLessThanScale"); | ||
yield return new TestCaseData(1, 1, true) | ||
.SetName("Constructor_ThrowsArgumentExceptionSamePrecisionAndScale"); | ||
yield return new TestCaseData(1, -1, true) | ||
.SetName("Constructor_ThrowsArgumentExceptionOnNegativeScale"); | ||
} | ||
|
||
private static IEnumerable CorrectConstructorParamsTests() | ||
{ | ||
yield return new TestCaseData(2, 1, true) | ||
.SetName("Constructor_WorksWhenPrecisionIsNonNegativeAndGreaterThanScale"); | ||
yield return new TestCaseData(2, 1, false) | ||
.SetName("Constructor_WorksWhenPrecisionIsNonNegativeAndGreaterThanScale"); | ||
} | ||
|
||
#endregion | ||
|
||
#region IsValidNumberTestsSources | ||
|
||
private static IEnumerable IsValidNumberPrecisionTests() | ||
{ | ||
yield return new TestCaseData(3, 2, true, "00.00") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenSumOfIntPartAndFracPartIsGreaterThanPrecision") | ||
.Returns(false); | ||
yield return new TestCaseData(3, 2, true, "-0.00") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenSignWithFracAndIntPartsIsGreaterThanPrecision") | ||
.Returns(false); | ||
yield return new TestCaseData(3, 2, true, "+0.00") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenSignWithFracAndIntPartsIsGreaterThanPrecision") | ||
.Returns(false); | ||
|
||
yield return new TestCaseData(17, 2, true, "0.0") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenSumOfIntPartAndFracPartIsNotGreaterThanPrecision") | ||
.Returns(true); | ||
yield return new TestCaseData(17, 2, true, "0") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenIntPartIsNotGreaterThanPrecision") | ||
.Returns(true); | ||
yield return new TestCaseData(17, 2, true, "+0.0") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenSumOfIntPartAndFracPartIsNotGreaterThanPrecision") | ||
.Returns(true); | ||
} | ||
|
||
private static IEnumerable IsValidNumberScaleTests() | ||
{ | ||
yield return new TestCaseData(17, 2, true, "0.111") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenFracPartIsGreaterThanScale") | ||
.Returns(false); | ||
|
||
yield return new TestCaseData(17, 2, true, "0.11") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenFracPartIsNotGreaterThanScale") | ||
.Returns(true); | ||
} | ||
|
||
private static IEnumerable IsValidNumberSignTests() | ||
{ | ||
yield return new TestCaseData(17, 2, true, "-0.11") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenSignIsNegativeWithOnlyPositive") | ||
.Returns(false); | ||
|
||
yield return new TestCaseData(17, 2, true, "+0.11") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenSignIsPositiveWithOnlyPositive") | ||
.Returns(true); | ||
yield return new TestCaseData(17, 2, false, "+0.11") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenSignIsPositiveWithoutOnlyPositive") | ||
.Returns(true); | ||
yield return new TestCaseData(17, 2, false, "-0.11") | ||
.SetName("IsValidNumber_ReturnsTrue_WhenSignIsNegativeWithoutOnlyPositive") | ||
.Returns(true); | ||
} | ||
|
||
private static IEnumerable IsValidNumberValueTests() | ||
{ | ||
yield return new TestCaseData(17, 2, true, null) | ||
.SetName("IsValidNumber_ReturnsFalse_WhenNullIsGiven") | ||
.Returns(false); | ||
yield return new TestCaseData(17, 2, true, "") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenEmptyStringIsGiven") | ||
.Returns(false); | ||
yield return new TestCaseData(17, 2, true, "a.a") | ||
.SetName("IsValidNumber_ReturnsFalse_WhenNonDigitStringIsGiven") | ||
.Returns(false); | ||
} | ||
|
||
#endregion | ||
|
||
[TestCaseSource(nameof(IncorrectConstructorParamsTests))] | ||
public void FailsWithIncorrectConstructorArguments(int precision, int scale, bool onlyPositive) | ||
{ | ||
new Func<NumberValidator>(() => new NumberValidator(precision, scale, onlyPositive)) | ||
.Should() | ||
.ThrowExactly<ArgumentException>(); | ||
} | ||
|
||
[TestCaseSource(nameof(CorrectConstructorParamsTests))] | ||
public void InitsWithCorrectConstructorArguments(int precision, int scale, bool onlyPositive) | ||
{ | ||
new Func<NumberValidator>(() => new NumberValidator(precision, scale, onlyPositive)) | ||
.Should() | ||
.NotThrow(); | ||
} | ||
|
||
[ | ||
TestCaseSource(nameof(IsValidNumberPrecisionTests)), | ||
TestCaseSource(nameof(IsValidNumberScaleTests)), | ||
TestCaseSource(nameof(IsValidNumberSignTests)), | ||
TestCaseSource(nameof(IsValidNumberValueTests)), | ||
] | ||
public bool IsValidNumber(int precision, int scale, bool onlyPositive, string value) | ||
{ | ||
return new Func<NumberValidator>(() => new NumberValidator(precision, scale, onlyPositive))() | ||
.IsValidNumber(value); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
using FluentAssertions; | ||
using NUnit.Framework; | ||
|
||
namespace HomeExercises.Tests | ||
{ | ||
public class ObjectComparison_Should | ||
{ | ||
private Person actualTsar; | ||
private Person expectedTsar; | ||
|
||
[SetUp] | ||
public void SetUp() | ||
{ | ||
actualTsar = TsarRegistry.GetCurrentTsar(); | ||
expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70, | ||
new Person("Vasili III of Russia", 28, 170, 60, null)); | ||
} | ||
|
||
[Test] | ||
[Description("Проверка текущего царя")] | ||
[Category("ToRefactor")] | ||
public void CheckCurrentTsar() | ||
{ | ||
actualTsar.Should().BeEquivalentTo(expectedTsar, options => | ||
options | ||
.IncludingFields() | ||
.Excluding(p => p.Id) | ||
.Excluding(p => p.Parent!.Id) | ||
.Excluding(p => p.Parent!.Weight) | ||
.Excluding(p => p.Parent!.Parent) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Объясни почему выбрал именно такие проверки. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Почему именно такая просьба, потому что если сравнить 2 этих теста, то можно увидеть что у них разная логика сравнения There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .Excluding(p => p.Parent!.Id) - данное поле у нас статический автоиндекс, как в бдшках. Его нужно исключить, одинаковые цари будут падать на нем, потому что это уникальный идентификатор .Excluding(p => p.Parent!.Weight) - данной проверки не было в тесте, который был до рефакторинга. И поэтому решил исключить .Excluding(p => p.Parent!.Parent) - если не исключить данное поле, то нужно будет бесконечно дописывать игнорирование поля Id в родителях. А так как мы не знаем, сколько их у нас, поэтому исключил. Также при написании теста я руководствовался логикой, что мы проверяем именно данного царя/персону, а падение теста при проверке его прародителей в данном случае будет являться неожиданным поведением Также, если посмотреть на метод CheckCurrentTsar_WithCustomEquality, то можно заметить рекурсивный вызов, который может сильно замедлять время работы теста. И опять же действия, которые мы выполняем при рекурсии не соответствуют названию теста There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Иногда когда ты приходишь рефачить тесты бывают баги) Я с точки зрения не вижу почему у родителя должен отличаться Weight
не согласен с этим пунктом, т.к. мы если у нас будет бага на 4 или допустим 100 родителе, то у нас тест не будет выполнят свою функцию отлавливать баги
попробуй посмотреть перегрузку метода Excluding |
||
); | ||
} | ||
|
||
[Test] | ||
[Description("Альтернативное решение. Какие у него недостатки?")] | ||
public void CheckCurrentTsar_WithCustomEquality() | ||
{ | ||
Assert.True(AreEqual(actualTsar, expectedTsar)); | ||
|
||
// Какие недостатки у такого подхода? | ||
// 1. При большой родословной будут проверяться все предки. И, при наличии расхождения у родителей, нам будет сложно понять в каком объекте произошло падение теста | ||
// 2. Из названия теста непонятно, что именно значит CustomEquality и какое у него поведение | ||
// 3. Не показывается информация, на каком именно объекте тест упал, что усложняет разработку | ||
// 4. При расширении класса Person нужно будет дописывать AreEqual, а в FluentAssertions достаточно указать IncludingFields в конфигурации и все поля автоматически будут сравниваться | ||
// 5. Повышается читабельность и тесты проще писать | ||
// 7. Не инкапсулирована функциональность сравнения внутри класса | ||
|
||
// Улучшайзинги | ||
// 1. Также я вынес инициализацию объектов в метод SetUp, помеченный соответствующим атрибутом | ||
// 2. Для структурированности проекта вынес тесты в папку Tests | ||
} | ||
|
||
private bool AreEqual(Person? actual, Person? expected) | ||
{ | ||
if (actual == expected) return true; | ||
if (actual == null || expected == null) return false; | ||
return | ||
actual.Name == expected.Name | ||
&& actual.Age == expected.Age | ||
&& actual.Height == expected.Height | ||
&& actual.Weight == expected.Weight | ||
&& AreEqual(actual.Parent, expected.Parent); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Бывают случаи когда тебе важна не факт ошибки в тесте, но и текст этой ошибки, допустим когда этот текст выводиться клиенту