Unit testing anti-patterns: Structural Inspection

Datetime:2016-08-23 02:51:19          Topic: Unit Testing           Share

This post is about the practice of Structural Inspection in unit testing and why I personally consider it an anti-pattern.

Structural Inspection

But first, let me explain Structural Inspection itself. Structural Inspection is generally about writing tests verifying that the system under test (SUT) has some particular structure. Let’s say we’ve got the following code base:

public class Message

{

    public string Header { get ; set ; }

    public string Body { get ; set ; }

    public string Metadata { get ; set ; }

}

public interface IProcessor

{

    string Process ( Message message);

}

public class MessageProcessor : IProcessor

{

    private readonly List < IProcessor > _subProcessors ;

    public IReadOnlyList < IProcessor > SubProcessors => _subProcessors ;

    public MessageProcessor ()

    {

        _subProcessors = new List < IProcessor >

        {

            new HeaderProcessor (),

            new BodyProcessor (),

            new MetadataProcessor ()

        };

    }

    // IProcessor implementation

}

Following the Structural Inspection unit testing technique, you could write a test that makes sure the MessageProcessor class implements the IProcessor interface, like this:

[ Fact ]

public void MessageProcessor_implements_IProcessor ()

{

    var processor = new MessageProcessor ();

    Assert . IsAssignableFrom < IProcessor >(processor);

}

And to verify that the routine the processor follows is also correct, you could add a test similar to the following:

[ Fact ]

public void MessageProcessor_uses_correct_sub_processors ()

{

    var processor = new MessageProcessor ();

    IReadOnlyList < IProcessor > processors = processor. SubProcessors ;

    Assert . Equal (3, processors. Count );

    Assert . IsAssignableFrom < HeaderProcessor >(processors[0]);

    Assert . IsAssignableFrom < BodyProcessor >(processors[1]);

    Assert . IsAssignableFrom < MetadataProcessor >(processors[2]);

}

It checks to see if the sub-processors are all of the expected types and appear in a correct order which means the way MessageProcessor processes messages must also be correct.

Structural Inspection is an anti-pattern

Alright, so why do I think this is a bad practice? Let’s take a look at it from the value proposition perspective I wrote about in anearlier post. Here are 3 components that comprise a valuable unit test:

  • High chance of catching a regression error.
  • Low chance of producing a false positive.
  • Fast feedback.

The first and the third attributes are good here. Such tests would most likely reveal an error assuming the existing implementation is correct and they run fast. However, they fall short of protection against false positives.

What if you change the way you arrange the sub-processors? Or remove the IProcessor interface implementation because you’ve come to realize that it’s not necessary in this particular case? The tests will turn red regardless of whether or not the functionality itself is broken.

Structural Inspection encourages you to couple your tests to the SUT’s implementation details which is never a good idea. The problem is essentially the same as with the excessive use of mocks: such tests are not able to distinguish a bug from a legit refactoring.

Structural Inspection is sometimes claimed to be able to prove the code base’s correctness. In reality, though, it sets your code in stone. The only thing it proves is that the SUT is implemented in one particular way. And that way may or may not be correct, you will need to introduce additional tests to verify that anyway.

When I think of inspecting the structure of the SUT, this is what comes to mind:

[ Fact ]

public void MessageProcessor_is_implemented_correctly ()

{

    string sourceCode = File . ReadAllText ( @”<project path>\MessageProcessor.cs” );

    Assert . Equal (

@”public class MessageProcessor : IProcessor

{

    private readonly List<IProcessor> _subProcessors;

    public IReadOnlyList<IProcessor> SubProcessors => _subProcessors;

    public MessageProcessor()

    {

        _subProcessors = new List<IProcessor>

        {

            new HeaderProcessor(),

            new BodyProcessor(),

            new MetadataProcessor()

        };

    }

}” , sourceCode);

}

This test is just plain ridiculous. At the same time, it’s not that different from the two tests I brought earlier. All 3 insist on a particular implementation of the SUT without taking into account the outcome it produces. And all 3 will break each time you change that implementation. Admittedly, though, this test will turn red more often.

Structural Inspection: alternatives

So what are the alternatives for the Structural Inspection technique? The alternative is to verify the end result the SUT generates and to distance yourself from the implementation details as much as possible.

So, in the example above, what is it that the MessageProcessor class actually does? Here’s its full source code:

public class MessageProcessor : IProcessor

{

    private readonly List < IProcessor > _subProcessors ;

    public IReadOnlyList < IProcessor > SubProcessors => _subProcessors ;

    public MessageProcessor ()

    {

        _subProcessors = new List < IProcessor >

        {

            new HeaderProcessor (),

            new BodyProcessor (),

            new MetadataProcessor ()

        };

    }

    public string Process ( Message message)

    {

        return _subProcessors

            . Select (x => x. Process (message))

            . Aggregate ( string . Empty , (str1, str2) => str1 + str2);

    }

}

The only thing that makes sense to check here is the string the Process method returns as it’s the only observable result we get out of it. As long as the output stays the same, we shouldn’t worry about how exactly that output was generated. These implementation details are simply irrelevant.

By the way, getting rid of inspection tests will allow us to remove the SubProcessors property. Which is a good thing because the only purpose of it is to expose the internal structure of the class in order to exercise it.

Overall, try to constantly ask yourself a question: does this test verify some business requirement? If the answer is no, remove it. The most valuable tests are always the tests that have at least some connection to the business requirements your code base is ought to address. Not surprisingly, such tests also fall into the formal definition I brought in the beginning of this article: they are likely to give a protection against regression errors and they are unlikely to turn red without a good reason.

Summary

Tests that employ the Structural Inspection technique couple to the SUT’s implementation details and thus are fragile. Because of that, they don’t contribute to your confidence in the software correctness.

The most valuable tests are tests that verify the observable behavior as it seems to appear from the end user’s perspective. The closer you can get to this kind of verification, the better.

Other articles in the series





About List