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

в 13:42, , рубрики: .net, software development, tdd

В статье про тестирование 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.

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

Автор: Aleksandr Goida

Источник


* - обязательные к заполнению поля


https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js