Количество дураков уменьшается, но качество их растет.
Михаил Генин
Цель
Цель статьи - показать как можно решить проблему при написании юнит тестов, когда в коде есть зависимости на внешний ресурс (база данных/сеть/файловая система).
Введение
Хотел бы опубликовать статью о unit тестировании из своего блога.
Для начала разберемся что же такое unit тестирование. Cогласно википедии:
Модульное тестирование, иногда блочное тестирование или юнит-тестирование (англ. unit testing) — процесс в программировании, позволяющий проверить на корректность отдельные модули исходного кода программы, наборы из одного или более программных модулей вместе с соответствующими управляющими данными, процедурами использования и обработки.
Честно говоря, описание из википедии кажется мне абстрактным. Так как не передаёт важное отличие интеграционных тестов от unit.
Я бы дал следующее определение для unit тестирования:
Unit тестирование - процесс в программировании, позволяющий проверить на корректность отдельные модули исходного кода программы, который работает в оперативной памяти и не взаимодействует с внешними источниками (файловая система, база данных, сеть и т.д).
Проблема
Необходимо протестировать алгоритм группировки файлов по расширению.
public class FileGrouper
{
private readonly string _folderPath;
public FileGrouper(string folderPath) => _folderPath = folderPath;
public void ByExtension()
{
var groupFiles = Directory.GetFiles(_folderPath)
.Select(fullFileName => new
{
FullFileName = fullFileName,
FileName = Path.GetFileName(fullFileName),
Extension = Path.GetExtension(fullFileName)
})
.Where(fileInfo => !string.IsNullOrEmpty(fileInfo.Extension))
.ToLookup(arg => arg.Extension);
foreach (var groupFile in groupFiles)
{
var newFolder = Path.Combine(_folderPath, groupFile.Key);
if (!Directory.Exists(newFolder))
Directory.CreateDirectory(newFolder);
foreach (var file in groupFile)
{
var newFile = Path.Combine(newFolder, file.FileName);
File.Move(file.FullFileName, newFile);
}
}
}
}
На данный момент мы не можем написать unit тесты на этот код, так как в нём есть зависимость на файловую систему.
Решение
Есть 2 основных способа избавиться от этой зависимости:
1. Создать интерфейс для работы с файловой системой IFileProvider, в котором будут все необходимые операции.
2. Создать базовый класс FileGrouperBase с абстрактными методами по обращению к файловой системе, а уже каждый наследник будет решать как их реализовать.
Лично мне импонирует первый вариант, так как он является более гибким. Тем более, что нам необходимо больше 1 метода для работы с файловой системой, и удобнее будет иметь для этого отдельную сущность.
IFileSystem
Давайте опишем IFileSystem.
public interface IFileSystem
{
string[] GetFiles(string directory);
bool DirectoryExists(string directory);
void CreateDirectory(string directory);
void MoveFile(string sourceFile, string destinationFile);
}
Сразу же пишем реализацию для реальной файловой системы:
public class PhysicianFileSystem : IFileSystem
{
public string[] GetFiles(string directory)
=> Directory.GetFiles(directory);
public bool DirectoryExists(string directory)
=> Directory.Exists(directory);
public void CreateDirectory(string directory)
=> Directory.CreateDirectory(directory);
public void MoveFile(string sourceFile, string destinationFile)
=> File.Move(sourceFile, destinationFile);
}
Дорабатываем FileGrouper
Изменения будут минимальными, заменяем все статические вызовы File и Directory на вызовы нового объекта.
public class FileGrouper
{
private readonly string _folderPath;
// Добавили объект ↓
private readonly IFileSystem _fileSystem;
public FileGrouper(string folderPath, IFileSystem fileSystem)
{
_folderPath = folderPath;
_fileSystem = fileSystem;
}
public void ByExtension()
{
// Меняем вызов ↓
var groupFiles = _fileSystem.GetFiles(_folderPath)
.Select(fullFileName => new
{
FullFileName = fullFileName,
FileName = Path.GetFileName(fullFileName),
Extension = Path.GetExtension(fullFileName)
})
.Where(fileInfo => !string.IsNullOrEmpty(fileInfo.Extension))
.ToLookup(arg => arg.Extension);
foreach (var group in groupFiles)
{
var newFolder = Path.Combine(_folderPath, group.Key);
// Меняем вызовы ↓
if (!_fileSystem.DirectoryExists(newFolder))
_fileSystem.CreateDirectory(newFolder);
foreach (var file in group)
{
var newFile = Path.Combine(newFolder, file.FileName);
// Меняем вызов ↓
_fileSystem.MoveFile(file.FullFileName, newFile);
}
}
}
}
Этого достаточно, чтобы можно было заменить работу с файловой системы на оперативную память или любой другой источник.
Пишем тесты
В качестве библиотеки для тестирования будем использовать xUnit. Это дело вкуса, можно использовать любую другую библиотеку для unit тестирования.
Для работы тестов, нам понадобится написать отдельную реализацию IFileSystem.
public sealed class MemoryFileSystem : IFileSystem
{
private readonly List<string> _availableFiles;
public MemoryFileSystem(IEnumerable<string> availableFiles)
=> _availableFiles = new List<string>(availableFiles);
public List<(string Source, string Destination)> MoveDetail { get; }
= new();
public string[] GetFiles(string directory)
=> _availableFiles.ToArray();
public bool DirectoryExists(string directory) => true;
public void CreateDirectory(string directory){}
public void MoveFile(string sourceFile, string destinationFile)
=> MoveDetail.Add((sourceFile, destinationFile));
}
Так как алгоритм оперирует путями к файлам, то нам достаточно сохранить путь файла который хотят переместить и место, куда он будет переносен алгоритмом.
MemberData
Данные для тестирования выглядят следующим образом
new List<object[]>
{
new object[]
{
@"D:\folder",
new List<string>(),
new List<(string Source, string Destination)>()
},
new object[]
{
@"D:\folder2",
new List<string>
{
@"D:\folder2\1.txt",
@"D:\folder2\2.txt",
@"D:\folder2\test.mp3",
@"D:\folder2\test2.png",
},
new List<(string Source, string Destination)>
{
(@"D:\folder2\1.txt", @"D:\folder2\.txt\1.txt"),
(@"D:\folder2\2.txt", @"D:\folder2\.txt\2.txt"),
(@"D:\folder2\test.mp3", @"D:\folder2\.mp3\test.mp3"),
(@"D:\folder2\test2.png", @"D:\folder2\.png\test2.png")
}
},
new object[]
{
@"D:\folder3\tmp",
new List<string>
{
@"D:\folder3\tmp\1.txt",
@"D:\folder3\tmp\2.txt",
@"D:\folder3\tmp\3",
@"D:\folder3\tmp\test.png",
@"D:\folder3\tmp\test2.png",
},
new List<(string Source, string Destination)>
{
(@"D:\folder3\tmp\1.txt", @"D:\folder3\tmp\.txt\1.txt"),
(@"D:\folder3\tmp\2.txt", @"D:\folder3\tmp\.txt\2.txt"),
(@"D:\folder3\tmp\test.png", @"D:\folder3\tmp\.png\test.png"),
(@"D:\folder3\tmp\test2.png", @"D:\folder3\tmp\.png\test2.png")
}
},
};
Проверяем группировку
[Theory]
[MemberData(nameof(TestData))]
public void Should_group_files_with_extension(string path,
List<string> availableFiles,
List<(string Source, string Destination)> expect)
{
// Arrange
var fileSystem = new MemoryFileSystem(availableFiles);
var group = new FileGrouper(path, fileSystem);
// Act
group.ByExtension();
var actual = fileSystem.MoveDetail;
// Assert
Assert.Equal(expect, actual);
}
Итог
Тестирование кода помогает помочь в проектировании класса и улучшить дизайн. Порой это приводит к избыточности, но это слихвой покрывется надёжностью кода.
Благодаря тестированию, процесс разработки идёт куда быстрее, так как разработчики меньше боятся поломать существующий функционал, например, очередным рефакторингом или добавлением новой функциональности.
Ссылки
UPD
Спасибо всем за комментарии, провёл работу над ошибками
Комментарии (12)
ruomserg
10.10.2022 19:14+10У меня есть вопрос — скажите, пожалуйста, с какой целью вы тестируете репозиторий? Причем, не реальный репозиторий — а с замокаными зависимостями ?! Какой класс ошибок вы хотите поймать своим тестом?
Дело в том, что каждая строчка тестов — на самом деле имеет цену. Цену за написание, цену за поддержку в актуальном состоянии, цену за запуск тестов при каждом билде и так далее. Соответственно, должно быть некоторое нетривиальное ПОВЕДЕНИЕ кода, которое мог бы проверить соответствующий тест. А что вы тут проверили? Как прохождение этого теста увеличивает вашу уверенность в том, что на проде боевая система будет работать как надо?
Одно дело, если репозиторий привязан к БД или ORM, и тестируется в этом комплексе. Тут вы хотя бы проверяете что везде где нужно поставили границы транзакций, аннотации и проч.
Также понятно, если бы у вас в репозитории были какие-то вычисляемые поля. Тут можно задать хранимые аттрибуты, и проверить что логика вычисления работает нормально.
Но тестировать голый репозиторий — это выше моего понимания. Нет, понятно что если работы нет, а что-то написать нужно — можно и так. Но это, в моем понимании, затраты без выхлопа годного.
Повторю простое и понятное определение юнит-тестов, которое когда-то нашел, и с тех пор запомнил применяю: «Юнит-тест — это проверка правильности поведения компонента, в предположении, что все другие компоненты от которых он зависит — работают правильно».
Соответственно, если у компонента нет нетривиального поведения — не нужно его тестировать. Если нетривиальное поведение есть, и оно является следствием работы другого компонента (например, репозиторий критически зависит от БД) — ну так и тестируйте его в типовой конфигурации тогда. А базу данных сделайте стандартным окружением для теста. И только если другие компоненты поставляют на вход теструемого тривиальные данные, а сложность заключается в логике их обработки — ну вот тогда и пускайтесь во все тяжкие: мокайте входные зависимости (благо, они должны быть тривиальными) — и проверяйте юнит-тестом логику обработки внутри тестируемого компонента.
Если же так сделать нельзя — значит не надо кодить неэффективные юнит-тесты. Делайте интеграционные. Если система спроектирована хорошо, то интеграционные тесты в достаточно небольшом количестве дают неплохое покрытие кода многих тестируемых компонентов. Да и во многих случаях, для выпуска в прод — вам не надо гарантировать правильное поведение компонентов. Вам надо, чтобы пользователь в типовых сценариях работы не натыкался на ошибки. А это как раз в большей степени ловит интеграция, нежели юниты.
Опять же, юнитами желательно тестировать логику, от которой ожидается что она влияет на целую кучу мест (неявным образом). Например, если у вас есть компонент, который генерирует коды EAN13 (с контрольной цифрой) — то затестить через юниты правильность генерации и проверки контрольной цифры — сам бог велел. Потому что вы это будете явно и неявно сто раз переиспользовать. А компонент, который вызывается явным образом в одной юзер-стори и один раз — так и тестировать тогда интеграционным тестом по этой самой стори…blowin Автор
10.10.2022 20:30Конечно пример надуманный, в реальной жизни вы вряд ли кто-то будет тестировать репозиторий, но вместо его может быть, например, получение курсов валют из сети. Цель статьи - показать проблемы и возможные способы их решения.
Никто не будет приводить сложный код для того, чтобы объяснить проблему. Пример должен уместиться в голове и быть простым для понимания и понятен каждому. Каждый в своём коде использует репозиторий и пример на нём банально нагляден.
ruomserg
10.10.2022 21:22+2Если у меня есть CurrencyExchangeRateProvider, то это, очевидно — интерфейс. И его тестировать не получится совсем. Если у меня есть сервис, который зависит от CurrencyExchangeRateProvider — то тогда мы действительно замокаем провайдер (скорее всего даже не обращаясь к файлам — а просто статическая тестовая реализация или генератор с seed — не обязательно случайный). И юнитом мы будем тестировать не ExchangeRateProvider — а тот самый сервис, который с этими rates что-то нетривиальное делает.
А вот если вы захотите проверить что у вас правильно работает провайдер, получающий данные из сети — вам придется мокать сеть и отвечающий сервер. Это тоже возможно, начиная от записи пакетов и их replay в тесте, заканчивая вполне-себе полноценным локальным сервером, с предопределенным ответами (а то еще и с каким-нибудь фаззингом в середине, чтобы понимать как вы реагируете на типичные сетевые проблемы типа пропадания или задвоения данных).
Я бы сказал, что беда тестирования заключается именно в том, что все хотят показать тестирование на вот таких простых случаях. Где оно не нужно. И народ потом пишет тесты ровно так как показали — тестируя (и успешно тестируя, чо!) компоненты без логики. А в итоге на релизах как сыпалось, так и сыпется… Тестировать тоже надо с умом…
funca
10.10.2022 20:45Отправлено-ли для репозитория абстрагировать доступ к файловой системе, или лучше было оставить как в первом варианте?
ruomserg
11.10.2022 06:44+1Давайте начнем с простого — если мы что-то абстрагируем, значит мы предполагаем что существует более одной (чтобы оправдать сложность — хотя бы три!) реализации этой абстракции. Соответственно, давайте поймем — можем ли мы придумать хотя бы три разных варианта реализации абстракции «файловая система»? Если не уходить в экзотику с магнитными лентами и перфокартами — очевидно, нет. Файловая система — это отраслевой стандарт и на всех платформах работает (или эмулируется нижним уровнем системы) примерно одинаково. Отсюда вывод — нет смысла абстрагировать доступ к файловой системе.
Не зная архитектуры, сложно давать советы — но я предположил бы, что в данном случае проблема даже не в тесте, а в архитектуре. Репозиторий должен быть объявлен как интерфейс. А дальше, должна быть сделана реализация JSONFileUserRepository. А также, может быть сделана StaticUserRepository (закодированные юзеры прямо в методах), OracleDBUserRepository, и так далее (если это требуется)…
Andrey_Solomatin
10.10.2022 20:18+4С существующей реализацией можно легко написать интеграционные тесты на наш репозиторий. Для этого добавим 1 метод в IFileProvider
Добавлять новые публичные методы для тестов считается дурным тоном.
Юниттесты лежат в том-же пакете и для них можно добавить непубличные методы.
Для интеграционных проверяйте через доступное API.blowin Автор
10.10.2022 20:32-1Согласен, лучше было бы в тесте написать код для удаления файла, а не менять public api
dopusteam
Что то тест с try-finally-try выглядит ужасно
Andrey_Solomatin
Поддерживаю. Автору рекомендую изучить документацию к тестовому фрймворку, во всех нормальных фрейморках есть механизм, как это сделать красиво.
blowin Автор
Это легко можно решит используя IDisposable
Сам тест
dopusteam
Все ещё ерунда какая то, если честно. Ещё и пустой catch.
shai_hulud