Conversation
| using NUnit.Framework; | ||
| using NUnit.Framework.Legacy; | ||
|
|
||
| namespace HomeExercise.Tasks.NumberValidator; |
There was a problem hiding this comment.
Не знаю, говорили ли вам, было бы круто вынести тесты в отдельный проект, чтобы код тестов не попадал вместе с библиотекой на продуктовую площадку.
There was a problem hiding this comment.
В код самого валидатора (NumberValidator) тоже можно заглянуть, может найдется что не так.
| ClassicAssert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent); | ||
| actualTsar.Should().BeEquivalentTo(expectedTsar, o => o | ||
| .Excluding(p => p.Id) | ||
| .Excluding(p => p.Parent.Id)); |
There was a problem hiding this comment.
Как быть, если там окажется целая династия, 5-10-20 царей?
Предлагаю прям попробовать.
Еще можно подумать: какие еще вариации на эту тему могут попасться?
There was a problem hiding this comment.
Да, проверил, тесты не падали, когда условный дед различался
И еще проблема, когда циклические ссылки есть и предков больше 10, т.к. по умолчанию глубина рекурсии в FLuentAssertions 10
Поправил
| [TestCase("++12.34", TestName = "MultipleSigns")] | ||
| [TestCase("12.", TestName = "FractionPartIsMissing")] | ||
| [TestCase(".34", TestName = "IntegerPartIsMissing")] | ||
| public void IsValidNumber_WithInvalidStringFormat_ShouldReturnFalse(string value) |
There was a problem hiding this comment.
Можно добавить смесь букв и цифр.
| { | ||
| var act = () => new NumberValidator(precision, scale); | ||
|
|
||
| act.Should().Throw<ArgumentException>(); |
There was a problem hiding this comment.
Здесь можно было бы проверить что вылетает именно то исключение, которые мы ожидаем, а не какое-то другое.
… homework/tests-refactoring
| [Description("Проверка текущего царя")] | ||
| [Category("ToRefactor")] | ||
| public void CheckCurrentTsar() | ||
| public void CheckTsarEquality_WithValidTsar_ShouldNotThrow() |
There was a problem hiding this comment.
Раз не получается в отдельное решение, давай тесты хотя бы в отдельный класс вынесем.
| } | ||
|
|
||
| private bool AreEqual(Person? actual, Person? expected) | ||
| private static Person CreateTsarWithWrongAncestor(int generations, int wrongAncestorLevel) |
There was a problem hiding this comment.
Посмотри, может для этого метода есть более подходящее место.
| actualTsar.Should().BeEquivalentTo(expectedTsar, o => o | ||
| .ExcludingMembersNamed(nameof(Person.Id)) | ||
| .IgnoringCyclicReferences() | ||
| .AllowingInfiniteRecursion()); |
There was a problem hiding this comment.
Круто что ты догадался про цикличность!
Как быть если у меня для объектов Person указано свойство типа class City { int Id; string Name; } и я бы хотел чтобы сравнение не прошло, если идентификатор города не совпадает?
There was a problem hiding this comment.
ага, поправил, теперь проверяю что type именно Person, добавил тест на проверку с City
| * 2.) Явно указываем, какие поля мы исключаем из проверки | ||
| * 3.) При добавлении новых полей в Person нужно провести минимальный рефакторинг(исключить из проверки | ||
| * или вообще ничего не трогать) | ||
| * 4.) Падаем с адекватной информацией об ошибке, в отличие от просто непонятного False |
There was a problem hiding this comment.
Напишем тут что-нибудь про рекурсию?
| if (scale < 0 || scale >= precision) | ||
| throw new ArgumentException("precision must be a non-negative number less or equal than precision"); | ||
| throw new ArgumentException("scale must be a non-negative number less than precision"); | ||
| numberRegex = new Regex(@"^([+-]?)(\d+)([.,](\d+))?$", RegexOptions.IgnoreCase); |
There was a problem hiding this comment.
Посмотри про RegexOptions.Compiled и, возможно, можно инстанс переиспользовать для всех NumberValidator?
2 participants
@lev_shisterov