Monday, 19 January 2009

Duplication, NUnit, Row Tests and MVC Views

I'm working on a project that uses the ASP.NET MVC framework. Learning how to use this is fun and I like the fact that I am able to write code that feels cleaner to me. We are working with preview 4 at the moment but should be moving to the release candidate as soon as it becomes available. I'm really looking forward to some of the "post preview 4" features, such as Model Binder support and the AcceptVerbs attribute. One of the nice things about developing with this framework is that it makes unit testing Web sites a whole lot easier. This is because the "controller" part of MVC is just a standard class that you can write unit tests against.


I came up with a neat piece of code as I was writing a test that checked the value of a variable stored in ViewData. I'm not saying it's new or original, just that it evolved naturally from my use of Test Driven Development (TDD). The value being checked was an enumeration, which was used to control an aspect of the Web page style. The value was different for each action on the controller and we had eleven actions. I wanted to test that the expected value was present in the view returned from each action. I have an example of the sort of thing that I was trying to do that you can download from my Google Code project. Here is the unit test I wrote for the example (I use 'MethodUnderTest_Scenario_ExpectedBehaviour' as my test naming convention).



[Test]
[Category("Unit")]
public void Index_IndexActionCalled_ViewDataContainsExpectedColour()
{
// arrange
var theExpectedColour = Colour.red;
// act
var homeController = new HomeController();
var viewResult = homeController.Index()as ViewResult;
// assert
Assert.That(viewResult, Is.Not.Null);
if (viewResult == null || !(viewResult.ViewData["Colour"] is Colour))
{
Assert.Fail("ViewData does not contain a Colour.");
return;
}
var actualColour = (Colour)viewResult.ViewData["Colour"];
Assert.That(actualColour, Is.EqualTo(theExpectedColour));
}



Once I made this test pass I moved on to the next one.

[Test]
[Category("Unit")]
public void About_AboutActionCalled_ViewDataContainsExpectedColour()
{
// arrange
var theExpectedColour = Colour.blue;
// act
var homeController = new HomeController();
var viewResult = homeController.About() as ViewResult;
// assert
Assert.That(viewResult, Is.Not.Null);
if (viewResult == null || !(viewResult.ViewData["Colour"] is Colour))
{
Assert.Fail("ViewData does not contain a Colour.");
return;
}
var actualColour = (Colour)viewResult.ViewData["Colour"];
Assert.That(actualColour, Is.EqualTo(theExpectedColour));
}
I got this second test passing and noticed that there was a lot of duplication in my test code. It is good TDD practice to treat test code the same as production code, so my next step was to refactor the test code. The sections outlined in blue above show the code differences and those differences are 1) the expected colour and 2) the name of the method being called. To refactor this I would need to have a test that takes parameters. I could then pass in the expected colour and the name of the method to be called. Reflection could then be used to call the method. This is where NUnit RowTest attributes come into play. RowTest attributes are part of nunit.framework.extensions.dll. Adding a reference to this dll allows me to create parameterised unit tests. The next stage of my refactoring was to implement a parameterised unit test.

[RowTest]
[Row("About", Colour.blue)]
[Category("Unit")]
public void About_AboutActionCalled_ViewDataContainsExpectedColour(string methodName, Colour expectedColour)
{
// arrange
var theExpectedColour = expectedColour;
// act
var homeController = new HomeController();
// Use reflection to call the action method
var controllerType = homeController.GetType();
var methodToTest = controllerType.GetMethod(methodName);
Assert.That(methodToTest, Is.Not.Null, string.Format("No method called {0} found.", methodName));
var viewResult = methodToTest.Invoke(homeController, null) as ViewResult;
// assert
Assert.That(viewResult, Is.Not.Null);
if (viewResult == null || !(viewResult.ViewData["Colour"] is Colour))
{
Assert.Fail("ViewData does not contain a Colour.");
return;
}
var actualColour = (Colour)viewResult.ViewData["Colour"];
Assert.That(actualColour, Is.EqualTo(theExpectedColour));
}
The method signature was changed to take the name of the method to be called and the expected colour. The call to homeController.About() is now carried out using reflection. This test passed and so I added a new Row attribute to test the call to homeController.Index().

[RowTest]
[Row("About", Colour.blue)]
[Row("Index", Colour.red)]
[Category("Unit")]
public void About_AboutActionCalled_ViewDataContainsExpectedColour(string methodName, Colour expectedColour)
{
// arrange
var theExpectedColour = expectedColour;
// act
var homeController = new HomeController();
// Use reflection to call the action method
var controllerType = homeController.GetType();
var methodToTest = controllerType.GetMethod(methodName);
Assert.That(methodToTest, Is.Not.Null, string.Format("No method called {0} found.", methodName));
var viewResult = methodToTest.Invoke(homeController, null) as ViewResult;
// assert
Assert.That(viewResult, Is.Not.Null);
if (viewResult == null || !(viewResult.ViewData["Colour"] is Colour))
{
Assert.Fail("ViewData does not contain a Colour.");
return;
}
var actualColour = (Colour)viewResult.ViewData["Colour"];
Assert.That(actualColour, Is.EqualTo(theExpectedColour));
}
Adding new tests for this scenario simply becomes a case of adding more Row attributes. I made a couple of final changes before I was happy with this test. I felt the reflection code distracted from the intent of the test, so I used the "Extract Method" refactoring to pull that code out into a separate method. I also renamed the methodName parameter to actionName. Using methodName wasn't wrong, I just feel that actionName describes the intent of what is being tested better. We are calling action methods on a controller. It's just a little more explicit than saying we are calling methods on a controller. Also, my naming convention is not quite right for this test because About is no longer the method under test. I decide to use HomeController_ActionCalled_ViewDataContainsExpectedColour as a method name to describe the intent of the test.

[RowTest]
[Row("Index",Colour.red)]
[Row("About",Colour.blue)]
[Category("Unit")]
public void HomeController_ActionCalled_ViewDataContainsExpectedColour(string actionName, Colour expectedColour)
{
// arrange
var theExpectedColour = expectedColour;
// act
var homeController = new HomeController();
var viewResult = CallControllerMethodAndReturnViewResult(homeController, actionName) ;
// assert
Assert.That(viewResult, Is.Not.Null);
if (viewResult == null || !(viewResult.ViewData["Colour"] is Colour))
{
Assert.Fail("ViewData does not contain a Colour.");
return;
}
var actualColour = (Colour)viewResult.ViewData["Colour"];
Assert.That(actualColour, Is.EqualTo(theExpectedColour));
}

private static ViewResult CallControllerMethodAndReturnViewResult(Controller controller, string methodName)
{
var controllerType = controller.GetType();
var methodToTest = controllerType.GetMethod(methodName);
Assert.That(methodToTest, Is.Not.Null, string.Format("No method called {0} found.", methodName));
return methodToTest.Invoke(controller, null) as ViewResult;
}

I like the result of this refactoring. The code is neat, compact and easily understandable. Any developer looking at this code should quickly be able to understand the expected behaviour of the code under test.



No comments:

Post a Comment