What are the possible problems with unit testing ASP.NET MVC code in the following way?

asp.net-mvc-3, unit-testing

Solution

Hmm. a few things here mate.

First, it looks like you're trying to test the a controller method. Great :)

So this means, that anything the controller needs, should be mocked. This is because

- You don't want to worry about what happens inside that dependency.

- You can verify that the dependency was called/executed.

Ok, so lets look at what you did and I'll see if i can refactor it to make it a bit more testable.

-REMEMBER- i'm testing the CONTROLLER METHOD, not the stuff the controller method calls/depends upon.

So this means I don't care about the service instance or the repository instance (which ever architectural way you decide to follow).

NOTE: I've kept things simple, so i've stripped lots of crap out, etc.

Interface

First, we need an interface for the repository. This can be implemented as a in-memory repo, an entity framework repo, etc.. You'll see why, soon.

public interface ILESRepository
{
    IQueryable<Person> GetAll();
}

Controller

Here, we use the interface. This means it's really easy and awesome to use a mock `IRepository` or a real instance.

public class PersonController : Controller
{
    private readonly ILESRepository _repository;

    public PersonController(ILESRepository repository)
    {
       if (repository == null)
       {
           throw new ArgumentNullException("repository");
       }
        _repository = repository;
    }

    public ActionResult Index(int id)
    {
        var model = _repository.GetAll<Person>()
            .FirstOrDefault(x => x.Id == id);

        var viewModel = new VMPerson(model);
        return View(viewModel);
    }
}

Unit Test

Ok - here's the magic money shot stuff. First, we create some Fake People. Just work with me here... I'll show you where we use this in a tick. It's just a boring, simple list of your `POCO`'s.

public static class FakePeople()
{
    public static IList<Person> GetSomeFakePeople()
    {
        return new List<Person>
        {
            new Person { Id = 1, Name = "John" },
            new Person { Id = 2, Name = "Fred" },
            new Person { Id = 3, Name = "Sally" },
        }
    }
}

Now we have the test itself. I'm using `xUnit` for my testing framework and `moq` for my mocking. Any framework is fine, here.

public class PersonControllerTests
{
    [Fact]
    public void GivenAListOfPeople_Index_Returns1Person()
    {
        // Arrange.
        var mockRepository = new Mock<ILESRepository>();
        mockRepository.Setup(x => x.GetAll<Person>())
                                   .Returns(
                                FakePeople.GetSomeFakePeople()
                                          .AsQueryable);
        var controller = new PersonController(mockRepository);
        controller.FakeOutContext();

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

        // Assert.
        Assert.NotNull(result);
        var model = result.Model as VMPerson;
        Assert.NotNull(model);
        Assert.Equal(1, model.Person.Id);
        Assert.Equal("John", model.Person.Username);

        // Make sure we actually called the GetAll<Person>() method on our mock.
        mockRepository.Verify(x => x.GetAll<Person>(), Times.Once());
    }
}

Ok, lets look at what I did.

First, I arrange my crap. I first create a mock of the `ILESRepository`. Then i say: If anyone ever calls the `GetAll<Person>()` method, well .. don't -really- hit a database or a file or whatever .. just return a list of people, which created in `FakePeople.GetSomeFakePeople()`.

So this is what would happen in the controller ...

var model = _repository.GetAll<Person>()
                       .FirstOrDefault(x => x.Id == id);

First, we ask our mock to hit the `GetAll<Person>()` method. I just 'set it up' to return a list of people .. so then we have a list of 3 `Person` objects. Next, we then call a `FirstOrDefault(...)` on this list of 3 `Person` objects .. which returns the single object or null, depending on what the value of `id` is.

Tada! That's the money shot :)

Now back to the rest of the unit test.

We `Act` and then we `Assert`. Nothing hard there. For bonus points, I `verify` that we've actually called the `GetAll<Person>()` method, on the mock .. inside the Controller's `Index` method. This is a safety call to make sure our controller logic (we're testing for) was done right.

Sometimes, you might want to check for bad scenario's, like a person passed in bad data. This means you might never ever get to the mock methods (which is correct) so you `verify` that they were never called.

Ok - questions, class?

Problem

Original source