Putting mocks in their place

Awhile back, I talked about 3 simple rules for Rhino Mocks:

1. Use the static MockRepository.GenerateMock method.  Don’t use an instance of a MockRepository, and don’t use the GenerateStub method.
2. If the mocked instance method returns a value, use the Stub method to set up a result for that value.  This is an indirect input.
3. If the mocked instance method does not return a value (void), use the AssertWasCalled/AssertWasNotCalled methods.  This is an indirect output.

This can be simplified even further – if we’re following command-query separation, then it really comes down to having two types of methods:

• Commands – these are Action-based delegates.  They perform an action, but return nothing (void)
• Queries – these are Func-based delegates.  They answer a question, but they do NOT modify state

If we follow this rule, then our Rhino Mocks rules become:

• Use Stub for Queries
• Use AssertWasCalled for Commands

After using flavors of TDD, BDD and so on for a few years, I’ve had a bit of an epiphany.  Mocks, stubs, fakes and the like are most valuable when they replace external dependencies.

Why?

Unit tests with mocks and stubs leak implementation details into the test, leading to a test that’s coupled to the internal implementation of how the tested class does its job.  Tests coupled to the implementation of the class under test are a barrier to refactoring.  If our tests become a barrier to refactoring, it kills one of the purported benefits of refactoring – providing a safety net for refactoring!

So where do mocks belong?

Where things are hard to test, like databases, web services, configuration files, and so on.  Components outside the core logic of our application, that we don’t have control over.

An example

Back in the original post, we looked at an order processor:

public class OrderProcessor
{
    private readonly ISmtpClient _client;
    private readonly IOrderSpec _spec;

    public OrderProcessor(ISmtpClient client, IOrderSpec spec)
    {
        _client = client;
        _spec = spec;
    }

    public void PlaceOrder(Order order)
    {
        if (_spec.IsMatch(order))
        {
            string body = "Huge frickin' order alert!!!rnOrder #:" + order.OrderNumber;

            MailMessage message =
                new MailMessage("[email protected]", "[email protected]",
                                "Order processed", body);

            _client.Send(message);
        }

        order.Status = OrderStatus.Submitted;
    }
}

Suppose that we don’t like how we chose how the OrderProcessor matched on the specification, and decided to go a completely different direction.  Yes, we can’t test email, but let’s look at the implementation details in our test:

[Test]
public void Should_send_an_email_when_the_order_spec_matches()
{
    // Arrange
    var client = MockRepository.GenerateMock<ISmtpClient>();
    var spec = MockRepository.GenerateMock<IOrderSpec>();
    var order = new Order {Status = OrderStatus.New, Total = 500m};

    spec.Stub(x => x.IsMatch(order)).Return(true);

    var orderProcessor = new OrderProcessor(client, spec);

    // Act
    orderProcessor.PlaceOrder(order);

    // Assert
    client.AssertWasCalled(x => x.Send(null), opt => opt.IgnoreArguments());
    order.Status.ShouldEqual(OrderStatus.Submitted);
}

The whole need for an OrderProcessor to need an IOrderSpec, or that the IsMatch method is called is an implementation detail.  If we want to make large, interesting changes to this class, we’ll need to just ditch these tests altogether.  And if we have to delete tests every time an implementation changes, how is that a safety net?  It’s not.

Instead, I’ll write my tests quite a bit differently, ones where dependencies don’t show up unless they truly cannot be used, and even then, I’ll try and build out fake implementations that are easily re-usable.  In the next post, I’ll go through this style of test that allows refactoring much more easily than the implementation leaks of the above test.