When unit testing, use Mocks for Commands, and Stubs for Queries.

A few years ago, I helped an organization adopt Test-Driven Development. One question that kept coming up was when to use Stubs, and when to use Mocks. (First, you need to understand the conceptual difference between Mocks and Stubs, so go read xUnit Test Patterns, and then resume reading this blog post once you're done. If you need a shorter introduction, you can read my MSDN Magazine article on the subject.)

After having answered the question on when to use what, a couple of times, I arrived at this simple rule, based on the language of Command Query Separation (CQS):

  • Use Mocks for Commands
  • Use Stubs for Queries
This makes lots of sense, because Commands are all about side effects, and Mocks are all about Behaviour Verification: that is, that side effects occurred. Stubs, on the other hand, mainly exist to 'make happy noises', and one of the ways they have to do that, is to return data from dependencies, when return data is required.

This discovery made me really happy, and I planned to blog about it. Then, soon after, I saw the exact same advice in GOOS (ch. 24, p. 278):

Allow Queries; Expect Commands
That verified that I was on the right track, but on the other hand, since the rule was already described, I didn't see the value of restating what was already public knowledge.

A couple of years later, it's clear to me that this rule may be public knowledge, but is certainly isn't common knowledge. This is, without comparison, the most common mistake I see people make, when I review unit tests. Thus, although it has already been said in GOOS, I'll repeat it here.

Use Mocks for Commands; Stubs for Queries
Through an example, you'll learn why Mocks for Queries are dangerous, and how a technique I call Data Flow Verification can help.

(Anti-)Example: Mocking Queries

In this example, you'll see how easily things can go wrong when you use Mocks for Queries. Assume that you want to test SomeController, that uses an IUserRepository:

public interface IUserRepository
{
    User Read(int userId);
 
    void Create(int userId);
}

In the first test, you want to verify that when you invoke SomeController.GetUser, it returns the user from the injected Repository, so you write this unit test:

[Fact]
public void GetUserReturnsCorrectResult()
{
    var expected = new User();
    var td = new Mock<IUserRepository>();
    td.Setup(r => r.Read(It.IsAny<int>())).Returns(expected);
    var sut = new SomeController(td.Object);
 
    var actual = sut.GetUser(1234);
 
    Assert.Equal(expected, actual);
}

This test uses Moq, but it could have used another dynamic mock library as well, or even hand-written Test Doubles.

The most remarkable characteristic of this test is the unconditional return of a value from the Test Double, as implemented by the use of It.IsAny<int>(). Why you'd do this is a small mystery to me, but I see it time and again when I review unit tests. This is the source of many problems.

Using a technique called Devil's Advocate, when reviewing, I usually tell the author that I can make the test pass with this (obviously degenerate) implementation:

public User GetUser(int userId)
{
    return this.userRepository.Read(0);
}

In my experience, this will most likely prompt you to add another test:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserCallsRepositoryWithCorrectValue(int userId)
{
    var td = new Mock<IUserRepository>();
    var sut = new SomeController(td.Object);
 
    sut.GetUser(userId);
 
    td.Verify(r => r.Read(userId));
}

Jolly good attempt, sport! Regrettably, it doesn't protect you against my Devil's Advocate technique; I can implement the GetUser method like this:

public User GetUser(int userId)
{
    this.userRepository.Read(userId);
    return this.userRepository.Read(0);
}

This is so obviously wrong that you'd be likely to argue that no-one in their right mind would do something like that. While I agree, the fact that this (obviously incorrect) implementation passes all unit tests should inform you about the quality of your unit tests.

Strict Mocks are not the solution

The problem is that the second test method (GetUserCallsRepositoryWithCorrectValue) attempts to use a Mock to verify a Query. This is wrong, as indicated by GOOS.

In desperation, some people attempt to resolve their conundrum by tightening the constraints of the Mock. This road leads towards Strict Mocks, and there lies madness. Soon, you'll learn the reason for this.

First, you may attempt to constrain the number of times the Read method can be invoked:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserCallsRepositoryWithCorrectValue(int userId)
{
    var td = new Mock<IUserRepository>();
    var sut = new SomeController(td.Object);
 
    sut.GetUser(userId);
 
    td.Verify(r => r.Read(userId), Times.Once());
}

Notice the addition of the Times.Once() clause in the last line. This instructs Moq that Read(userId) can only be invoked once, and that an exception should be thrown if it's called more than once. Surprisingly, the Devil's Advocate implementation still passes all tests:

public User GetUser(int userId)
{
    this.userRepository.Read(userId);
    return this.userRepository.Read(0);
}

While this may seem surprising, the reason it passes is that the assertion only states that Read(userId) should be invoked exactly once; it doesn't state anything about Read(0), as long as userId isn't 0.

Second, you attempt to resolve that problem by stating that no matter the input, the Read method must be invoked exactly once:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserCallsRepositoryWithCorrectValue(int userId)
{
    var td = new Mock<IUserRepository>();
    var sut = new SomeController(td.Object);
 
    sut.GetUser(userId);
 
    td.Verify(r => r.Read(It.IsAny<int>()), Times.Once());
}

Notice that the input constraint is loosened to It.IsAny<int>(); combined with Times.Once(), it should ensure that the Read method is invoked exactly once. It does, but the devil is still mocking you (pun intended):

public User GetUser(int userId)
{
    return this.userRepository.Read(0);
}

This is the first degenerate implementation I added, so now you're back where you began: it passes all tests. The new test case (that uses a Mock against a Query) has added no value at all.

Third, in utter desperation, you turn to Strict Mocks:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserCallsRepositoryWithCorrectValue(int userId)
{
    var td = new Mock<IUserRepository>(MockBehavior.Strict);
    td.Setup(r => r.Read(userId)).Returns(new User());
    var sut = new SomeController(td.Object);
 
    sut.GetUser(userId);
 
    td.Verify();
}

Notice the use of MockBehavior.Strict in the Mock constructor, as well as the explicit Setup in the Fixture Setup phase.

Finally, this reigns in the devil; I have no other recourse than to implement the Read method correctly:

public User GetUser(int userId)
{
    return this.userRepository.Read(userId);
}

If the Devil's Advocate technique indicates that a faulty implementation implies a bad test suite, then a correct implementation must indicate a good test suite, right?

Not quite, because the use of a Strict Mock makes your tests much less maintainable. You should already know this, but I'll show you an example.

Assume that you have a rule that if the User returned by the Read method has an ID of 0, it means that the user doesn't exist, and should be created. (There are various problems with this design, most notably that it violates CQS, but that's another story...)

In order to verify that a non-existing User is created during reading, you add this unit test:

[Fact]
public void UserIsSavedIfItDoesNotExist()
{
    var td = new Mock<IUserRepository>();
    td.Setup(r => r.Read(1234)).Returns(new User { Id = 0 });
    var sut = new SomeController(td.Object);
 
    sut.GetUser(1234);
 
    td.Verify(r => r.Create(1234));
}

This is is fine test that verifies a Command (Create) with a Mock. This implementation passes the test:

public User GetUser(int userId)
{
    var u = this.userRepository.Read(userId);
    if (u.Id == 0)
        this.userRepository.Create(1234);
    return u;
}

Alas! Although this implementation passes the new test, it breaks an existing test! Can you guess which one? Yes: the test that verifies a Query with a Strict Mock. It breaks because it explicitly states that the only method call allowed on the Repository is the Read method.

You can resolve the problem by editing the test:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserCallsRepositoryWithCorrectValue(int userId)
{
    var td = new Mock<IUserRepository>(MockBehavior.Strict);
    td.Setup(r => r.Read(userId)).Returns(new User { Id = userId });
    var sut = new SomeController(td.Object);
 
    sut.GetUser(userId);
 
    td.Verify();
}

Notice that userId was added to the returned User in the Setup for the Strict Mock.

Having to edit existing tests is a genuine unit test smell. Not only does it add an unproductive overhead (imagine that many tests break instead of a single one), but it also decreases the trustworthiness of your test suite.

Data Flow Verification

The solution is really simple: use a conditional Stub to verify the data flow through the SUT. Here's the single test you need to arrive at a correct implementation:

[Theory]
[InlineData(1234)]
[InlineData(9876)]
public void GetUserReturnsCorrectValue(int userId)
{
    var expected = new User();
    var td = new Mock<IUserRepository>();
    td.Setup(r => r.Read(userId)).Returns(expected);
    var sut = new SomeController(td.Object);
 
    var actual = sut.GetUser(userId);
 
    Assert.Equal(expected, actual);
}

Even when I attempt to use the Devil's Advocate technique, there's no way I can provide a faulty implementation. The method must invoke Read(userId), because otherwise it can't return expected. If it invokes the Read method with any other value, it may get a User instance, but not expected.

The Data Flow Verification test uses a (conditional) Stub to verify a Query. The distinction may seem subtle, but it's important. The Stub doesn't require that Read(userId) is invoked, but is configured in such a way that if (and only if) Read(userId) is invoked, it'll return expected.

With the Stub verifying the Query (GetUserReturnsCorrectValue), and the other test that uses a Mock to verify a Command (UserIsSavedIfItDoesNotExist), the final implementation is:

public User GetUser(int userId)
{
    var u = this.userRepository.Read(userId);
    if (u.Id == 0)
        this.userRepository.Create(1234);
    return u;
}

These two tests correctly specify the behaviour of the system in a terse and maintainable way.

Conclusion

Data Flow Verification is remarkably simple, so I'm continually taken aback that people seem to go out of their way to avoid it. Use it to verify Queries with Stubs, and keep the Mocks with the Commands.



Wish to comment?

You can add a comment to this post by sending me a pull request. Alternatively, you can discuss this post on Twitter or Google Plus, or somewhere else with a permalink. Ping me with the link, and I may add it as a comment.

Published

Wednesday, 23 October 2013 21:45:00 UTC

Tags



"Our team wholeheartedly endorses Mark. His expert service provides tremendous value."
Hire me!