-
Notifications
You must be signed in to change notification settings - Fork 57
Павел Шипицын #37
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
base: master
Are you sure you want to change the base?
Павел Шипицын #37
Changes from all commits
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,24 @@ | ||
| using NUnit.Framework; | ||
| using NUnit.Framework.Legacy; | ||
| using FluentAssertions; | ||
|
|
||
| namespace HomeExercise.Tasks.ObjectComparison; | ||
| public class ObjectComparison | ||
| { | ||
| [Test] | ||
| [Description("Проверка текущего царя")] | ||
| [Category("ToRefactor")] | ||
| public void CheckCurrentTsar() | ||
| public void GetCurrentTsar_WhenComparingWithExpected_ShouldBeEquivalent() | ||
| { | ||
| 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); | ||
| // Преимущества решение с FluentAssertions: | ||
| // - Легко расширяем при добавлении новых свойств в Person | ||
| // - Автоматически проверяем все свойства, включая вложенные объекты до 10 уровней вложенности (по умолчанию) | ||
| // - При несовпадении конкретного свойства будет выдана информация какое именно свойство не совпало | ||
| actualTsar.ShouldBeEquivalentToPerson(expectedTsar); | ||
| } | ||
|
|
||
| [Test] | ||
|
|
@@ -34,10 +29,12 @@ 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)); | ||
|
|
||
| // Какие недостатки у такого подхода? | ||
| // Недостатки подхода с CustomEquality: | ||
| // - Нет детальной информации о том, какое именно свойство не совпало | ||
| // - При добавлении новых свойств в Person нужно менять метод AreEqual | ||
|
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. Помимо того что метод AreEqual неудобный, есть ли у него какие-то ещё проблемы, из-за которых что-то может пойти не так при работе с ним?
Author
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. Наверное проблемой может быть переполнения стека при большом уровне вложенности. Метод уйдет в бесконечную рекурсию. |
||
| // - Риск переполнения стека при большом уровне вложенности | ||
| ClassicAssert.True(AreEqual(actualTsar, expectedTsar)); | ||
| } | ||
|
|
||
| private bool AreEqual(Person? actual, Person? expected) | ||
| { | ||
| if (actual == expected) return true; | ||
|
|
@@ -50,3 +47,15 @@ private bool AreEqual(Person? actual, Person? expected) | |
| && AreEqual(actual.Parent, expected.Parent); | ||
| } | ||
| } | ||
|
|
||
| public static class PersonAssertions | ||
| { | ||
| public static void ShouldBeEquivalentToPerson(this Person actual, Person expected) | ||
| { | ||
| actual.Should().BeEquivalentTo(expected, options => options | ||
| .Excluding(field => | ||
| field.DeclaringType == typeof(Person) && | ||
| field.Name == nameof(Person.Id)) | ||
| .AllowingInfiniteRecursion()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,112 @@ | ||
| | ||
| using FluentAssertions; | ||
| using NUnit.Framework; | ||
| using NUnit.Framework.Legacy; | ||
|
|
||
| namespace HomeExercise.Tasks.NumberValidator; | ||
|
|
||
| [TestFixture] | ||
| public class NumberValidatorTests | ||
|
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. Вообще этот класс можно поделить на несколько. К примеру, разделить тесты на проверку конструктора и конкретного метода, выделив два файла ConstructorTests.cs и IsValidNumberTests.cs. NumberValidator можно упустить в названии файла, потому как по названию папки и так понятно какую сущность тут тестируем. Так будет проще ориентироваться в тестах, но здесь это не критично и скорее будет оверхедом. |
||
| { | ||
| public static IEnumerable<TestCaseData> InvalidConstructorArgsTestCases() | ||
| { | ||
| foreach (var onlyPositive in new[] { true, false }) | ||
| { | ||
| yield return new TestCaseData(0, 1, onlyPositive) | ||
| .SetName($"precision is zero onlyPositive={onlyPositive}"); | ||
| yield return new TestCaseData(-1, 2, onlyPositive) | ||
| .SetName($"precision is negative onlyPositive={onlyPositive}"); | ||
| yield return new TestCaseData(1, -1, onlyPositive) | ||
| .SetName($"scale is negative onlyPositive={onlyPositive}"); | ||
| yield return new TestCaseData(1, 2, onlyPositive) | ||
| .SetName($"scale greater than precision onlyPositive={onlyPositive}"); | ||
| yield return new TestCaseData(1, 1, onlyPositive) | ||
| .SetName($"scale equals precision onlyPositive={onlyPositive}"); | ||
| } | ||
| } | ||
|
|
||
| [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")); | ||
| [TestCaseSource(nameof(InvalidConstructorArgsTestCases))] | ||
| public void Constructor_WhenInvalidArgs_ThrowsArgumentException(int precision, int scale, bool 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. Вообще здесь было бы идеально перебирать все варианты значения onlyPositive, т.к. никакого влияния на конструктор он не имеет, но внутрь передаётся. Можно реализовать это через TestCaseSource https://docs.nunit.org/articles/nunit/writing-tests/attributes/testcasesource.html |
||
| { | ||
| Action act = () => new NumberValidator(precision, scale, onlyPositive); | ||
| act.Should().Throw<ArgumentException>(); | ||
| } | ||
|
|
||
| [TestCase(1, 0, true, TestName = "valid precision and scale")] | ||
| [TestCase(2, 1, false, TestName = "scale less than precision")] | ||
| public void Constructor_WhenValidArgs_Success(int precision, int scale, bool onlyPositive) | ||
| { | ||
| Action act = () => new NumberValidator(precision, scale, onlyPositive); | ||
| act.Should().NotThrow(); | ||
| } | ||
|
|
||
| [TestCase("0", 4, 2, true, TestName = "integer zero")] | ||
| [TestCase("0.0", 4, 2, true, TestName = "fractional zero")] | ||
| [TestCase("+0.0", 3, 2, true, TestName = "sign with zero")] | ||
| [TestCase("01.23", 4, 2, true, TestName = "leading zeros")] | ||
| [TestCase("+1.23", 4, 2, true, TestName = "positive with sign")] | ||
| [TestCase("-1.23", 4, 2, false, TestName = "negative when allowed")] | ||
| [TestCase("123.4", 4, 1, true, TestName = "precision equals number length")] | ||
| [TestCase("11,234", 5, 4, true, TestName = "comma separator")] | ||
| public void IsValidNumber_WhenValidNumbers_ReturnTrue(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).Should().BeTrue(); | ||
| } | ||
|
|
||
| [TestCase("", 1, 0, false, TestName = "empty string")] | ||
|
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. Можно к этим тесткейсам добавить случаи, когда перед числовым значением есть пробелы, после значения пробелы и случай когда до и после они есть |
||
| [TestCase(" 123", 4, 0, true, TestName = "leading space before number")] | ||
| [TestCase("123 ", 4, 0, true, TestName = "trailing space after number")] | ||
| [TestCase(" 123 ", 5, 0, true, TestName = "spaces both sides")] | ||
| [TestCase("1.2\n3", 4, 2, true, TestName = "special character inside")] | ||
| [TestCase("\n1.23", 4, 2, true, TestName = "special character before")] | ||
| [TestCase(null, 1, 0, false, TestName = "null value")] | ||
| [TestCase("abc", 3, 0, true, TestName = "non-digit string")] | ||
| [TestCase("1a.2", 3, 2, true, TestName = "invalid character in integer part")] | ||
| [TestCase("1.2a", 3, 2, true, TestName = "invalid character in fraction part")] | ||
| [TestCase("-1.23", 4, 2, true, TestName = "negative fraction when positive only")] | ||
| [TestCase("-123", 4, 0, true, TestName = "negative integer when positive only")] | ||
| [TestCase("123.", 3, 0, true, TestName = "missing fractional part")] | ||
| [TestCase(".123", 4, 3, true, TestName = "missing integer part")] | ||
| [TestCase("++1.23", 5, 2, true, TestName = "multiple signs")] | ||
| [TestCase("1.2.3", 3, 2, true, TestName = "multiple separators")] | ||
| [TestCase("1/23", 3, 2, true, TestName = "invalid separator")] | ||
| public void IsValidNumber_WhenInvalidNumbers_ReturnFalse(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).Should().BeFalse(); | ||
| } | ||
|
|
||
| [TestCase("12", 2, 0, false, TestName = "integer fits precision")] | ||
| [TestCase("-1", 2, 0, false, TestName = "negative integer fits precision")] | ||
| public void IsValidNumber_WhenIntegerBoundsValid_ReturnsTrue(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).Should().BeTrue(); | ||
| } | ||
|
|
||
| [TestCase("123", 2, 0, false, TestName = "integer exceeds precision")] | ||
| [TestCase("-12", 2, 0, false, TestName = "negative integer exceeds precision")] | ||
| public void IsValidNumber_WhenIntegerBoundsInvalid_ReturnsFalse(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).Should().BeFalse(); | ||
| } | ||
|
|
||
| [TestCase("12.34", 8, 4, false, TestName = "fraction fits scale")] | ||
| [TestCase("11.234", 5, 4, false, TestName = "fraction fits precision and scale")] | ||
| [TestCase("-12.3", 5, 4, false, TestName = "negative fraction fits bounds")] | ||
| public void IsValidNumber_WhenFractionalBoundsValid_ReturnsTrue(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).Should().BeTrue(); | ||
| } | ||
|
|
||
| [TestCase("+12.34", 4, 2, false, TestName = "signed number exceeds precision")] | ||
| [TestCase("1.234", 6, 2, false, TestName = "fraction exceeds scale")] | ||
| [TestCase("-123,4", 4, 1, false, TestName = "negative number exceeds precision")] | ||
| public void IsValidNumber_WhenFractionalBoundsInvalid_ReturnsFalse(string value, int precision, int scale, bool onlyPositive) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
| validator.IsValidNumber(value).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.
Для того чтобы понять, хорошо ли новый способ сравнения работает можно было бы написать тесткейсы с разными случаями. Здесь я тебя не прошу этого сделать, но вообще это может помочь в среднем обнаружить случаи, которые не отсмотрены с текущими проверками. Сейчас в текущем способе проверки есть проблема, попробуй её таким образом найти.