Test your constructors to be simple

In the previous blog post I wrote how it is important to have a simple constructor that only accepts dependencies. And violation of this rule cost me at least 12 hours of debugging and blocking issue for some developers in the team. Now I have a unit test to prevent this kind of problem in the future.

One of the suggestions in the comments was to use strict mocks and inject them into a controller. And don’t set up any kind of expectations on mocks. This way if dependencies are doing anything in the controller, they will fail the test.

Here is a simple example. Suppose my controller looks like this:

public partial class AccountController : Controller
{
    private readonly IUserRepository userRepository;

    public AccountController(IUserRepository userRepository)
    {
        this.userRepository = userRepository;
    }
    // actions to follow
}

Naive test would look like this:

    [Test]
    public void Constructor_Created_NoWorkHappening()
    {
        var mock = new Mock<IUserRepository>(MockBehavior.Strict);

        var sut = new AccountController(mock.Object);
    }

This test would pass, but as soon as I add userRepository.Find(0); to the constructor, the test fails. That is a good start.

As you can imagine, with adding more of controllers and dependencies maintaining of naive tests will be too much work. And I don’t like extra work, especially in maintaining tests. And in our project we have about a hundred of controllers with a lot more dependencies. So I sat down and used some reflection to help me out.

During writing of the test, it turned out that Moq isolation framework does not support creation of mock-objects by supplying a type in run-time. It insisted on knowing of mocked-type in compile time. In other words, I could not do this:

var controllerType = typeof(AccountController);
var mock = new Mock(controllerType);

I could have used reflection again to trick Moq into doing what I need, but I fancied trying out other isolation frameworks. NSubstitute looks like a great framework and can create a mock by providing Type object:

var controllerType = typeof(AccountController);
var mock = Substitute.For(new Type[] { controllerType }, new object[] {});

The only snag was that NSubstitute does not support strict mocks. So I had to jump some hoops to get what I needed:

if(mock.ReceivedCalls().Count() != 0)
{
    Assert.Fail("Some method was called on strict mock");
}

At the end of the day this worked out quite nice, comparing to the strict mock by definition.

Apart from being a simple constructor, I wanted to make sure that all my controllers are working against abstractions, i.e. interfaces. So I had to check that all controller parameters are actually interfaces. The first test in the listing below is to check that.

And one more problem I had to solve before I can show you the listing: I have ILoggingService dependency that is used throughout. And before I can start using the logging service, one must call .SetLoggerName("blah") to define logger name, so log messages would come up against correct names. And call to this method must be done in controller constructor. This complication translates into a following requirement: All controller dependencies must not have any methods called in the constructor, unless the dependency is ILoggingService where one absolutely must call SetLoggerName and nothing else.

Here is the full code of the test:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Web.Mvc;
using MyApp.Domain.Repositories;
using MyApp.Domain.Services;
using MyApp.Web;
using MyApp.Web.Controllers;
using NSubstitute;
using NSubstitute.Exceptions;
using NUnit.Framework;

namespace Web.Tests.Controllers
{
    public class AllControllersConstructorTests
    {

        private static List<Type> GetControllerTypes()
        {
            return Assembly.GetAssembly(typeof(MvcApplication))
                .GetTypes()
                .Where(t => !t.IsAbstract && t.IsSubclassOf(typeof(Controller)))
                 // I'm using T4MVC. Don't care about these controllers
                .Where(t => t.Namespace != null && !t.Name.Contains("T4MVC")) 
                .ToList();
        }

        [Test]
        public void Constructors_OnlyDepend_OnInterfaces()
        {
            var errors = new List<String>();
            var controllerTypes = GetControllerTypes(); // get list of types of your controllers 

            foreach (var controllerType in controllerTypes)
            {
                // you can have more than one constructor. Make sure to check all of them
                var constructors = controllerType.GetConstructors(); 
                foreach (var ctor in constructors)
                {
                    var parameters = ctor.GetParameters();  // constructor arguments
                    foreach (var parameter in parameters)
                    {
                        var parameterType = parameter.ParameterType;
                        if (!parameterType.IsInterface)
                        {
                            var message = String.Format("Constructor in type {0} is using non interfaced object {1}", 
                                controllerType, parameter.ParameterType);
                            errors.Add(message);
                        }
                    }
                }
            }

            var finalMessage = String.Join(Environment.NewLine, errors);
            Assert.IsEmpty(errors, finalMessage);
        }


    [Test]
    public void Constructors_Only_ExpectedBehaviour()
    {
        var errors = new List<String>();
        var controllerTypes = GetControllerTypes(); // get list of types of your controllers

        foreach (var controllerType in controllerTypes)
        {
            // you can have more than one constructor. Check all of them
            var constructors = controllerType.GetConstructors();
            foreach (var ctor in constructors)
            {
                var mocks = new List<object>();
                foreach (var parameter in ctor.GetParameters()) // constructor arguments
                {
                    var parameterType = parameter.ParameterType;

                    // create a mock for every constructor argument
                    mocks.Add(Substitute.For(new Type[] { parameterType }, new object[] { }));
                }

                // and create an instance of the controller with mocked out dependencies
                Activator.CreateInstance(controllerType, mocks.ToArray());

                // now verify that mocks are strict
                foreach (var mock in mocks)
                {
                    // if the dependency is a logger, make sure we call only SetLoggerName method.
                    var logger = mock as ILoggingService;
                    if (logger != null)
                    {
                        try
                        {
                            logger.Received(1).SetLoggerName(Arg.Any<String>());
                        }
                        catch (ReceivedCallsException)
                        {
                            errors.Add(String.Format("ILoggingService was not set a name in constructor of controller {0}", controllerType));
                        }
                        if (logger.ReceivedCalls().Count() > 1)
                        {
                            var calledMethods = String.Join(", ", logger.ReceivedCalls().Where(c => c.GetMethodInfo().Name != "SetLoggerName").Select(c => c.GetMethodInfo().Name));
                            errors.Add(String.Format("In the constructor of {0} dependency ILoggingService method(s) {1} was called", controllerType, calledMethods));
                        }
                    }
                    else if (mock.ReceivedCalls().Count() != 0)
                    {
                        var calledMethods = String.Join(", ", mock.ReceivedCalls().Select(c => c.GetMethodInfo().Name));
                        var message = String.Format("In the constructor of {0} dependency {1} had method(s) called {2}", controllerType, mock, calledMethods);
                        errors.Add(message);
                    }
                }
            }
        }

        // your error messages are stored in the array and if there are errors, this nicely shows all 
        // the errors in one place. 
        var finalMessage = String.Join(Environment.NewLine, errors);
        Assert.IsEmpty(errors, finalMessage);
    }
}

The test is massive! But it covers a lot of ground and makes sure that all developers in the team are following the best practices. Here I’m showing you how I test constructors, but you can extend this for all your dependencies and test that all your classes are following the best Dependency Injection practices.