Blog

I’ve been reading a lot of code lately. When I’m doing this, I find it very important to have some unit tests that makes it easier for me to comprehend the actual production code. In order to do that the unit tests have to very readable.

Something that I see quite a lot in the projects that I’m involved with is what I call ‘Betrayal of Context’. In short, this means that some BDD style unit tests (specifications) are not eligible for the context that has been set up for them. This results in unit tests that are more verbose than they should be which makes them harder to read.

As a piece of code can say more than a thousand words, let me show you the simplest example I could think of.

public class ApplicationRequest
{
    private ApplicationRequestStatus Status { get; set; }
    public Boolean IsApproved()
    {
        return ApplicationRequestStatus.Approved == Status;
    }
    public void ApproveUsing(IStrictRegulation
                                     strictRegulation)
    {
        if(ApplicationRequestStatus.Pending != Status)
            throw new InvalidOperationException("Oeps");
        if(strictRegulation.Complies(this))
        {
            Status = ApplicationRequestStatus.Approved;
        }
        else
        {
            Status = ApplicationRequestStatus.Rejected;
        }
    }
}

public enum ApplicationRequestStatus
{
    Pending = 0,
    Approved = 1,
    Rejected = 2
}

public interface IStrictRegulation
{
    Boolean Complies(ApplicationRequest request);
}

What we have here is an utterly useless domain that handles application requests, but it will do for our example. In order to get approved, an application request needs to comply to some strict regulations.

Now that we’ve got ourselves acquainted with the subject-under-test, let me show you some example of what I consider ‘Betrayal of Context’.

[TestFixture]
[Category("ApplicationRequestTestFixture")]
public class When_approving_an_application_request
    : InstanceSpecification<ApplicationRequest>
{
    protected override void Establish_context()
    {
        StrictRegulationStub = MockRepository
            .GenerateStub<IStrictRegulation>();
        StrictRegulationStub.Stub(strictRegulation =>
             strictRegulation.Complies(null))
            .IgnoreArguments()
            .Return(true);
    }

    [Test]
    public void Then_it_should_be_approved_if_it_meets_strict_regulations()
    {
        SUT.ApproveUsing(StrictRegulationStub);
        Assert.That(SUT.IsApproved());
    }

    [Test]
    public void Then_it_should_be_rejected_if_it_does_not_meet_strict_regulations()
    {
        StrictRegulationStub.BackToRecord();
        StrictRegulationStub.Stub(strictRegulation
            => strictRegulation.Complies(null))
            .IgnoreArguments()
            .Return(false);
        SUT.ApproveUsing(StrictRegulationStub);
        Assert.That(SUT.IsApproved(), Is.False);
    }

    [Test]
    [ExpectedException(typeof(InvalidOperationException))]
    public void Then_an_exception_should_be_thrown_if_its_status_is_not_pending()
    {
        SUT.ApproveUsing(StrictRegulationStub);
        SUT.ApproveUsing(StrictRegulationStub);
    }

    protected override ApplicationRequest Create_subject_under_test()
    {
        return new ApplicationRequest();
    }

    private IStrictRegulation StrictRegulationStub
    { get; set; }
}

I don’t know about you, but I have issues with this code. Two out of three specifications have nothing to do with the context setup in the Establish_context method. In fact, the second unit test needs to redo the entire setup for the stub object. All too often I see this happening, which is bad for my heart (at least that’s what my doctor keeps telling me :-) ). By organizing unit tests this way, we are also missing out on the ‘Because’ goodness I’ll show you later on.

The thing that disturbs me the most is that these ‘shortcuts’ add clutter which makes them less readable than they should be. That’s what our craft is all about. Communicating! Not only with the compiler, but most importantly with the poor fellow that comes after you (in this case, me!) and needs to understand what you have been doing.

So let us refactor these specifications and put them in the right context.

public abstract class behaves_like_an_application_request_that_meets_strict_regulations
    : InstanceSpecification<ApplicationRequest>
{
    protected override void Establish_context()
    {
        StrictRegulationStub = MockRepository
            .GenerateStub<IStrictRegulation>();
        StrictRegulationStub.Stub(strictRegulation
            => strictRegulation.Complies(null))
            .IgnoreArguments()
            .Return(true);
    }

    protected override void Because()
    {
        SUT.ApproveUsing(StrictRegulationStub);
    }

    protected override ApplicationRequest Create_subject_under_test()
    {
        return new ApplicationRequest();
    }

    protected IStrictRegulation StrictRegulationStub
    { get; set; }
}

[TestFixture]
[Category("ApplicationRequestTestFixture")]
public class When_approving_a_pending_application_request_that_meets_strict_regulations
    : behaves_like_an_application_request_that_meets_strict_regulations
{
    [Test]
    public void Then_it_should_get_approved()
    {
        Assert.That(SUT.IsApproved());
    }
}

[TestFixture]
[Category("ApplicationRequestTestFixture")]
public class When_approving_an_application_request_that_is_not_pending
    : behaves_like_an_application_request_that_meets_strict_regulations
{
    [Test]
    [ExpectedException(typeof(InvalidOperationException))]
    public void Then_an_exception_should_be_thrown()
    {
        SUT.ApproveUsing(StrictRegulationStub);
    }
}

[TestFixture]
[Category("ApplicationRequestTestFixture")]
public class When_approving_a_pending_application_request_that_does_not_meet_strict_regulations
    : InstanceSpecification<ApplicationRequest>
{
    protected override void Establish_context()
    {
        _strictRegulationStub = MockRepository
            .GenerateStub<IStrictRegulation>();
        _strictRegulationStub.Stub(strictRegulation
            => strictRegulation.Complies(null))
            .IgnoreArguments()
            .Return(false);
    }

    protected override void Because()
    {
        SUT.ApproveUsing(_strictRegulationStub);
    }

    [Test]
    public void Then_it_should_not_get_approved()
    {
        Assert.That(SUT.IsApproved(), Is.False);
    }

    protected override ApplicationRequest Create_subject_under_test()
    {
        return new ApplicationRequest();
    }
    
    private IStrictRegulation _strictRegulationStub;
}

Oh no, you’ve started out with only one test fixture and now you’ve got three of them and one base class? How can this be better? Well, I think it is better.

Notice how the context setup code that leaked into the specifications is now moved to where it belongs, namely the Establish_context method where everything is arranged.

By putting each specification in the right context (which is represented by a test fixture), I’ve been able to act on the subject-under-test in a single reusable method named ‘Because’. This saves a lot of copy/paste kung fu when we have a real-world scenario with more than one specification per context.

Also notice that the specifications themselves are now reduced to a single line of code that only asserts the outcome. The fact that there is no more than a single line of code ensures me that I can’t get the specifications any simpler than that, which makes everything very comprehensible.

I’ve moved some common context code into a base class. This is probably overkill for this simple example, but I wanted to show this because it can be a life saver whenever the complexity starts to increase.

Anyway, some last advice for 2008: stay faithful to your context.

Jan Van Ryswyck

Thank you for visiting my blog. I’m a professional software developer since Y2K. A blogger since Y2K+5. Provider of training and coaching in XP practices. Curator of the Awesome Talks list. Past organizer of the European Virtual ALT.NET meetings. Thinking and learning about all kinds of technologies since forever.