Сазонов Александр #220
Сазонов Александр #220
Conversation
There was a problem hiding this comment.
Круто, что сразу растащил по разным проектам основной код и тесты
ObjectPrintingTests/Person.cs
Outdated
| @@ -0,0 +1,14 @@ | |||
| namespace ObjectPrintingTests; | |||
|
|
|||
| public class Person(Guid id, string name, double height, int age, DateTime birthDate, Person? parent = null) | |||
There was a problem hiding this comment.
А зачем тебе и primary конструктор, и свойства, которые он инициализирует?
Почему не оставишь что-то одно?
|
|
||
| namespace ObjectPrintingTests; | ||
|
|
||
| [TestFixture] |
There was a problem hiding this comment.
[TestFixture] можно не писать, nunit и так поймет, что это класс с тестами по наличию [Test] или [TestCase] внутри него
| fullText | ||
| .Should() | ||
| .Contain(expectedNameString); |
There was a problem hiding this comment.
Тест на самом деле ничего не проверяет: если метод TrimmedToLength ничего не сделает и оставит Name с исходной длиной, например Alexander, то в expectedNameString будет лежать Alex, который является подстрокой Alexander даже если он напечатается полностью
И тут же в целом комментарий почти ко всем тестам класса - Contain не очень полезная проверка, ведь тот же Alex может являться подстрокой совсем другого элемента сериализованной строки, но тест будет зеленым
Аргумент "но ведь мы сами контролируем данные теста" будет лишь отчасти верным, потому что используемый тобой класс Person могут начать использовать в других тестах, добавить туда свойств, которые как раз и приведут к обозначенной выше проблеме. Когда тестов в проекте очень много поддерживать их становится непросто и периодически больно, поэтому полагаться на случай, что конфиг теста останется неприкосновенным нельзя
| .PrintToString(alex); | ||
| var expectedAgeString = alex.BirthDate.ToString(CultureInfo.InvariantCulture); | ||
|
|
||
| using (new AssertionScope()) |
There was a problem hiding this comment.
Можно убрать вложенность using var _ = new AssertionScope()
ObjectPrinting/PrintingConfig.cs
Outdated
| internal readonly Dictionary<Type, Delegate> TypeSerializationMethod = new(); | ||
| internal readonly Dictionary<MemberInfo, Delegate> MemberSerializationMethod = new(); |
There was a problem hiding this comment.
Кажется, что можно взять более точные типы вместо Delegate. Что нам нужно по факту от этой функции? Чтобы она некоторый тип приводила к строке. Поэтому можно раскрутить что-то типа Func<..., string>, тогда не надо будет писать DynamicInvoke при вызове этих функций
ObjectPrinting/PrintingConfig.cs
Outdated
| internal readonly Dictionary<Type, Delegate> TypeSerializationMethod = new(); | ||
| internal readonly Dictionary<MemberInfo, Delegate> MemberSerializationMethod = new(); |
There was a problem hiding this comment.
Круто, что сделал сразу internal и позаботился о защите доступа извне 🔥
| if (obj is IEnumerable enumerable) | ||
| sb.Append(GetCollectionString(enumerable, numberOfTabs, nestingLevel)); | ||
| else | ||
| sb.Append(GetSingleElementString(obj, numberOfTabs, nestingLevel)); |
There was a problem hiding this comment.
К слову о надежности тестов, попробовал засериализовать словарь
Получил такое значение (ещё с decimal и byte что-то, но это скорее всего от того, что ты их не добавил в FinalTypes)
Person
Height = 182
BirthDate = 12/14/2024 00:00:00
Some = Dictionary`2
[
KeyValuePair`2
Key = firstValue
Value = one
KeyValuePair`2
Key = secondValue
Value = two
KeyValuePair`2
Key = thirdValue
Value = three
]
Parent = null
Name = Alex
BestField = Decimal
Scale = Byte
|
|
||
| public class PropertyPrintingConfig<TOwner, TPropType>( | ||
| PrintingConfig<TOwner> printingConfig, | ||
| MemberInfo? memberInfo = null) |
There was a problem hiding this comment.
Тут что-то странное вышло: говорим в конструкторе, что можем принимать null, но при этом позже при обращении к PropertyMemberInfo, если он null, кинем исключение
| ]; | ||
|
|
||
| [Test] | ||
| public void NotThrowingStackOverflowException_WhenPrintToStringHasRecursion() |
There was a problem hiding this comment.
Давай докрутим немного нейминг тестов:
- В первой части (до первого подчеркивания) мы пишем обычно то, какой метод сущности мы тестируем. В нашем случае
PrintToString - Во второй части пишем, что должен сделать метод в ответ на наши входные параметры теста. Как ты уже написал
NotThrowingStackOverflowException(только Shoud добавить в начало, тогда можно будет написатьShouldNotThrow...) илиShouldSerialize...и тд - Третья часть обычно начинается с
..._When...или..._On...и отвечает за то, при каких условиях тест должен сделать то, то написано во втором пункте. Например, как ты уже хорошо это сделал в этом тестеWhenPrintToStringHasRecursion. Надо так же добавить и в другие
There was a problem hiding this comment.
Идея нейминга была в том, чтобы читать название метода вместе с названием класса. То есть PrintingConfigShould_NotThrowingStackOverflowException_WhenPrintToStringHasRecursion
There was a problem hiding this comment.
Блин, моя ошибка, не прочитал название класса
@desolver