Wednesday, 16 February 2011

Unit Testing Best Practice

I'll start this off with the things I think make good best practice for unit tests. Please feel free to add more or to correct me in the comments.

A quick summary of the principles under discussion can be found here. This page refers to using a controller factory to satisfy controller dependencies - just note that thinking is now moving towards using an IOC container and MVC3's IDependencyResolver alongside .Net 4's PreApplicationStartMethod attribute - I'll make another post on this, so we can discuss that there.

OK, on to the list:

  • Each unit of functionality should have at least* one unit test, or a good explanation as to why not.

    *I'm a bit ambivalent about this. If you've got a unit of functionality that can have, say, two states (maybe it's got a big if statement in the middle), would we better off having a unit test for each state, or a single unit test for both? I'm of the opinion that unit test should be as succinct as possible, so am leaning towards multiple tests on this one.

  • Each unit test should test strictly one unit of functionality. We need to be very careful that we're not accidentally testing multiple things in one test. Once way do achieve this is to ensure that all dependencies are satisfied with a test double of some kind.

  • Unit tests should consist of three phases: Arrange, Act and Assert. As some unit tests involve a lot of test doubles and therefore have rather long arrange phases, readability can be improved by grouping the phases of the unit test with //Arrange //Act and //Assert comments.

  • System Under Test (SUT) refers to the class being tested. It is conventional for the instance of this class in the unit test be called target. As before, this aids readability and debugging.

  • None of our client classes should be written against concrete implementations of the service classes that they use; we should always be coding against interfaces. This may not be possible when using some CLR types, over which we have no control, but there are ways of mocking out some of the these (HttpContext is a good example).

  • Controllers should not refer to System.Web.HttpContext.Current. ControllerContext.HttpContext should be used instead - this is preferable because ControllerContext is a public property on the controller class and as such can be replaced with a test double, allowing the HTTPContext to be mocked.
That should be enough for starters. A post on IDependencyResolver to come.

4 comments:

  1. After reading Martin Fowler http://martinfowler.com/articles/mocksArentStubs.html

    I was a little confused...
    Within the context of this piece of code

    public ActionResult Index()
    {
    var event = _eventsRepository.Get(x=> x.date > DataTime.now)
    return view(event)
    }

    What should we test?

    - that a specific call was made to the repository
    - that Index returns a view
    - that the view data is valid ?

    Wait a second that 3 things!

    the test
    public void Index_retruns_a_view_with_event_data()
    {
    // Arrange
    var _eventRepository = GenerateMock>();
    _eventRepository.Expect(x => x.Get(y => y.date > DateTime.Now)).Return(new Event{ ... });

    HomeController controller = new HomeController(_eventsReposiory);

    // Act
    ViewResult result = controller.Index() as ViewResult;

    // Assert
    _eventsRepository.VerifiAllExpectaions();
    Assert.IsNotNull(result);
    Assert.AreEqual(MyEvent, result.viewData);

    }

    Is this OK? or are we better using stubs?
    what do you think ?

    ReplyDelete
  2. I guess we don't need to check that the view data is valid - that's the responsibility of the repository and should be tested elsewhere? So really I think we need to test that:

    - the action returns the right thing (i.e. a view)
    - the right call is made into the repository
    - the filter used is what we expect

    That can all be done with a Mock.

    What do you think?

    ps. Might be easier to test if the filter was inside the repository?

    ReplyDelete
  3. I would agree about the view data. Interesting question about the Repository. we could have a specific method for each Filtered Call. we are going to use, but it seems a little verbose. I think it depends on how much reuse we can get out of that repository method.

    ReplyDelete
  4. Agreed - until now we've tended to put generally reusable filters in the repository, with less general stuff being done in the controllers - makes sense to me. So for controller unit tests, I guess it depends on what the controller is doing; if it's using a filter, we can check it with a Mock, if the filter is in the repository, we don't have to worry about it.

    ReplyDelete