Копытов Михаил #49
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,61 @@ | ||
| using FluentAssertions; | ||
| using NUnit.Framework; | ||
| using NUnit.Framework.Legacy; | ||
| using FluentAssertions.Equivalency; | ||
|
|
||
| namespace HomeExercise.Tasks.ObjectComparison; | ||
|
|
||
| public class ObjectComparison | ||
| { | ||
| [Test] | ||
| [Description("Проверка текущего царя")] | ||
| public void CurrentTsar_Should_HaveExpectedProperties() | ||
| { | ||
| 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)); | ||
|
|
||
| AreEquivalent(actualTsar, expectedTsar); | ||
|
|
||
| //Преимущества такой реализации: | ||
| //-тест занимает меньше строк кода, | ||
| //-добавление новых свойств не приводит к изменению теста (кроме самой инициализации expectedTsar), | ||
| //-добавление вложенных объектов не приводит к изменению теста (кроме самой инициализации expectedTsar), | ||
|
There was a problem hiding this comment. А все ли вложенные объекты мы проверим? Советую почитать доку FluentAssertions https://fluentassertions.com/
Author
There was a problem hiding this comment. Нет, проверяются только до 10 уровня вложения, но можно отключить при помощи AllowingInfiniteRecursion |
||
| //-явно указывается несовпадающее свойство. | ||
| } | ||
|
|
||
| [Test] | ||
| [Description("Альтернативное решение. Какие у него недостатки?")] | ||
| public void CheckCurrentTsar_WithCustomEquality() | ||
| { | ||
| 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)); | ||
|
|
||
| ClassicAssert.True(AreEqual(actualTsar, expectedTsar)); | ||
|
|
||
| //Какие недостатки у такого подхода? | ||
| //Недостатки: | ||
|
There was a problem hiding this comment. Помимо того что метод AreEqual неудобный, есть ли у него какие-то ещё проблемы, из-за которых что-то может пойти не так при работе с ним?
Author
There was a problem hiding this comment. Теоретически при добавлении "родственников" может получится цикл, а значит и бесконечная рекурсия. There was a problem hiding this comment. Ну не только это на самом деле прям самый крайний случай. При достаточно большой структуре, которая будет иметь много Parent-ов, мы просто упадём со StackOverflowException т.к. зайдём слишком глубоко. |
||
| //-линейный рост количества строк кода при добавлении новых свойств, | ||
| //-при добавлении новых свойст в класс их необходимо вносить в метод AreEqual, | ||
| //-можно забыть внести проверку совпадения свойств в AreEqual, при этом тест будет выполняться, | ||
| //-при падении теста явно не указывается свойство, из-за которого тест падает. | ||
| } | ||
|
|
||
| private void AreEquivalent(Person actualTsar, Person expectedTsar) | ||
| { | ||
| actualTsar | ||
| .Should() | ||
| .BeEquivalentTo(expectedTsar, options => | ||
| options.Excluding(info => IsPersonId(info))); | ||
| } | ||
|
|
||
| private bool IsPersonId(IMemberInfo info) | ||
| { | ||
| return info.Name == nameof(Person.Id) && | ||
| info.DeclaringType == typeof(Person); | ||
| } | ||
|
|
||
| private bool AreEqual(Person? actual, Person? expected) | ||
|
|
@@ -49,4 +69,4 @@ private bool AreEqual(Person? actual, Person? expected) | |
| && actual.Weight == expected.Weight | ||
| && AreEqual(actual.Parent, expected.Parent); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,87 @@ | ||
| using NUnit.Framework; | ||
| using FluentAssertions; | ||
|
|
||
| namespace HomeExercise.Tasks.NumberValidator; | ||
|
|
||
| [TestFixture] | ||
| public class NumberValidatorTests | ||
|
There was a problem hiding this comment. Вообще этот класс можно поделить на несколько. К примеру, разделить тесты на проверку конструктора и конкретного метода, выделив два файла ConstructorTests.cs и IsValidNumberTests.cs. NumberValidator можно упустить в названии файла, потому как по названию папки и так понятно какую сущность тут тестируем. Так будет проще ориентироваться в тестах, но здесь это не критично и скорее будет оверхедом. |
||
| { | ||
| public static IEnumerable<TestCaseData> InvalidConstructorParameters() | ||
| { | ||
| var onlyPositiveValues = new[] { true, false }; | ||
|
|
||
| foreach (var onlyPositive in onlyPositiveValues) | ||
| { | ||
| yield return new TestCaseData(-1, 2, onlyPositive) | ||
| .SetName($"Negative precision should throw (onlyPositive={onlyPositive})"); | ||
| yield return new TestCaseData(0, 5, onlyPositive) | ||
| .SetName($"Zero precision should throw (onlyPositive={onlyPositive})"); | ||
| yield return new TestCaseData(17, 17, onlyPositive) | ||
| .SetName($"Precision equals scale should throw (onlyPositive={onlyPositive})"); | ||
| yield return new TestCaseData(17, -1, onlyPositive) | ||
| .SetName($"Negative scale should throw (onlyPositive={onlyPositive})"); | ||
| yield return new TestCaseData(17, 18, onlyPositive) | ||
| .SetName($"Scale greater than precision should throw (onlyPositive={onlyPositive})"); | ||
| } | ||
| } | ||
|
|
||
| [Test, TestCaseSource(nameof(InvalidConstructorParameters))] | ||
| public void Constructor_Should_Throw_ArgumentException_When_Invalid_Parameters(int precision, int scale, bool onlyPositive) | ||
| { | ||
| var action = () => new NumberValidator(precision, scale, onlyPositive); | ||
|
|
||
| action.Should().Throw<ArgumentException>(); | ||
| } | ||
|
|
||
| [TestCase(10, 5, true, TestName = "Valid positive number")] | ||
| public void Constructor_Should_NotThrow_When_Valid_Parameters(int precision, int scale, bool onlyPositive) | ||
| { | ||
| var action = () => new NumberValidator(precision, scale, onlyPositive); | ||
|
|
||
| action.Should().NotThrow(); | ||
| } | ||
|
|
||
| [TestCase(17, 2, true, "+123.45", TestName = "Valid positive number with plus sign")] | ||
| [TestCase(17, 2, false, "-123.45", TestName = "Valid negative number with minus sign")] | ||
| [TestCase(17, 2, false, "123,45", TestName = "Valid number with with ',' between integer and scale parts")] | ||
| [TestCase(17, 0, true, "123", TestName = "Valid integer without scale part")] | ||
| [TestCase(17, 2, true, "0000123.45", TestName = "Valid number with leading zeros")] | ||
| [TestCase(17, 2, false, "-0", TestName = "Negative zero with onlyPositive=false")] | ||
| [TestCase(2, 0, false, "+0", TestName = "Positive zero with onlyPositive=false")] | ||
| public void IsValidNumber_Should_Return_True_When_Valid_Numbers(int precision, int scale, bool onlyPositive, string value) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
|
|
||
| var actualResult = validator.IsValidNumber(value); | ||
|
|
||
| actualResult.Should().BeTrue(); | ||
| } | ||
|
|
||
| [TestCase(17, 2, true, "1.2.3", TestName = "multiple dots")] | ||
| [TestCase(17, 2, true, "--123", TestName = "double minus")] | ||
|
There was a problem hiding this comment. Есть проверка на двойной минус, но не на плюс. Почему решил добавить её?
Author
There was a problem hiding this comment. Хотел проверить двойной знак перед числом. Подумал, что с "++" будет повторяющийся тест. |
||
| [TestCase(17, 2, true, "++123", TestName = "double plus")] | ||
| [TestCase(17, 2, false, "123..45", TestName = "double dots")] | ||
| [TestCase(17, 2, true, "12-3", TestName = "minus inside number")] | ||
| [TestCase(17, 2, true, "+-123", TestName = "mixed signs")] | ||
| [TestCase(17, 2, false, "12a.3", TestName = "Invalid character in number")] | ||
| [TestCase(17, 2, false, "\n1.23", TestName = "Special character before")] | ||
| [TestCase(17, 2, true, "1234.", TestName = "Trailing dot without fractional part")] | ||
| [TestCase(17, 2, false, ".12", TestName = "Leading dot without integer part")] | ||
| [TestCase(17, 2, true, "-123", TestName = "Negative number with onlyPositive=true")] | ||
|
There was a problem hiding this comment. Тут хорошим тестом видится тест на 0 с разными знаками |
||
| [TestCase(17, 0, true, "-0", TestName = "Negative zero with onlyPositive=true")] | ||
| [TestCase(1, 0, false, "+0", TestName = "Positive zero with precision 1")] | ||
| [TestCase(1, 0, false, "", TestName = "Empty string")] | ||
| [TestCase(1, 0, false, null, TestName = "Null string")] | ||
| [TestCase(3, 0, false, "a", TestName = "Not a number")] | ||
| [TestCase(3, 0, false, " 1", TestName = "Space before the number")] | ||
| [TestCase(3, 0, false, "1 ", TestName = "Space after the number")] | ||
| [TestCase(3, 0, false, "1 1", TestName = "Space inside the number")] | ||
| public void IsValidNumber_Should_Return_False_When_Invalid_Numbers(int precision, int scale, bool onlyPositive, string value) | ||
| { | ||
| var validator = new NumberValidator(precision, scale, onlyPositive); | ||
|
|
||
| var actualResult = validator.IsValidNumber(value); | ||
|
|
||
| actualResult.Should().BeFalse(); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"> | ||
| <s:String x:Key="/Default/CodeInspection/ExcludedFiles/FilesAndFoldersToSkip2/=7020124F_002D9FFC_002D4AC3_002D8F3D_002DAAB8E0240759_002Ff_003AAssertionChain_002Ecs_002Fl_003A_002E_002E_003F_002E_002E_003F_002E_002E_003FAppData_003FRoaming_003FJetBrains_003FRider2025_002E2_003Fresharper_002Dhost_003FSourcesCache_003Fc022dff8ceab1a21e3e96b991f8736b3ab0d2a79db4ad60795485d39280_003FAssertionChain_002Ecs/@EntryIndexedValue">ForceIncluded</s:String> | ||
| <s:String x:Key="/Default/CodeInspection/ExcludedFiles/FilesAndFoldersToSkip2/=7020124F_002D9FFC_002D4AC3_002D8F3D_002DAAB8E0240759_002Ff_003ACompositeWorkItem_002Ecs_002Fl_003A_002E_002E_003F_002E_002E_003F_002E_002E_003FAppData_003FRoaming_003FJetBrains_003FRider2025_002E2_003Fresharper_002Dhost_003FDecompilerCache_003Fdecompiler_003F66eca7339e514aca875e7e128c50570880600_003F25_003F073b6a06_003FCompositeWorkItem_002Ecs/@EntryIndexedValue">ForceIncluded</s:String> | ||
| <s:String x:Key="/Default/CodeInspection/ExcludedFiles/FilesAndFoldersToSkip2/=7020124F_002D9FFC_002D4AC3_002D8F3D_002DAAB8E0240759_002Ff_003ALateBoundTestFramework_002Ecs_002Fl_003A_002E_002E_003F_002E_002E_003F_002E_002E_003FAppData_003FRoaming_003FJetBrains_003FRider2025_002E2_003Fresharper_002Dhost_003FSourcesCache_003Ff9878f643c2ecf1b4ce73234de56c9754a451c835620eae85ca8babbcb5889c4_003FLateBoundTestFramework_002Ecs/@EntryIndexedValue">ForceIncluded</s:String> | ||
| <s:String x:Key="/Default/CodeInspection/ExcludedFiles/FilesAndFoldersToSkip2/=7020124F_002D9FFC_002D4AC3_002D8F3D_002DAAB8E0240759_002Ff_003AObjectAssertions_002Ecs_002Fl_003A_002E_002E_003F_002E_002E_003F_002E_002E_003FAppData_003FRoaming_003FJetBrains_003FRider2025_002E2_003Fresharper_002Dhost_003FSourcesCache_003Fcde5ccd369841c3855ce75617c91cb69a307cdd9958f7add3e43fabab82bc6_003FObjectAssertions_002Ecs/@EntryIndexedValue">ForceIncluded</s:String> | ||
| <s:String x:Key="/Default/CodeInspection/ExcludedFiles/FilesAndFoldersToSkip2/=7020124F_002D9FFC_002D4AC3_002D8F3D_002DAAB8E0240759_002Ff_003AWorkItem_002Ecs_002Fl_003A_002E_002E_003F_002E_002E_003F_002E_002E_003FAppData_003FRoaming_003FJetBrains_003FRider2025_002E2_003Fresharper_002Dhost_003FDecompilerCache_003Fdecompiler_003F66eca7339e514aca875e7e128c50570880600_003Fb3_003Fb7fabac5_003FWorkItem_002Ecs/@EntryIndexedValue">ForceIncluded</s:String> | ||
| <s:String x:Key="/Default/Environment/UnitTesting/UnitTestSessionStore/Sessions/=25b42caf_002Debb7_002D49b4_002Db7b5_002Dd35530da5665/@EntryIndexedValue"><SessionState ContinuousTestingMode="0" IsActive="True" Name="All tests from ObjectComparison.cs" xmlns="urn:schemas-jetbrains-com:jetbrains-ut-session">
 | ||
| <Project Location="C:\Users\MihailK\Desktop\shpora-testing\Testing\Basic" Presentation="&lt;Basic&gt;" />
 | ||
| </SessionState></s:String> | ||
| <s:String x:Key="/Default/Environment/UnitTesting/UnitTestSessionStore/Sessions/=8aa1bf41_002D3bcb_002D486e_002D96f3_002D3ee4aeed767d/@EntryIndexedValue"><SessionState ContinuousTestingMode="0" Name="Session" xmlns="urn:schemas-jetbrains-com:jetbrains-ut-session">
 | ||
| <Or>
 | ||
| <ProjectFile>6ED454CB-A772-43E5-B72D-3FE9DA27337F/d:Homework/d:1. ObjectComparison/f:ObjectComparison.cs</ProjectFile>
 | ||
| <ProjectFolder>6ED454CB-A772-43E5-B72D-3FE9DA27337F/d:Homework/d:2. NumberValidator</ProjectFolder>
 | ||
| </Or>
 | ||
| </SessionState></s:String> | ||
| </wpf:ResourceDictionary> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Для того чтобы понять, хорошо ли новый способ сравнения работает можно было бы написать тесткейсы с разными случаями. Здесь я тебя не прошу этого сделать, но вообще это могло помочь в среднем обнаружить случаи, которые не отсмотрены с текущими проверками. Сейчас в текущем способе проверки есть проблема, попробуй её таким образом воспроизвести.