How to properly mock and unit test

Question

I'm basically trying to teach myself how to code and I want to follow good practices. There are obvious benefits to unit testing. There is also much zealotry when it comes to unit-testing and I prefer a much more pragmatic approach to coding and life in general. As context, I'm currently writing my first "real" application which is the ubiquitous blog engine using asp.net MVC. I'm loosely following the MVC Storefront architecture with my own adjustments. As such, this is my first real foray into mocking objects. I'll put the code example at the end of the question.

I'd appreciate any insight or outside resources that I could use to increase my understanding of the fundamentals of testing and mocking. The resources I've found on the net are typically geared towards the "how" of mocking and I need more understanding of the where, why and when of mocking. If this isn't the best place to ask this question, please point me to a better place.

I'm trying to understand the value that I'm getting from the following tests. The UserService is dependent upon the IUserRepository. The value of the service layer is to separate your logic from your data storage, but in this case most of the UserService calls are just passed straight to IUserRepository. The fact that there isn't much actual logic to test could be the source of my concerns as well. I have the following concerns.

• It feels like the code is just testing that the mocking framework is working.
• In order to mock out the dependencies, it makes my tests have too much knowledge of the IUserRepository implementation. Is this a necessary evil?
• What value am I actually gaining from these tests? Is the simplicity of the service under test causing me to doubt the value of these tests.

I'm using NUnit and Rhino.Mocks, but it should be fairly obvious what I'm trying to accomplish.

    [SetUp]
    public void Setup()
    {
        userRepo = MockRepository.GenerateMock<IUserRepository>();
        userSvc = new UserService(userRepo);
        theUser = new User
        {
            ID = null,
            UserName = "http://joe.myopenid.com",
            EmailAddress = "[email protected]",
            DisplayName = "Joe Blow",
            Website = "http://joeblow.com"
        };
    }

    [Test]
    public void UserService_can_create_a_new_user()
    {
        // Arrange
        userRepo.Expect(repo => repo.CreateUser(theUser)).Return(true);

        // Act
        bool result = userSvc.CreateUser(theUser);

        // Assert
        userRepo.VerifyAllExpectations();
        Assert.That(result, Is.True, 
          "UserService.CreateUser(user) failed when it should have succeeded");
    }

    [Test]
    public void UserService_can_not_create_an_existing_user()
    {
        // Arrange
        userRepo.Stub(repo => repo.IsExistingUser(theUser)).Return(true);
        userRepo.Expect(repo => repo.CreateUser(theUser)).Return(false);
        // Act
        bool result = userSvc.CreateUser(theUser);

        // Assert
        userRepo.VerifyAllExpectations();
        Assert.That(result, Is.False, 
            "UserService.CreateUser() allowed multiple copies of same user to be created");
    }

Answer 1

You are right: the simplicity of the service makes these tests uninteresting. It is not until you get more business logic in the service, that you will gain value from the tests.

You might consider some tests like these:

CreateUser_fails_if_email_is_invalid()
CreateUser_fails_if_username_is_empty()

Another comment: it looks like a code-smell, that your methods return booleans to indicate success or failure. You might have a good reason to do it, but usually you should let the exceptions propagate out. It also makes it harder to write good tests, since you will have problems detecting whether your method failed for the "right reason", f.x. you might write the CreateUser_fails_if_email_is_invalid()-test like this:

[Test]
public void CreateUser_fails_if_email_is_invalid()
{
    bool result = userSvc.CreateUser(userWithInvalidEmailAddress);
    Assert.That(result, Is.False);
}

and it would probably work with your existing code. Using the TDD Red-Green-Refactor-cycle would mitigate this problem, but it would be even better to be able to actually detect that the method failed because of the invalid email, and not because of another problem.

Answer 2

Essentially what you are testing here is that the methods are getting called, not whether or not they actually work. Which is what mocks are supposed to do. Instead of calling the method, they just check to see if the method got called, and return whatever is in the Return() statement. So in your assertion here:

Assert.That(result, Is.False, "error message here");

This assertion will ALWAYS succeed because your expectation will ALWAYS return false, because of the Return statement:

userRepo.Expect(repo => repo.CreateUser(theUser)).Return(false);

I'm guessing this isn't that useful in this case.

Where mocking is useful is when you want to, for example, make a database call somewhere in your code, but you don't want to actually call to the database. You want to pretend that the database got called, but you want to set up some fake data for it to return, and then (here's the important part) test the logic that does something with the fake data your mock returned. In the above examples you are omitting the last step. Imagine you had a method that displayed a message to the user that said whether the new user was created:

public string displayMessage(bool userWasCreated) {
    if (userWasCreated)
        return "User created successfully!";
    return "User already exists";
}

then your test would be

userRepo.Expect(repo => repo.CreateUser(theUser)).Return(false);
Assert.AreEqual("User already exists", displayMessage(userSvc.CreateUser(theUser)))

Now this has some value, because you are testing some actual behavior. Of course, you could also just test this directly by passing in "true" or "false." You don't even need a mock for that test. Testing expectations is fine, but I've written plenty of tests like that, and have come to the same conclusion that you are reaching - it just isn't that useful.

So in short, mocking is useful when you want to abstract away externalities, like databases, or webservice calls, etc, and inject known values at that point. But it's not often useful to test mocks directly.

Answer 3

If you write your tests before you write your code, you'll gain much more from your unit tests. One of the reasons that it feels like your tests aren't worth much is that you're not deriving the value of having your tests drive the design. Writing your tests afterwards is mostly just an exercise in seeing if you can remember everything that can go wrong. Writing your tests first causes you to think about how you would actually implement the functionality.

These tests aren't all that interesting because the functionality that is being implemented is pretty basic. The way you are going about mocking seems pretty standard -- mock the things the class under test depends on, not the class under test. Testability (or good design sense) has already led you to implement interfaces and use dependency injection to reduce coupling. You might want to think about changing the error handling, as other have suggested. It would be nice to know why, if only to improve the quality of your tests, CreateUser has failed, for instance. You could do this with exceptions or with an out parameter (which is how MembershipProvider works, if I remember correctly).

Answer 4

You are facing the question of "classical" vs. "mockist" approaches to testing. Or "state-verification" vs. "behaviour-verification" as described by Martin Fowler: http://martinfowler.com/articles/mocksArentStubs.html#ClassicalAndMockistTesting

Another most excellent resource is Gerard Meszaros' book "xUnit Test Patterns: Refactoring Test Code"