-
Notifications
You must be signed in to change notification settings - Fork 32
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
Леонтьев Дмитрий #19
base: master
Are you sure you want to change the base?
Леонтьев Дмитрий #19
Changes from 8 commits
5e91213
321e64c
138ba86
25512ae
a494c2c
c25ecda
383e965
dfebc84
4ab39c2
b71dc00
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 |
---|---|---|
@@ -1,29 +1,26 @@ | ||
using NUnit.Framework; | ||
using NUnit.Framework.Legacy; | ||
using FluentAssertions; | ||
|
||
namespace HomeExercise.Tasks.ObjectComparison; | ||
|
||
public class ObjectComparison | ||
{ | ||
[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, options => | ||
options | ||
.AllowingInfiniteRecursion() | ||
.Excluding(o => o.DeclaringType == typeof(Person) && o.Name == nameof(Person.Id))); | ||
} | ||
|
||
[Test] | ||
|
@@ -35,18 +32,26 @@ public void CheckCurrentTsar_WithCustomEquality() | |
new Person("Vasili III of Russia", 28, 170, 60, null)); | ||
|
||
// Какие недостатки у такого подхода? | ||
// При изменении класса Person придётся изменять метод AreEqual, | ||
// т.е. если мы добавим поле в Person, то придётся добавить проверку на него в этом методе. | ||
// При провале теста мы не получаем конкретной информации из-за чего тест не прошёл, | ||
// т.к. метод AreEqual возвращает bool и скажет нам только Success или Failed. | ||
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. resolved |
||
// Невозможно сравнить двух Person без определённых полей, | ||
// придётся создавать различные реализации метода AreEqual. | ||
ClassicAssert.True(AreEqual(actualTsar, expectedTsar)); | ||
} | ||
|
||
private bool AreEqual(Person? actual, Person? expected) | ||
{ | ||
if (actual == expected) return true; | ||
if (actual == null || expected == null) return false; | ||
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); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,31 +1,107 @@ | ||
| ||
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")); | ||
private static NumberValidator GetCorrectValidator(bool onlyPositive = false) => new(2, 1, onlyPositive); | ||
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. На мой взгляд, в таких маленьких тестах это не нужно. Обычно тест должен быть максимально наглядным и это нормально, если в 10 методах подряд написано new NumberValidator(), зато сразу видно с какими параметрами. В тестах, где блок Arrange становится слошком громозким, более оправдано что-то выносить вовне. |
||
|
||
[TestCase(1, TestName = "Correct parameters")] | ||
public void Constructor_WithCorrectParameters_NotThrows(int precision, int scale = 0) | ||
{ | ||
var creation = () => new NumberValidator(precision, scale, true); | ||
|
||
creation.Should() | ||
.NotThrow<ArgumentException>(); | ||
} | ||
|
||
[TestCase(-1, TestName = "negative precision")] | ||
[TestCase(0, TestName = "zero precision")] | ||
public void Constructor_WithIncorrectPrecision_Throws(int precision, int scale = 0) | ||
{ | ||
var creation = () => new NumberValidator(precision, scale); | ||
|
||
creation.Should() | ||
.Throw<ArgumentException>() | ||
.WithMessage("precision must be a positive number"); | ||
} | ||
|
||
[TestCase(1, -1, TestName = "negative scale")] | ||
[TestCase(1, 2, TestName = "scale greater than precision")] | ||
[TestCase(1, 1, TestName = "scale equal to precision")] | ||
public void Constructor_WithIncorrectScale_Throws(int precision, int scale) | ||
{ | ||
var creation = () => new NumberValidator(precision, scale); | ||
|
||
creation.Should() | ||
.Throw<ArgumentException>() | ||
.WithMessage("scale must be a non-negative number less than precision"); | ||
} | ||
|
||
[TestCase(2, 0, true, "+1", TestName = "number with a plus")] | ||
public void IsValidNumber_CorrectPlus_ReturnsTrue( | ||
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. С названиями теперь в целом хорошо. Совсем минорная шутка: на мой вкус не хватает какого-нибудь предлога во второй части, чтобы это было не просто Что_Условия_Результат, а Что_ПриКакихУсловиях_Результат. В тестах на конструктор ты используешь With, а ниже предлог потерялся. |
||
int precision, | ||
int scale, | ||
bool onlyPositive, | ||
string expectedResultValue) | ||
{ | ||
new NumberValidator(precision, scale, onlyPositive).IsValidNumber(expectedResultValue).Should().BeTrue(); | ||
} | ||
|
||
[TestCase(2, 0, false, "-1", TestName = "number with a minus")] | ||
public void IsValidNumber_CorrectMinus_ReturnsTrue( | ||
int precision, | ||
int scale, | ||
bool onlyPositive, | ||
string expectedResultValue) | ||
{ | ||
new NumberValidator(precision, scale, onlyPositive).IsValidNumber(expectedResultValue).Should().BeTrue(); | ||
} | ||
|
||
[TestCase(1, 0, true, "0", TestName = "one digit")] | ||
[TestCase(3, 0, true, "123", TestName = "three digits")] | ||
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. Остались странные тесты |
||
public void IsValidNumber_CorrectNumberOfDigits_ReturnsTrue( | ||
int precision, | ||
int scale, | ||
bool onlyPositive, | ||
string expectedResultValue) | ||
{ | ||
new NumberValidator(precision, scale, onlyPositive).IsValidNumber(expectedResultValue).Should().BeTrue(); | ||
} | ||
|
||
[TestCase("0.1", TestName = "separator dot")] | ||
[TestCase("0,1", TestName = "separator comma")] | ||
public void IsValidNumber_CorrectSeparator_ReturnsTrue(string expectedResultValue) | ||
{ | ||
GetCorrectValidator().IsValidNumber(expectedResultValue).Should().BeTrue(); | ||
} | ||
|
||
[TestCase("", TestName = "empty string")] | ||
[TestCase(null, TestName = "null")] | ||
public void IsValidNumber_IsNullOrEmpty_ReturnsFalse(string expectedResultValue) | ||
{ | ||
GetCorrectValidator().IsValidNumber(expectedResultValue).Should().BeFalse(); | ||
} | ||
|
||
[TestCase(".0", TestName = "intPart is missing")] | ||
[TestCase("0.", TestName = "fracPart part is missing")] | ||
public void IsValidNumber_MatchIsNotSuccess_ReturnsFalse(string expectedResultValue) | ||
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. Вот тут имя концептуально неверное). Ты должен проверять бизнес-кейсы, а не догадываться, что в реализации есть некий Matсh по регулярке, который ничего не найдет. Понятно, что ты можешь заглянуть в реализацию и на основе этого писать тесты, в данном случае так и нужно делать. Но класс с тестами должен быть написан в терминах бизнес-требований, а не нюансов реализации. |
||
{ | ||
GetCorrectValidator().IsValidNumber(expectedResultValue).Should().BeFalse(); | ||
} | ||
|
||
[TestCase("+11", TestName = "more numbers than precision (only intPart)")] | ||
[TestCase("-1.1", TestName = "more numbers than precision (intPart and fracPart)")] | ||
public void IsValidNumber_IncorrectPrecision_ReturnsFalse(string expectedResultValue) | ||
{ | ||
GetCorrectValidator().IsValidNumber(expectedResultValue).Should().BeFalse(); | ||
} | ||
|
||
[TestCase("0.11", TestName = "more digits in fracPart than scale")] | ||
public void IsValidNumber_IncorrectScale_ReturnsFalse(string expectedResultValue) | ||
{ | ||
GetCorrectValidator().IsValidNumber(expectedResultValue).Should().BeFalse(); | ||
} | ||
} |
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.
Теперь окей. При сравнении объектового графа еще иногда полезно игнорировать циклические ссылки. Но в данном случае это не обязательно, тут по бизнеслогике цари в династии повторяться не должны вроде как)