Галичев Артем #213
Галичев Артем #213
Conversation
There was a problem hiding this comment.
Хорошей практикой считается разделять основной код и тесты на него в разных проектах. В каждом проекте будут только те зависимости, что нужны конкретному типу проекта, а также они начнут быстрее собираться и выполняться
Может быть на текущем объеме это будет незаметно, но когда у тебя проект на сотни тысяч строк кода и сотни тестов на него, становится уже заметнее))
ObjectPrinting/ObjectPrinting.csproj
Outdated
| @@ -5,6 +5,7 @@ | |||
| </PropertyGroup> | |||
|
|
|||
| <ItemGroup> | |||
| <PackageReference Include="FluentAssertions" Version="8.0.0-alpha.1" /> | |||
There was a problem hiding this comment.
Не сильно важный коммент, но альфы в рабочее решение тащить опасненько
ObjectPrinting/PrintingConfig.cs
Outdated
| typeof(int), | ||
| typeof(double), | ||
| typeof(float), | ||
| typeof(string), | ||
| typeof(DateTime), | ||
| typeof(TimeSpan) |
There was a problem hiding this comment.
Ещё забыл всякие uint, ulong, short и тд.
Но есть более простой способ узнать, является ли тип простым, не перечисляя все вариации простых типов
Небольшая подсказка: посмотри в сторону класса Type
| return this; | ||
| } | ||
|
|
||
| public PrintingConfig<TOwner> Excluding<TProperty>(Expression<Func<TOwner, TProperty>> memberSelector) |
There was a problem hiding this comment.
Сейчас у тебя возможно исключить только свойства класса, а на полях сериализация ломается. Попробуй в классе PersonWithParent сделать Id полем - тесты отвалятся, хотя поведение должно быть то же
ObjectPrinting/PrintingConfig.cs
Outdated
| public Dictionary<Type, Func<object, string>> TypeSerializers { get; } = []; | ||
| public Dictionary<Type, CultureInfo> Cultures { get; } = []; | ||
| public Dictionary<PropertyInfo, Func<object, string>> PropertySerializers { get; } = []; | ||
| public Dictionary<PropertyInfo, int> PropertiesMaxLength { get; } = []; |
There was a problem hiding this comment.
Давай закроем взаимодействие с этими свойствами хотя бы в рамках сборки, чтобы при ручном создании этого конфига нельзя было вмешиваться в работу программы руками меняя эти коллекции
| var person = new Person { Name = "John", Age = 25, Height = 175}; | ||
|
|
||
| var actualResult = ObjectPrinter.For<Person>().PrintToString(person); | ||
| var expectedResult = "Person\r\n\tId = Guid\r\n\tName = John\r\n\tHeight = 175\r\n\tAge = 25\r\n"; |
There was a problem hiding this comment.
На линуксе и макос твои тесты будут падать из-за \r\n (переноса строки на винде). Причем в коде ты юзаешь Environment.NewLine, а тут почему-то захардкодил явно значения
Относится ко всем тестам
|
|
||
| namespace ObjectPrinting.Tests; | ||
|
|
||
| [TestFixture] |
There was a problem hiding this comment.
[TestFixture] можно не писать, nunit и так поймет, что это класс с тестами по наличию [Test] или [TestCase] внутри него
| } | ||
|
|
||
| [Test] | ||
| public void PrintToString_ShouldReturnSerializedPerson_WithExcludedId() |
There was a problem hiding this comment.
Тут скорее не WithExcludedId, а WithExcludedProperty, чтобы более явно кейс из требований обозначить
| namespace ObjectPrinting.Tests; | ||
|
|
||
| [TestFixture] | ||
| public class ObjectPrinterTests |
There was a problem hiding this comment.
Классно поработал над покрытием тестами 🔥
| public class ObjectPrinterTests | ||
| { | ||
| [Test] | ||
| public void PrintToString_ShouldReturnSerializedPerson_WithAllProperties() |
There was a problem hiding this comment.
Есть корректировка по именованию тестов. В целом ты правильно выделил первые две части нейминга - метод и что он должен сделать, но сейчас нет последней части - а при каких условиях это происходит
Например, на этом тесте разберем
PrintToString - тестируемый метод
ShouldReturnSerializedPerson - что долежн вернуть. Окей, должен вернуть сериализованный класс
WithAllProperties - "со всеми свойствами". Но разве это условия выполнения? Это скорее дополнение к предыдущему пункту "Должен вернуть сериализованный класс со всеми свойствами"
То есть нет третьей части PrintToString_ShouldSerializeClassWithAllProperties_When...() (подсократил сразу, чтобы побольше влезло в when)
| { | ||
| public static PrintingConfig<TOwner> MaxLength<TOwner>( | ||
| this PropertyPrintingConfig<TOwner, string> propertyPrintingConfig, | ||
| int maxLen) |
@desolver