When testing through a public method starts to stink (example)

In the article about testing public methods, I touched on unit testing of private class logic. I think it would be worthwhile for me to redo the thesis, since the majority, in my opinion, perceived that it was a question of testing private methods, although it was a matter of private logic. In this article I want to illustrate with a practical example the main thesis. Under a cat an example with the small analysis.

An example with a smell

In order to illustrate the problem, came up with an example. His idea is taken from one real project. Ideally, as someone might notice, a class would have to be written differently. But now no one will rewrite it (or refactor), because this will lead to a great investment of time in editing and testing what works. Therefore, it will not be approved. At the same time it is necessary to change the code and the changes must be correct. And so, here is the code. The important logic is concentrated in the ProcessMessage method. Objective: to introduce a new flag of business logic in message processing. This flag has nontrivial logic.

How will you implement and test the flag?

using System;
using System.Threading.Tasks;
using System.Threading;
using System.Messaging;
namespacePublicMethodNottrivialSample
{
    publicclassMessageProcessorService
    {
        privateobject _lockObject = newobject();
        private CancellationTokenSource _cancellationSource;
        private Action _receiveMessage;
        privateint _listenerThreads = 5;
        publicvoidStart()
        {
            lock (_lockObject)
            {
                _cancellationSource = new CancellationTokenSource();
                Task.Factory.StartNew(() => InitializeReceiveLoop(_cancellationSource.Token), _cancellationSource.Token);
            }
        }
        privatevoidInitializeReceiveLoop(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);
            }
        }
        privatevoidProcessMessage(Message message)
        {
            // некоторый важный функционал, куда внедряется новый флаг
        }
        privatevoidStartProcessingLoop(CancellationToken cancellationToken)
        {
            Task.Factory.StartNew(_receiveMessage, cancellationToken);
        }
    }
}

In the class, it is clear that the only public method is Start (). You can test it if you change the signature, but in this case the public interface changes. In addition, you will need to change several methods to return running threads and then wait for them to end in the test.

But, as we remember, the requirement concerned only the introduction of a flag in the processing of a message, and did not imply a change in the operation of the mechanism for receiving messages. Therefore, no matter who makes the change, I would expect that only one method would be fixed, and the unit test would be related only to the logic being changed. To achieve this, remaining within the framework of the principle is difficult: the test will turn out to be non-trivial, which will entail either a refusal to write it or complicated code. This is how it begins to pitch testing through a public method. And then someone will write emotionally: “TDD is long”, “Customer does not pay”, or “Tests do not work well”.

In general, it is necessary to test such code, but not by unit tests, but by integration tests.

"You checkered, or go"

Of course, it is necessary to write a unit test for the changed logic. I believe that the dilemma, in this case, is to choose the least expensive way to write a test, and not a useful code. Here I mean the fact that whatever you do: refactoring for a public method or another solution - you will do it with the goal of writing a test, and not solving the requirement from the client’s task. In this case, it is advisable to evaluate the costs and effects. In addition to the above solution with refactoring, there are several other alternative solutions. I cite everything, discuss the pros and cons below:

1. you can test private method
2. method can be made public
3. can be made internal
4. can be pulled out in a separate class, as a public method

If we compare the first three ways with the solution through the public method, but they all require less labor (it’s not a private fact). Also, all of them, in fact, are the same solution, with minor stylistic differences. Because The result will be obtained in any way, therefore, in my opinion, the choice in favor of one of these solutions should be based not on the technical capabilities of the language, but on what you want to show other developers:

1. if the method can be run from any place in the solution and it is part of the class behavior, then make it public and pull into the class interface;
2. if the method can be used from any other class, but only inside this assembly, then make it internal ;
3. if the method can be used only inside the main class, where it can be called from any other method, then make it protected internal .

Internal methods can be made visible to the assembly with tests using the InternalsVisibleTo attribute and called as usual. This approach simplifies writing tests, and the result will be the same.

Testing ProcessMessage

Let's return to the task. Guided by the approach above, I would make a few changes:

• would make ProcessMessage public
• would create a new protected internal method for flag logic (for example, GetFlagValue)
• would write tests for GetFlagValue for all cases
• would write a test for ProcessMessage to make sure that GetFlagValue is used correctly: parameters are passed correctly and it is actually used

I’ll clarify that I wouldn’t write unit tests for all cases of using GetFlagValue in the ProcessMessage method with the condition that I tested these cases in GetFlagValue unit tests. In case of detection of uncovered cases, they must be added. In this case, the main approach remains przhny:

• all cases are covered by unit tests to getFlagValue
• ProcessMessage unit tests check for proper use of the flag method

I believe that in accordance with this, you can write only one unit test for ProcessMessage and several for GetFlagValue.

Something like this. Your opinions?