1

I'm just looking into tdd with mvc 4.

I have an orderscontroller which takes a unit of work interface in its constructor:

public OrdersController(IUnitOfWork db)
{
this.db = db;           
}

//
// GET: /Orders/
public ActionResult Index()
{                      
    return View(db.Orders.GetAll());
}

I have a couple of tests for this Index().

[TestClass]
    public class when_the_order_controller_index_action_executes
    {
        [TestMethod]
        public void it_should_render_the_default_view()
        {
            var uow = new Mock<IUnitOfWork>();
            var db = uow.Object;

            var orders = new List<Order>()
            {
                new Order{CreatedDate = DateTime.Now.AddMonths( -3),OrderID = Guid.NewGuid()},
                new Order{CreatedDate = DateTime.Now,OrderID = Guid.NewGuid()}
            };

            uow.Setup(r => r.Orders.GetAll())
                .Returns(orders);

            //arrange
            var controller = new OrdersController(db);

            //act
            var result = controller.Index() as ViewResult;

            //assert
            Assert.AreEqual("", result.ViewName);
        }

        [TestMethod]
        public void it_should_pass_orders_as_the_model()
        {
             var uow = new Mock<IUnitOfWork>();
            var db = uow.Object;

            var orders = new List<Order>()
            {
                new Order{CreatedDate = DateTime.Now.AddMonths( -3),OrderID = Guid.NewGuid()},
                new Order{CreatedDate = DateTime.Now,OrderID = Guid.NewGuid()}
            };


            uow.Setup(r => r.Orders.GetAll())
                .Returns(orders);

            //arrange
            var controller = new OrdersController(db);

            //act
            var model = ((ViewResult)controller.Index()).ViewData.Model as IEnumerable<Order>;

            //assert
            Assert.IsTrue(orders.Equals(model));
        }
    }

You will notice there is quite a lot of code duplication in the two tests for creating a list that GetAll can return...

Is it good/bad practice to have say a re-usuable function to return that list that both test could call?

Any general advice on the tests I have written would be appreciated also as I'm only now embracing the tdd love!

1
  • I'm just learning TDD at the moment as well and I am just looking at your code and wondering if I've missed a concept with mocking/testing Why are you going to the trouble of actually putting data into your OUW.db mock in your first test? Since all you need the test to set as the model is null because all you're testing in that first method is that the Index view was returned to you; you're then covering the test that the Index action sets the model correctly in your second test. Not trying to be smart at all, just really wondering if I've missed some concept? Jun 16, 2013 at 20:57

1 Answer 1

3

Yes, it is generally considered good practice to refactor all common code initialization in a [TestInitialize] phase of your unit test. You could also externalize all common initialization/assertion parts of your unit tests that you feel quite repetitive into reusable methods.

So in your particular example:

[TestClass]
public class when_the_order_controller_index_action_executes
{
    private IUnitOfWork db;
    private OrdersController sut;

    [TestInitialize]
    public void TestInitialize()
    {
        var uow = new Mock<IUnitOfWork>();
        this.db = uow.Object;

        var orders = new List<Order>()
        {
            new Order{ CreatedDate = DateTime.Now.AddMonths(-3),OrderID = Guid.NewGuid() },
            new Order{ CreatedDate = DateTime.Now,OrderID = Guid.NewGuid() }
        };

        uow.Setup(r => r.Orders.GetAll()).Returns(orders);

        this.sut = new OrdersController(db);
    }

    [TestMethod]
    public void it_should_render_the_default_view()
    {
        //act
        var result = this.sut.Index() as ViewResult;

        //assert
        Assert.AreEqual("", result.ViewName);
    }

    [TestMethod]
    public void it_should_pass_orders_as_the_model()
    {
        //act
        var model = ((ViewResult)this.sut.Index()).ViewData.Model as IEnumerable<Order>;

        //assert
        Assert.IsTrue(orders.Equals(model));
    }
}

But usually the uow.Setup(r => r.Orders.GetAll()).Returns(orders); is part of the //arrange phase of each unit test as it differs. it's where you define the expectations. In your particular example I would also merge the 2 tests into a single one:

[TestClass]
public class when_the_order_controller_index_action_executes
{
    private IUnitOfWork db;
    private OrdersController sut;

    [TestInitialize]
    public void TestInitialize()
    {
        var uow = new Mock<IUnitOfWork>();
        this.db = uow.Object;

        var orders = new List<Order>()
        {
            new Order{ CreatedDate = DateTime.Now.AddMonths(-3),OrderID = Guid.NewGuid() },
            new Order{ CreatedDate = DateTime.Now,OrderID = Guid.NewGuid() }
        };

        uow.Setup(r => r.Orders.GetAll()).Returns(orders);

        this.sut = new OrdersController(db);
    }

    [TestMethod]
    public void it_should_render_the_default_view_and_pass_the_expected_view_model_to_it()
    {
        //act
        var actual = this.sut.Index();

        //assert
        Assert.IsInstanceOfType(actual, typeof(ViewModel));
        var viewResult = (ViewResult)actual;
        Assert.AreEqual(model, viewResult.Model);
    }
}
3
  • Thanks for that detailed reply. Is it not best practice to have one assert per test though?
    – Simon
    Jun 16, 2013 at 14:53
  • 1
    @Simon: "one assert" should be understood as "one logical concept", not one line of code with Assert.
    – k.m
    Jun 16, 2013 at 18:12
  • Note that for some testing libraries such as xUnit.net , the initialization code is in the test class constructor, while cleanup is done by making the test class implement IDisposable. Btw you should give a try to the FluentAssertions library as well
    – MikeSW
    Jun 17, 2013 at 7:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.