Karl Seguin

As of late, I've been spending a lot of time writing unit tests for a complex application. In doing so, I've also been working hard to refine my skill of writing effective unit tests. I realize that to make my tests robust, I have to keep my unit tests as well as the code under tests as simple as possible. This also led to a higher quality in code coverage.

Robustness is vitally important for unit testing to be worthwhile. If you make a change in your code which breaks seemingly unrelated tests, you'll end up spending too much time maintaining your tests to get any real value from them. It's critical that you test as granular a behavior as possible with the minimum possible amount of coupling.

Here's a contrived example that exaggerates a bad test (we'll move on to more practical and subtle examples next).

[Test]
public void SetBirthdayUpdatesAge()
{
  var user = new User();
  user.BirthDay = DateTime.Now.AddYears(-17);
  Assert.AreEqual(17, user.Age);
  Assert.IsFalse(user.CanBuyBooze());
}

The last assert might seem harmless – why not verify other expected states, right? The problem is that when we change the logic of the CanBuyBooze() method, our test, which focuses on the behavior of setting a user's age, breaks. In other words, what appears to be adding value to our tests is actually making it brittle. The solution is to add another test (or maybe more than one) to specifically cover the CanBuyBooze() method.

The same issue exists in a far more subtle manner – interaction testing. Look at this example:

[Test]
public void Login_SetsUserInViewData()
{
  var foundUser = new User();
  var repository = Mock.StrictMock<IRepository>();
  repository.Expect(r => r.LoadUser("abc", "123")).Return(foundUser);
  
  var controller = new LoginController();
  controller.Index("abc", "123");
  
  Assert.AreSame(foundUser, controller.ViewData["User"]);

  repository.VerifyAllExpectations();
}

This is probably how I would have written unit tests in the past. It isn't horrible, but it's more brittle than it has to be (the rot will simply get worse for a more complex scenario). If you haven't spotted the problem, ask yourself two simple questions:

1. What is the purpose of the test?
2. What does the test actually do?

The purpose is stated plainly enough in the method name – verify that the appropriate user is loaded in the controller's ViewData. But that isn't all we're testing, we're also setting expectations on how we'll interact with our repository. Here's a revised version:

[Test]
public void Login_SetsFoundUserInViewData()
{
  var foundUser = new User();
  var repository = Mock.DynamicMock<IRepository>();
  repository.Stub(r => r.LoadUser(null, null)).IgnoreArguments().Return(foundUser);
  
  var controller = new LoginController();
  controller.Index("abc", "123");
  
  Assert.AreSame(foundUser, controller.ViewData["User"]);
}

There are two key differences. First, rather than using a strict mock we're using a dynamic mock. Second, we've specified IgnoreArguments(). The difference between a strict mock and a dynamic mock is important. A strict mock will fail if members were called which weren't explicitly stated. A dynamic mock is far more forgiving. Secondly, we're using IgnoreArguments() because verifying the inputs into our repository isn't the purpose of this test.

Our new unit test is considerably more robust. You might be thinking that none of this is very pragmatic, but let me give you a simple example. Here's what our initial Login method looked like:

public void Index(string userName, string password)
{
  ViewData["User"] = repository.LoadUserFromCredentials(userName, password);
}

With this code, both tests will work just fine. Now let's make a realistic change:

public void Index(string userName, string password)
{
  passsword = repository.EncryptPassword(password); 
  ViewData["User"] = repository.LoadUserFromCredentials(userName, password);
}

Our initial unit test will fail, while our revised one will continue to work. All we want to do is test that the returned value from our repository is stored in the ViewData – that should require a minimal amount of configuration. If you're writing such test and find yourself setting up multiple expectations (possibly on multiple mock objects) consider rethinking the purpose of your specific test.

There is a place for our strict mock and our explicit argument list – but it isn't within this test. It's for another test (or group of tests) specifically tailored and meant to test the interaction between our controller and repository.