#2782

TDD anti-pattern “inherited or hidden test”

This post actually started out as my TDD and DSLs post but I got sidetracked into trying to describe an anti-pattern I’d noticed in a some of the unit tests we wrote as part of Service Factory: Modeling Edition. I came across James Carr’s excellent TDD anti-patterns post which got me thinking about this new variation on one of his anti-patterns. 

The pattern I’m seeing is slightly different. It’s the evil cousin of the “Excessive Setup” anti-pattern. Suppose I have a class that looks like this:

public class ClassUnderTest
{
    public ApplicationState ApplicationState;

    public ClassUnderTest(ApplicationState applicationState)
    {
        ApplicationState = applicationState;
    }

    public void Update(string state)
    {
        ApplicationState.State += ";" + state;
    }
}

public class ApplicationState
{
    public string State;
}

I’d like to test it but has some state that I need to pass in before I can do any testing. In some cases setting this up involves several lines of code. Here’s my test:

[TestClass]
public class ComplexTestFixture : TestFixtureBase
{
    [TestMethod]
    public void MyEvilTest()
    { 
        ClassUnderTest target = new ClassUnderTest(State);
       
        target.Update("NewState");

        Assert.AreEqual<string>(
            "InitialState;NewState",
            target.ApplicationState.State); 
     }
}

So this uses the 3-A’s Arrange, Act Assert pattern but something weird is going on in the arrange part. Where did State come from? Why do I need a base class? Further digging shows:

public class TestFixtureBase
{
    private ApplicationState _state;
   
    public ApplicationState State
    {
        get { return _state; }
        set { _state = value; }
    }

    [TestInitialize]
    public void Initialize()
    {
        this.State = new ApplicationState();
        this.State.State = "InitialState";}

    [TestCleanup]
    public void Cleanup()
    {
        Assert.IsNotNull(this.State);
        this.State = null;
    }
}

So in fact this test has initialization and cleanup associated with it and the application state is setup there but it’s in the base class. While this removes repetition of code (following the DRY principle) it does so at the expense of readability. The State property magically appears in the test code out of nowhere in this model. There’s also that sneaky Assert in the cleanup code, rather than at the end of each test. This isn’t excessive setup it’s hidden setup/teardown and potentially test code (the Assert).

This pattern gets more evil (and unreadable) the more you follow it. Imagine a test base class that actually creates target and initializes it! Or further base classes. Now someone reading your test code has no idea what state target was arranged in before the the test acts on it.

A better approach? How about the following:

[TestClass]
public class SimpleTestFixure
{
    [TestMethod]
    public void MyBetterTest()
    {
        ClassUnderTest target =
            new ClassUnderTest(
                CreateConfiguredState());

        target.Update("NewState");
        
        Assert.AreEqual<string>(
            "InitialState;NewState",
            target.ApplicationState.State);
    }

    private ApplicationState CreateConfiguredState()
    {
        ApplicationState state = new ApplicationState();
        state.State = "InitialState"; 
        return state;}
    }
}

Why is this better? Well, it’s clearer. The new test still follows the 3-A’s pattern but just looking at the test it’s clear that my ClassUnderTest is configured with some sort of state and the code to do that appears in the same class. If things get more complex then I’d suggest a helper class that gets instantiated in each test during the arrange step not a base class