Karl Seguin

Sponsors

The Lounge

Advertisement

Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
Simplicity is key to successful unit testing - Part 2

A couple weeks ago we looked at the importance of making tests robust - which can be accomplished by keeping them simple, focused and loosely coupled. However, as you test more complicated methods, you'll find that no matter how well-written your tests are, they'll bloat and grow brittle. Let's look at a simple example:

public ActionResult Save(int userId)
{
   User user = repository.LoadUser(userId);
   //todo handle null user
   user.Name = Request.Form["name"];
   user.Password = Request.Form["password"];
   try
   {
      user.Update();
   }
   catch (ValidationException ve)
   {
      ViewData["Errors"] = ve;
   }
   return View();
}

Here we have a controller action responsible for handling changes made to a user. Pretty straightforward stuff, right? Unfortunately, the code isn't particularly easy to unit test (well, it isn't really difficult, but we're using a straightforward example for demonstration purposes). The problem is that our action actually has a number of distinct behaviours. One solution is to write a single monolithic test to cover everything in our method. Of course, we already know that'll make it unbelievably brittle, as well as making hard to test variations (different inputs, different return values). The other solution is to make extensive use of stubs and make sure each dependent call is accounted for.

For example, if we want to write a test that focuses on the handling of ValidationException, we need to do quite a bit of stubbing:

[Test]
public void Save_SetsValidationErrorsInViewData()
{
  var controller = Mock.PartialMock<UsersController>();
  var repository = Mock.StrictMock<IRepository>();
  var user = Mock.PartialMock<User>();
  var expected = new ValidationException("name", "invalid");

  Mock.SetFakeControllerContext(controller);
  repository.Stub(r => r.LoadUser(3)).Return(user);
  controller.Request.Stub(r => r.Form)
    .Repeat.Any()
    .Return(new NameValueCollection());
  user.Stub(u => u.Update()).Throw(expected);

  controller.Save(3);
  Assert.AreSame(expected, controller.ViewData["Errors"]);          
}

Even for this simple example, the test is less than ideal. We need to setup a fake controller context (using the MvcMockHelper), handle our repository interaction and our Request.Form mapping. Worse, the same setup is required for each behaviour of this method that we'll test. The solution is to break-down our method into smaller more focused chunks. Here's an extreme example:

public ActionResult Save(int userId)
{
   User user = BindUser(userId, Request.Form);
   UpdateUser(user);
   return View();
}
internal virtual User BindUser(int userId, NameValueCollection parameters)
{
   User user = repository.LoadUser(userId);
   user.Name = parameters["name"];
   user.Password = parameters["password"];   
   return user;
}
internal virtual void UpdateUser(User user)
{
   try
   {
      user.Update();
   }
   catch (ValidationException ve)
   {
      ViewData["Errors"] = ve;
   }
}

Now, take a look at our test:

[Test]
public void UpdateUser_SetsValidationErrorsInViewData()
{
  var user = Mock.PartialMock<User>();
  var controller = new AwardsController();
  var expected = new ValidationException("name", "invalid");

  user.Expect(u => u.Update()).Throw(expected);
  controller.UpdateUser(user);          
  Assert.AreSame(expected, controller.ViewData["Errors"]);          
}

The test is majorly simplified - in fact, we don't even have to worry about our controller's context, or our interaction with the repository. The best part is that each of our methods behaviours (get the user, bind it, save it and mange the viewdata+view) can now be tested with equally simple test - rather than all-encompassing ones.

While I realize refactoring into really small methods can pose a psychological barrier for many, it's a truly useful technique with benefits beyond testability.


Posted 10-15-2008 8:27 PM by karl

[Advertisement]

Comments

Duncan wrote re: Simplicity is key to successful unit testing - Part 2
on 10-15-2008 10:44 PM

I've been writing unit tests for a while, but nothing too extreme.  I look at your code and understand only at a very high level what's going on.  I'd love to learn more -- where do you recommend a relative newb start?  I have been doing database development for a few years now, so I understand the "big pieces" in C# and NUnit, but I'm nobody's expert.

Neil Mosafi wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 5:40 AM

A lot of people get put off by all the virtual methods e.g "I don't want anyone overriding those methods so why should I make them virtual?"

I guess you could use the strategy pattern but that makes the code even more confusing!

2008 October 16 - Links for today « My (almost) Daily Links wrote 2008 October 16 - Links for today &laquo; My (almost) Daily Links
on 10-16-2008 5:47 AM

Pingback from  2008 October 16 - Links for today &laquo; My (almost) Daily Links

karl wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 8:33 AM

@Neil:

I agree people get put off, but I disagree with them. The virtual-by-default debate is old - with C# opting against it and Java opting for it. In most cases, I don't think performance concerns are realistic. And I don't think most .NET developers put so much thought into their class design to consider someone else extending the class in the future.

That said, it would be nice to be able to mock non-virtual members :)

karl wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 8:38 AM

@Duncan,

Glad you're willing to learn. Don't get too put off by my examples, I'm using lambdas, proxies and extension methods (Oh My!). Certainly not the most friendly examples to people not already familiar with it (although, ultimately the best way to write clean and concise code).

Different people learn differently - some are book learners, some are hands-on learners. If you like to look at examples and play with them, you might be interested in my Learning Application. It's meant to be a tool to learn high level concepts (o/r mapping, testing, decoupling, dependency injection), but there's no reason why you shouldn't start by learning how to program right:

codebetter.com/.../foundations-of-programming-learning-application.aspx

Dew Drop - October 16, 2008 | Alvin Ashcraft's Morning Dew wrote Dew Drop - October 16, 2008 | Alvin Ashcraft's Morning Dew
on 10-16-2008 9:33 AM

Pingback from  Dew Drop - October 16, 2008 | Alvin Ashcraft's Morning Dew

Koistya Navin wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 12:33 PM

I would use custom model binders instead (inhereted from ModelBinder).

Duncan wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 1:12 PM

Thanks!

karl wrote re: Simplicity is key to successful unit testing - Part 2
on 10-16-2008 1:23 PM

@Koistya: u might be missing the point.

Mario wrote re: Simplicity is key to successful unit testing - Part 2
on 10-20-2008 12:30 AM

This is a really great example of refactoring to Compose Method.  It makes the code easier to follow since each method is extremely focused and written at one level of detail.

Glitch4572 wrote re: Simplicity is key to successful unit testing - Part 2
on 10-24-2008 4:24 PM

In going to the refactored code, you will need to add another test.

Consider the following code:

public ActionResult Save(int userId)  

{  

  User user = BindUser(userId, Request.Form);  

  //UpdateUser(user);  

  return View();  

}  

Now, your later test version will still pass but your Save() method is no longer working properly.  You'd need a test to ensure that Save is actually updating the user.

Add a Comment

(required)  
(optional)
(required)  
Remember Me?