Should you unit test order of operations? If so, how?

Question

Imagine a psedo-code class like this which does name comparisons:

public class NameComparer
{
    public bool DoNamesMatch(Name name1, Name name2)
    {
        if(WholeNameMatch(name1, name2))
        {
            return true;
        }

        return ProcessorIntensiveFuzzyMatcher(name1.Forename, name2.Forename)
                && ProcessorIntensiveFuzzyMatcher(name1.Surname, name2.Surname)
    }

    private bool WholeNameMatch(Name name1, Name name2)
    {
        return (name1.Forename == name2.Forename)
                && (name1.Surname == name2.Surname);
    }

    private bool ProcessorIntensiveFuzzyMatcher(string string1, string string2)
    {
        // complex fuzzy matching
    }
}

Best practice suggests that this class only ought to have one public method, and that we ought to unit test the private methods in this class through that public method.

While I can write various tests that ensure WholeNameMatch and ProcessorIntensiveFuzzyMatcher both do what they're supposed to do, the order of operation is important here. If a developer mistakenly moves one above the other, the program is going to take a massive performance hit.

Is it desirable - and possible - to test this order of operations via the exposed public method?

Answer 1

The premise of your question is incorrect. You don't want to test that first WholeNameMatch is called and then ProcessorIntensiveFuzzyMatcher is called.

You want to test

GIVEN WholeNameMatch returns true
WHEN NamesMatch is called
THEN ProcessorIntensiveFuzzyMatch is not called

This is easily done if the fuzzy matching is done by an injected dependency that is mocked by the unit test. Most mocking frameworks allow you to verify that a call did not occur.

If the fuzzy matcher is not an injected dependency, it should be.

Answer 2

@KevinCline gave an excellent answer, but I would like to add something one can do when injecting the fuzzy matcher does not seem worth the hassle, and to solve things in a more simple way.

One can simply add a public boolean read-only property LastMatchWasFuzzy to NameComparer, and initialize it this way:

public bool DoNamesMatch(Name name1, Name name2)
{
    LastMatchWasFuzzy=false;
    if(WholeNameMatch(name1, name2))
    {
        return true;
    }

    LastMatchWasFuzzy = ProcessorIntensiveFuzzyMatcher(name1.Forename, name2.Forename)
                     && ProcessorIntensiveFuzzyMatcher(name1.Surname, name2.Surname);
    return LastMatchWasFuzzy;
}

This gives the caller a chance to test how the last match was achieved, which can be beneficial not just for a unit test, but for other use cases as well.

In general, sometimes it is not the worst strategy to add some "maintenance hatches" to a class, which means properties or functions just for testing purposes (however, this is a always a tradeoff, these should not pollute the public API in a disturbing way).

Answer 3

Best practice suggests that this class only ought to have one public method, and that we ought to unit test the private methods in this class through that public method.

Best practice suggests that tests do not care about implementation details like private methods. Public methods are a way to expose functionality to the outside world, not a way to test private methods.

While I can write various tests that ensure WholeNameMatch and ProcessorIntensiveFuzzyMatcher both do what they're supposed to do,

Maybe you can but that is not what you want. You want to ensure that the public interface does what it is supposed to do. How it achieves that is not relevant to clients of your class and should not be relevant to your tests.

the order of operation is important here. If a developer mistakenly moves one above the other, the program is going to take a massive performance hit.

The order of private operations is meaningless to clients of your class, including tests, because they are not even aware that those operations exists, let alone in which order they are executed. If performance is important then test performance. Alternatively, follow kevin's advice about using dependency injection and the whole issue about private methods will disappear.

Is it desirable - and possible - to test this order of operations via the exposed public method?

Anything about private methods are implementation details. If you test implementation details and later you change implementation details your tests will fail. That is not desiderable, as long as the public interface provides the same functionality your tests should not break.

Answer 4

No.

Unit tests are supposed to let you safely refactor or rewrite code by showing everything still works as it is supposed to. But if the test code is coupled to the implementation details of the unit, you will have to modify the test at the same time when you change the code. Which means you could easily introduce errors, and the whole benefit of unit tests is lost.

In short, unit tests should verify the requirements of the unit are fulfilled, but not care about implementation details.

This is why it is bad practice to test private methods. Private methods are supposed to be implementation details. The same goes for verifying if a private method is called or not, or in what order.

In your case, the expected behavior is some level of fuzzy matching. What algorithm is used, and whether it is implemented in one or more methods is an implementation detail.

Imagine you find a better fuzzy matching algorithm which have the property that it is as fast or faster as a regular string match when there is an exact match. This allows you to skip the special casing of the exact match. This would be an improvement which preserved the expected behavior - but it would cause your test to fail! The test prevent you from improving the code.

Or you might realize that if Forename has an exact match but Surname does not, then there is no reason to perform a costly fuzzy match on Forename again. But preventing this will require a restructuring of the order of method calls, so most likely you test will again prevent this change, even though behavior remains correct and performance is improved. So the test have negative value.

But I understand you want to ensure that comparing strings which are fully equal should not be significantly slower due to the fuzzy algorithm. I see two possibilities:

• the performance benefit for this optimization is insignificant
• the performance benefit is significant

If it is insignificant, you should not waste time on testing it. If it is significant you should test it, but you should test the actual performance (time or processor cycles), not irrelevant implementation details.