Рефакторинг кода для unit тестирования

[dopusteam]

Что то тест с try-finally-try выглядит ужасно

[Andrey_Solomatin]

Поддерживаю. Автору рекомендую изучить документацию к тестовому фрймворку, во всех нормальных фрейморках есть механизм, как это сделать красиво.

[blowin]

Это легко можно решит используя IDisposable

public readonly struct DeleteFileScope : IDisposable
{
    private readonly IFileProvider _fileProvider;
    private readonly string _path;

    public DeleteFileScope(IFileProvider fileProvider, string path)
    {
        _fileProvider = fileProvider;
        _path = path;
    }

    public void Dispose()
    {
        try
        {
            _fileProvider.Delete(_path);
        }
        catch 
        {
        }
    }
}

Сам тест

[Theory]
[MemberData(nameof(TestData))]
public void Save(string expect, List<User> items)
{
    // Arrange
    const string filePath = "tmp.json";
    var fileProvider = new MemoryFileProvider();
    var repository = new UserRepository(filePath, fileProvider);

    using (new DeleteFileScope(fileProvider, filePath))
    {
        // Act
        repository.Save(items);
        var json = fileProvider.ToJson(filePath);
    
        // Assert
        Assert.Equal(expect, json);
    }
}

[dopusteam]

Все ещё ерунда какая то, если честно. Ещё и пустой catch.

[shai_hulud]

using var fileStream = new FileStream(
  "./file.dat", 
  FileMode.Create, 
  FileAccess.ReadWrite, 
  FileShare.Delete, 
  4096, 
  FileOptions.DeleteOnClose // <- magic
);

[ruomserg]

У меня есть вопрос — скажите, пожалуйста, с какой целью вы тестируете репозиторий? Причем, не реальный репозиторий — а с замокаными зависимостями ?! Какой класс ошибок вы хотите поймать своим тестом?

Дело в том, что каждая строчка тестов — на самом деле имеет цену. Цену за написание, цену за поддержку в актуальном состоянии, цену за запуск тестов при каждом билде и так далее. Соответственно, должно быть некоторое нетривиальное ПОВЕДЕНИЕ кода, которое мог бы проверить соответствующий тест. А что вы тут проверили? Как прохождение этого теста увеличивает вашу уверенность в том, что на проде боевая система будет работать как надо?

Одно дело, если репозиторий привязан к БД или ORM, и тестируется в этом комплексе. Тут вы хотя бы проверяете что везде где нужно поставили границы транзакций, аннотации и проч.

Также понятно, если бы у вас в репозитории были какие-то вычисляемые поля. Тут можно задать хранимые аттрибуты, и проверить что логика вычисления работает нормально.

Но тестировать голый репозиторий — это выше моего понимания. Нет, понятно что если работы нет, а что-то написать нужно — можно и так. Но это, в моем понимании, затраты без выхлопа годного.

Повторю простое и понятное определение юнит-тестов, которое когда-то нашел, и с тех пор запомнил применяю: «Юнит-тест — это проверка правильности поведения компонента, в предположении, что все другие компоненты от которых он зависит — работают правильно».

Соответственно, если у компонента нет нетривиального поведения — не нужно его тестировать. Если нетривиальное поведение есть, и оно является следствием работы другого компонента (например, репозиторий критически зависит от БД) — ну так и тестируйте его в типовой конфигурации тогда. А базу данных сделайте стандартным окружением для теста. И только если другие компоненты поставляют на вход теструемого тривиальные данные, а сложность заключается в логике их обработки — ну вот тогда и пускайтесь во все тяжкие: мокайте входные зависимости (благо, они должны быть тривиальными) — и проверяйте юнит-тестом логику обработки внутри тестируемого компонента.

Если же так сделать нельзя — значит не надо кодить неэффективные юнит-тесты. Делайте интеграционные. Если система спроектирована хорошо, то интеграционные тесты в достаточно небольшом количестве дают неплохое покрытие кода многих тестируемых компонентов. Да и во многих случаях, для выпуска в прод — вам не надо гарантировать правильное поведение компонентов. Вам надо, чтобы пользователь в типовых сценариях работы не натыкался на ошибки. А это как раз в большей степени ловит интеграция, нежели юниты.

Опять же, юнитами желательно тестировать логику, от которой ожидается что она влияет на целую кучу мест (неявным образом). Например, если у вас есть компонент, который генерирует коды EAN13 (с контрольной цифрой) — то затестить через юниты правильность генерации и проверки контрольной цифры — сам бог велел. Потому что вы это будете явно и неявно сто раз переиспользовать. А компонент, который вызывается явным образом в одной юзер-стори и один раз — так и тестировать тогда интеграционным тестом по этой самой стори…

[blowin]

Конечно пример надуманный, в реальной жизни вы вряд ли кто-то будет тестировать репозиторий, но вместо его может быть, например, получение курсов валют из сети. Цель статьи - показать проблемы и возможные способы их решения.

Никто не будет приводить сложный код для того, чтобы объяснить проблему. Пример должен уместиться в голове и быть простым для понимания и понятен каждому. Каждый в своём коде использует репозиторий и пример на нём банально нагляден.

[ruomserg]

Если у меня есть CurrencyExchangeRateProvider, то это, очевидно — интерфейс. И его тестировать не получится совсем. Если у меня есть сервис, который зависит от CurrencyExchangeRateProvider — то тогда мы действительно замокаем провайдер (скорее всего даже не обращаясь к файлам — а просто статическая тестовая реализация или генератор с seed — не обязательно случайный). И юнитом мы будем тестировать не ExchangeRateProvider — а тот самый сервис, который с этими rates что-то нетривиальное делает.

А вот если вы захотите проверить что у вас правильно работает провайдер, получающий данные из сети — вам придется мокать сеть и отвечающий сервер. Это тоже возможно, начиная от записи пакетов и их replay в тесте, заканчивая вполне-себе полноценным локальным сервером, с предопределенным ответами (а то еще и с каким-нибудь фаззингом в середине, чтобы понимать как вы реагируете на типичные сетевые проблемы типа пропадания или задвоения данных).

Я бы сказал, что беда тестирования заключается именно в том, что все хотят показать тестирование на вот таких простых случаях. Где оно не нужно. И народ потом пишет тесты ровно так как показали — тестируя (и успешно тестируя, чо!) компоненты без логики. А в итоге на релизах как сыпалось, так и сыпется… Тестировать тоже надо с умом…

[funca]

Отправлено-ли для репозитория абстрагировать доступ к файловой системе, или лучше было оставить как в первом варианте?

[ruomserg]

Давайте начнем с простого — если мы что-то абстрагируем, значит мы предполагаем что существует более одной (чтобы оправдать сложность — хотя бы три!) реализации этой абстракции. Соответственно, давайте поймем — можем ли мы придумать хотя бы три разных варианта реализации абстракции «файловая система»? Если не уходить в экзотику с магнитными лентами и перфокартами — очевидно, нет. Файловая система — это отраслевой стандарт и на всех платформах работает (или эмулируется нижним уровнем системы) примерно одинаково. Отсюда вывод — нет смысла абстрагировать доступ к файловой системе.

Не зная архитектуры, сложно давать советы — но я предположил бы, что в данном случае проблема даже не в тесте, а в архитектуре. Репозиторий должен быть объявлен как интерфейс. А дальше, должна быть сделана реализация JSONFileUserRepository. А также, может быть сделана StaticUserRepository (закодированные юзеры прямо в методах), OracleDBUserRepository, и так далее (если это требуется)…

[Andrey_Solomatin]

С существующей реализацией можно легко написать интеграционные тесты на наш репозиторий. Для этого добавим 1 метод в IFileProvider

Добавлять новые публичные методы для тестов считается дурным тоном.

Юниттесты лежат в том-же пакете и для них можно добавить непубличные методы.
Для интеграционных проверяйте через доступное API.

[blowin]

Согласен, лучше было бы в тесте написать код для удаления файла, а не менять public api

[Death666Lich]

Для решения проблемы абстрагирования от файловой системы я бы предложил готовое решение https://github.com/TestableIO/System.IO.Abstractions которое в точности направлено на это и имеет набор тестовых моков для файлов, директорий, и эмулирует реальную файловую систему в памяти. Так же для удобства использования данной библиотеки есть пакет с Roslyn анализатором для поиска по существующему коду использования классов из пространства имен System.IO: https://github.com/TestableIO/System.IO.Abstractions.Analyzers