CODEWITHSUNDEEP
unit-testing
Neil Barnwell
Neil Barnwell

Reputation: 42125

Unit Testing in ASP.NET MVC: Minimising the number of asserts per test

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:

...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?

Upvotes: 1

Views: 321

Answers (3)

AbstractCode
AbstractCode

Reputation: 61

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"]);
    }
}

Upvotes: 1

Adrian Grigore
Adrian Grigore

Reputation: 33318

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.

Upvotes: 3

Damian Powell
Damian Powell

Reputation: 8775

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.

Upvotes: 2

Related Questions