В статье про тестирование public-методов коснулся юнит-тестирования приватной логики классов. Думаю, мне стоило бы переделать тезис, так как большинство, на мой взгляд, восприняло, что речь идет о тестировании именно private-методов, хотя речь шла о приватной логике. В этой статье хочу проиллюстрировать практическим примером главный тезис. Под катом пример с небольшим анализом.
Пример с запашком
Для того, чтобы проиллюстрировать проблему, придумал пример. Идея его взята из одного реального проекта. В идеале, как кто-то может заметить, класс надо было бы написать иначе. Но сейчас никто не станет его переписывать (или рефакторить), т.к. это приведет к большим затратам времени на правку и тестирование того, что и так работает. Поэтому, это не будет одобрено. При этом изменять код надо и изменения должны быть правильными. И так, вот код. Важная логика сосредоточена в методе ProcessMessage. Задача: внедрить новый флаг бизнесс-логики в обработку сообщений. Этот флаг имеет нетривиальную логику.
Как будете внедрять и тестировать флаг?
using System;
using System.Threading.Tasks;
using System.Threading;
using System.Messaging;
namespace PublicMethodNottrivialSample
{
public class MessageProcessorService
{
private object _lockObject = new object();
private CancellationTokenSource _cancellationSource;
private Action _receiveMessage;
private int _listenerThreads = 5;
public void Start()
{
lock (_lockObject)
{
_cancellationSource = new CancellationTokenSource();
Task.Factory.StartNew(() => InitializeReceiveLoop(_cancellationSource.Token), _cancellationSource.Token);
}
}
private void InitializeReceiveLoop(CancellationToken cancellationToken)
{
_receiveMessage = () =>
{
while (!cancellationToken.IsCancellationRequested)
{
using (MessageQueueTransaction msgTx = new MessageQueueTransaction())
{
try
{
msgTx.Begin();
// получить сообщение
MessageQueue queue = new MessageQueue();
Message message = queue.Receive(msgTx);
// проверить остановлен ли процесс, пока получали сообщение
if (!cancellationToken.IsCancellationRequested)
{
ProcessMessage(message);
}
msgTx.Commit();
}
catch (Exception ex)
{
// some logging
}
}
}
};
for (int n = 0; n < _listenerThreads; n++)
{
StartProcessingLoop(cancellationToken);
}
}
private void ProcessMessage(Message message)
{
// некоторый важный функционал, куда внедряется новый флаг
}
private void StartProcessingLoop(CancellationToken cancellationToken)
{
Task.Factory.StartNew(_receiveMessage, cancellationToken);
}
}
}
В классе видно, что единственный public-метод — это Start(). Его можно протестировать, если изменить сигнатуру, но в этом случае изменится публичный интерфейс. Кроме того, потребуется менять несколько методов, чтобы возвращать запущенные потоки и потом ожидать их завершения в тесте.
Но, как помним, требование касалось только внедрения флага в процесс обработки сообщения, и не предполагало изменение работы механизма получения сообщений. Поэтому, кто бы ни вносил изменения, я ожидал бы, что исправлен будет только один метод, а юнит-тест имел бы отношение только к изменяемой логике. Добиться этого, оставаясь в рамках принципа сложно: тест получится нетривиальным, что повлечет либо отказ от его написания, либо усложненный код. Вот так и начинает подванивать тестирование через публичный метод. И кто-то потом эмоционально напишет: «TDD — это долго», «Заказчик не оплачивает», или «Тесты плохо работают».
Вообще, тестировать такой код надо, но не юнит-тестами, а интеграционными.
«Вам шашечки, или ехать»
Безусловно, написать юнит-тест на изменённую логику необходимо. Считаю, что дилемма, в данном случае, состоит в выборе наименее затратного способа написания теста, а не полезного кода. Тут я имею виду тот факт, что что бы вы ни делали: рефакторинг для public-метода или другое решение — вы будете делать это с целью написать тест, а не решить требование из задачи клиента. В этом случае, целесообразно оценить затраты и эффект. В дополнение к выше указанному решению с рефакторингом существует еще несколько альтернативных решений. Привожу все, ниже обсудим За и Против:
- можно тестировать private-метод
- можно метод сделать публичным
- можно сделать внутренним (internal)
- можно вытащить в отдельный класс, как публичный метод
Если сравнить первые три способа с решением через публичный метод, но все они требуют меньше трудозатрат (с private не факт). Так же, все они, по сути, являются одинаковым решением, с небольшими стилистическими отличиями. Т.к. результат будет получен любым способом, поэтому, на мой взгляд, выбор в пользу одного из этих решений, должен опираться не на технические возможности языка, а на то, что вы хотите показать другим разработчикам:
- если метод можно запускать из любого места в солюшене и он является частью поведения класса, то делайте его публичным и вытягивайте в интерфейс класса;
- если метод можно использовать из любого другого класса, но только внутри данной сборки, то делайте его internal;
- если метод можно использовать только внутри основного класса, где он может быть вызван из любого другого метода, то делайте его protected internal.
Internal-методы можно сделать видимыми сборке с тестами с помощью атрибута InternalsVisibleTo и вызывать, как обычно. Такой подход упрощает написание тестов, а результат будет тем же.
Тестируем ProcessMessage
Вернемся к задаче. Руководствуясь подходом выше я внёс бы несколько изменений:
- сделал бы ProcessMessage публичным
- создал бы новый protected internal метод для логики флага (например, GetFlagValue)
- написал бы тесты для GetFlagValue на все случаи
- написал бы тест для ProcessMessage, чтобы убедиться, что GetFlagValue правильно используется: правильно передаются параметры и он действительно используется
Уточню, что я не стал бы писать юнит-тесты на все случаи использования GetFlagValue в методе ProcessMessage с условием, что я протестировал эти случаи в юнит-тестах GetFlagValue. В случае обнаружения непокрытых случаев, их необходимо дописывать. При этом основной подход остается пржним:
- все случаи покрываются юнит-тестами к GetFlagValue
- юнит-тесты ProcessMessage проверяют правильное использование метода флага
Считаю, что в соответствии с этим можно написать только один юнит-тест для ProcessMessage и несколько для GetFlagValue.
Как то так. Ваши мнения?
Автор: Aleksandr Goida
