Controller that uses a tested repository (or service)

Question

I have a repository called PostsRepository who implements an interface called IPostsRepository:

public interface IPostsRepository
{
    IEnumerable<Post> All();
}

public class PostsRepository : IPostsRepository
{
    private readonly DatabaseContext context;

    public PostsRepository(DatabaseContext context)
    {
        this.context = context;
    }

    public IEnumerable<Post> All()
    {
        return context.Posts.OrderBy(post => post.PublishDate);
    } 
}

And I have a controller called HomeController who uses the aforementioned repository:

public class HomeController : Controller
{
    private readonly IPostsRepository repository;

    public HomeController(IPostsRepository repository)
    {
        this.repository = repository;
    }

    public ViewResult Index()
    {
        return View(repository.All());
    }
}

These classes are currently resoundingly simple because they are experimental. Now for each of these classes, I have unit tests:

[TestFixture]
public class PostsRepositoryTest
{
    private PostsRepository repository;
    private MockDbSet<Post> databaseSet;
    private Mock<DatabaseContext> databaseContext;

    [SetUp]
    public void SetUp()
    {
        databaseSet = new MockDbSet<Post>();
        databaseSet.SetupLinq();

        databaseContext = new Mock<DatabaseContext>();
        databaseContext.Setup(context => context.Posts).Returns(databaseSet.Object);

        repository = new PostsRepository(databaseContext.Object);
    }

    [Test]
    public void All_ReturnsAllPosts()
    {
        databaseSet.SetupSeedData(Enumerable.Repeat(new Post(), 2));

        var actual = repository.All();

        Assert.AreEqual(2, actual.Count());
    }

    [Test]
    public void All_ReturnsAllPostsOrderedByPublishDate()
    {
        databaseSet.SetupSeedData(new List<Post>
        {
            new Post { PublishDate = new DateTime(2014, 2, 1) },
            new Post { PublishDate = new DateTime(2014, 1, 1) },
        });

        var actual = repository.All();
        var expected = databaseSet.Data.OrderBy(post => post.PublishDate);

        Assert.AreEqual(expected, actual);
    }
}
[TestFixture]
public class HomeControllerTest
{
    private Mock<IPostsRepository> repository;
    private HomeController controller;

    [SetUp]
    public void SetUp()
    {
        repository = new Mock<IPostsRepository>();
        controller = new HomeController(repository.Object); 
    }

    [Test]
    public void Index_RendersCorrectView()
    {
        controller.WithCallTo(c => c.Index())
                  .ShouldRenderDefaultView();
    }

    [Test]
    public void Index_ReturnsCorrectModelType()
    {
        var actual = controller.Index().Model;
        Assert.That(actual, Is.AssignableTo<IEnumerable<Post>>());
    }

    [Test]
    public void Index_EmptyRepository_ReturnsNoPosts()
    {
        var actual = (IEnumerable<Post>) controller.Index().Model;

        Assert.AreEqual(0, actual.Count());
    }

    [Test]
    public void Index_ReturnsAllPosts()
    {
        var posts = Enumerable.Repeat(new Post(), 5);
        repository.Setup(repo => repo.All()).Returns(posts);

        var actual = (IEnumerable<Post>) controller.Index().Model;

        Assert.AreEqual(posts.Count(), actual.Count());
    }
}

As you can see, I have a test for the repository by the name of All_ReturnsAllPostsOrderedByPublishDate which asserts that the posts are ordered by their publish date.

I wonder if I should test the same thing in the controller. On one hand, I do not as feel as though testing this behaviour again will add any value. But on the other hand, I worry that the controller is not bound to use the repository and that if the controller stops depending on / using the repository class, the test will break. I am inclined to verify that the controller calls the repository but as I interpret it, this answerer says that I shouldn't.

With that concern in mind, would you please review the correctness of my unit tests? Please identify any other concerns.

Answer 1

The functionality of the Home Controller is not to be responsible for the order of the items, so I don't think you should validate that here.

When writing unit tests it's best to look at the class you are testing as a box. The box has information that comes in and information that goes out. Our single goal when unit testing is to validate what happens between what is going in and what is going out.

When writing tests I think the best place to start is to write out just the names of your test methods. This allows you to make sure you have a good high level approach that covers the main functionality. Once that is done, you can go through the code piece by piece and find smaller nuances that you want to test. Finally, you can run test coverage and see if there's anything missing. Be careful though! Full test coverage certainly does not mean that all of the functionality is tested.

In this specific example I would validate that the correct methods are called on the injected dependency.

What I would add though is a .Verify on the mock to make sure the correct methods are called in the injected dependency.

Since we are calling respository.All() in the controller, you should verify that.

It might look something like this:

repository.Verify(r => r.All(), Times.Once);

So what this is saying is: I am using an injected repository. Please make sure that after the code has been executed in this test that the .All() method has been called exactly once.

For ordering I think you have the test at the right level with your All_ReturnsAllPostsOrderedByPublishDate(). I would suggest that you make your expected explicit though.

While the code uses .OrderBy, here I would say "I am looking for a list with this as the first item and this as the second."

There are other options that you have if you want to enforce this ordering. The first option would be to add the OrderBy to the repository call before passing it to your view. This would be one way of enforcing the constraint. Another would be to create a ViewModel that takes the items in the repository and when it returns them they are ordered. Then you could add unit tests around either of those. Each would enforce the ordering irrespective of the repository functionality(in case you switch it out or change it.) In my personal opinion I think this would be preferred as the responsibility of ordering shouldn't really go in the repository. That is a requirement of your view and there may be other consumers of the repository that want the data in a different order, or un-ordered.

One other thing, I would suggest you use var only when the type is visible. For method calls I would declare the variable with its explicit type.