Когда тестирование через public-метод начинает вонять (пример)

В статье про тестирование 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-метода или другое решение — вы будете делать это с целью написать тест, а не решить требование из задачи клиента. В этом случае, целесообразно оценить затраты и эффект. В дополнение к выше указанному решению с рефакторингом существует еще несколько альтернативных решений. Привожу все, ниже обсудим За и Против:

1. можно тестировать private-метод
2. можно метод сделать публичным
3. можно сделать внутренним (internal)
4. можно вытащить в отдельный класс, как публичный метод

Если сравнить первые три способа с решением через публичный метод, но все они требуют меньше трудозатрат (с private не факт). Так же, все они, по сути, являются одинаковым решением, с небольшими стилистическими отличиями. Т.к. результат будет получен любым способом, поэтому, на мой взгляд, выбор в пользу одного из этих решений, должен опираться не на технические возможности языка, а на то, что вы хотите показать другим разработчикам:

1. если метод можно запускать из любого места в солюшене и он является частью поведения класса, то делайте его публичным и вытягивайте в интерфейс класса;
2. если метод можно использовать из любого другого класса, но только внутри данной сборки, то делайте его internal;
3. если метод можно использовать только внутри основного класса, где он может быть вызван из любого другого метода, то делайте его protected internal.

Internal-методы можно сделать видимыми сборке с тестами с помощью атрибута InternalsVisibleTo и вызывать, как обычно. Такой подход упрощает написание тестов, а результат будет тем же.

Тестируем ProcessMessage

Вернемся к задаче. Руководствуясь подходом выше я внёс бы несколько изменений:

• сделал бы ProcessMessage публичным
• создал бы новый protected internal метод для логики флага (например, GetFlagValue)
• написал бы тесты для GetFlagValue на все случаи
• написал бы тест для ProcessMessage, чтобы убедиться, что GetFlagValue правильно используется: правильно передаются параметры и он действительно используется

Уточню, что я не стал бы писать юнит-тесты на все случаи использования GetFlagValue в методе ProcessMessage с условием, что я протестировал эти случаи в юнит-тестах GetFlagValue. В случае обнаружения непокрытых случаев, их необходимо дописывать. При этом основной подход остается пржним:

• все случаи покрываются юнит-тестами к GetFlagValue
• юнит-тесты ProcessMessage проверяют правильное использование метода флага

Считаю, что в соответствии с этим можно написать только один юнит-тест для ProcessMessage и несколько для GetFlagValue.

Как то так. Ваши мнения?