tags:

views:

77

answers:

3

I'm trying out TDD on a greenfield hobby app in ASP.NET MVC, and have started to get test methods such as the following:

[Test]
public void Index_GetRequest_ShouldReturnPopulatedIndexViewModel()
{
    var controller = new EmployeeController();
    controller.EmployeeService = GetPrePopulatedEmployeeService();

    var actionResult = (ViewResult)controller.Index();

    var employeeIndexViewModel = (EmployeeIndexViewModel)actionResult.ViewData.Model;
    EmployeeDetailsViewModel employeeViewModel = employeeIndexViewModel.Items[0];

    Assert.AreEqual(1, employeeViewModel.ID);
    Assert.AreEqual("Neil Barnwell", employeeViewModel.Name);
    Assert.AreEqual("ABC123", employeeViewModel.PayrollNumber);
}

Now I'm aware that ideally tests will only have one Assert.xxx() call, but does that mean I should refactor the above to separate tests with names such as:

  • Index_GetRequest_ShouldReturnPopulatedIndexViewModelWithCorrectID
  • Index_GetRequest_ShouldReturnPopulatedIndexViewModelWithCorrectName
  • Index_GetRequest_ShouldReturnPopulatedIndexViewModelWithCorrectPayrollNumber

...where the majority of the test is duplicated code (which therefore is being tested more than once and violates the "keep tests fast" advice)? That seems to be taking it to the extreme to me, so if I'm right as I am, what is the real-world meaning of the "one assert per test" advice?

+3  A: 

It seems just as extreme to me, which is why I a also write multiple asserts per test. I already have >500 tests, writing just one assert per test would blow this up to at least 2500 and my tests would take over 10 minutes to run.

Since a good rest runner (such as Resharper's) lets you see the line where the test failed very quickly, you should still be able to figure out why a test failed with little effort. If you don't mind the extra effort, you can also add an assert description ("asserting payroll number correct"), so that you can even see this without even looking at the source code. With that, there is very little reason left to just one assert per test.

Adrian Grigore
I agree, having one assert per test just leads to test method explosion. I too use the description, but mostly it's not actually needed as NUnit (the test runner I use) actually tells you which line failed.Having said that, if you refactor your test code test method explosion shouldn't result in any code duplication at all.
Antony Scott
I agree that code duplication is not an issue if you refactor the tests properly. However, the speed of running an entire test suite remains an issue.
Adrian Grigore
If 2,500 tests are taking 10 minutes then that's about 4 seconds per test. Arguably, these are integration tests, not unit tests.
Damian Powell
@Damian Powell: I just timed a run of my entire test suite. You are right, the tests are quite a bit faster than that - 24 seconds (even though it always seems like an eternity when I am waiting for them to finish...). So 2500 should take approximaltely 2 minutes. Not too bad, but I still prefer having to wait as little as possible. Then I wouldn't have to kill time by writing comments such as this one ;-)
Adrian Grigore
+2  A: 

In his book The Art of Unit Testing, Roy Osherove talks about this subject. He too is in favour of testing only one fact in a unit test, but he makes the point that that doesn't always mean only one assertion. In this case, you are testing that given a GetRequest, the Index method ShouldReturnPopulatedIndexViewModel. It seems to me that a populated view model should contain an ID, a Name, and a PayrollNumber so asserting on all of these things in this test is perfectly sensible.

However, if you really want to split out the assertions (say for example if you are testing various aspects which require similar setup but are not logically the same thing), then you could do it like this without too much effort:

[Test] 
public void Index_GetRequest_ShouldReturnPopulatedIndexViewModel() 
{
    var employeeDetailsViewModel = SetupFor_Index_GetRequest();
    Assert.AreEqual(1, employeeDetailsViewModel.ID);
}

[Test] 
public void Index_GetRequest_ShouldReturnPopulatedIndexViewModel() 
{
    var employeeDetailsViewModel = SetupFor_Index_GetRequest();
    Assert.AreEqual("Neil Barnwell", employeeDetailsViewModel.Name); 
}

[Test] 
public void Index_GetRequest_ShouldReturnPopulatedIndexViewModel() 
{
    var employeeDetailsViewModel = SetupFor_Index_GetRequest();
    Assert.AreEqual("ABC123", employeeDetailsViewModel.PayrollNumber); 
}

private EmployeeDetailsViewModel SetupFor_Index_GetRequest()
{
    var controller = new EmployeeController(); 
    controller.EmployeeService = GetPrePopulatedEmployeeService(); 

    var actionResult = (ViewResult)controller.Index(); 

    var employeeIndexViewModel = (EmployeeIndexViewModel)actionResult.ViewData.Model; 
    var employeeDetailsViewModel = employeeIndexViewModel.Items[0]; 

    return employeeDetailsViewModel;
}

It could also be argued that since these tests require the same setup, they should get their own fixture and have a single [SetUp] method. There's a downside to that approach though. It can lead to lots more unit test classes than actual, real classes which might be undesirable.

Damian Powell
I'm choosing your answer because I like the rationale of testing a **fact**, which may require some related **assertions**. Multiple calls to `Assert` isn't a problem, as long as they're part of a single "assertion".
Neil Barnwell
@Neil Cheers! I should point out though, that I don't think Roy uses the term *fact* exactly, and I'm afraid I can't remember what term he does use.
Damian Powell
+1  A: 

I use a helper class to contain the asserts. This keeps the test methods tidy and focused on what they're actually trying to establish. It looks something like:

public static class MvcAssert
{
    public static void IsViewResult(ActionResult actionResult)
    {
        Assert.IsInstanceOfType<ViewResult>(actionResult);
    }

    public static void IsViewResult<TModel>(ActionResult actionResult, TModel model)
    {
        Assert.IsInstanceOfType<ViewResult>(actionResult);
        Assert.AreSame(model, ((ViewResult) actionResult).ViewData.Model);
    }

    public static void IsViewResult<TModel>(ActionResult actionResult, Func<TModel, bool> modelValidator)
        where TModel : class
    {
        Assert.IsInstanceOfType<ViewResult>(actionResult);
        Assert.IsTrue(modelValidator(((ViewResult) actionResult).ViewData.Model as TModel));
    }

    public static void IsRedirectToRouteResult(ActionResult actionResult, string action)
    {
        var redirectToRouteResult = actionResult as RedirectToRouteResult;
        Assert.IsNotNull(redirectToRouteResult);
        Assert.AreEqual(action, redirectToRouteResult.RouteValues["action"]);
    }
}
AbstractCode