Jeffrey Palermo (.com)

Sponsors

The Lounge

Wicked Cool Jobs

News

Advertisement

Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
This is how ASP.NET MVC controller actions should be unit tested

I'm upgrading my partywithpalermo.com website for the MVP Summit party, and I'm retrofitting my tests to use the March CTP of the MVC Framework.  I have the following action that I need to unit test:

public class MainController : ControllerBase
{
    private readonly IAttendeesRepository _repository;
 
    public MainController(IAttendeesRepository repository, IViewEngine viewEngine)
    {
        _repository = repository;
        ViewEngine = viewEngine;
    }
 
 
    public void Register(string name, string website, string comment)
    {
        var attendee = new Attendee(name, website, comment);
        _repository.SaveNewAttendee(attendee);
        RenderView("confirm", attendee);
    }
}

 

Note the explicit dependencies on IAttendeeRepository and IViewEngine.  That means that I'll be interacting with those two dependencies in this controller.  Here is my unit test (this passes, by the way):

 

[Test]
public void ShouldSaveAttendee()
{
    var repository = new FakeRepository();
 
    var mockViewEngine = new MockViewEngine();
    var controller = new MainController(repository, mockViewEngine);
    controller.ControllerContext = new ControllerContext(new RequestContext(new HttpContextStub(), new RouteData()), controller);
    controller.Register("Jeffrey.',", "http://www.jeffreypalermo.com?=&@%20", "this comment!?,'.");
 
    Attendee attendee = repository.SavedAttendee;
    Assert.That(attendee.Name, Is.EqualTo("Jeffrey.',"));
    Assert.That(attendee.Website, Is.EqualTo("http://www.jeffreypalermo.com?=&@%20"));
    Assert.That(attendee.Comment, Is.EqualTo("this comment!?,'."));
 
    Assert.That(mockViewEngine.ActualViewContext.ViewName, Is.EqualTo("confirm"));
    Assert.That(mockViewEngine.ActualViewContext.ViewData, Is.EqualTo(attendee));
}
 
private class HttpContextStub : HttpContextBase
{
 
}

 

This is a pretty straightforward unit test except for the line before calling the Register() method.  I have to use setter-injection to set a stubbed ControllerContext.  If I don't, the Register() method will bomb with an exception when the ViewContext is created, and the code tries to get an HttpContextBase off of the ControllerContext.  It'll throw a NullReferenceException.  My code doesn't really care about the ControllerContext or what is in it, but because of how the code is structured, I must use setter injection to break this dependency.  Note that testability is next up on the list of things to do for the MVC Framework team.

Preview2 (March CTP) was all about getting the routing engine out into its own assembly so that it can be used separate from MVC controllers.  Also, the MVC Framework is "binnable".  You can xcopy deploy it.  There is plenty of time to fix these things, and the team is working on it.  You can also be sure that I'll keep raising the issue because I've been test-driving code for three years, and it's instinct now to see what is easy and frictionless and separate it from code that is harder to test than it should be.  Overall, pretty good for a CTP.

 

The following is what I would like to write.  The following is how I would like my test to look.  Notice just the absence of the ControllerContext line.

[TestFixture]
public class MainControllerTester
{
    [Test]
    public void ShouldSaveAttendee()
    {
        var repository = new FakeRepository();
 
        var mockViewEngine = new MockViewEngine();
        var controller = new MainController(repository, mockViewEngine);
        controller.Register("Jeffrey", "http://www.jeffreypalermo.com", "this comment");
 
        Attendee attendee = repository.SavedAttendee;
        Assert.That(attendee.Name, Is.EqualTo("Jeffrey"));
        Assert.That(attendee.Website, Is.EqualTo("http://www.jeffreypalermo.com"));
        Assert.That(attendee.Comment, Is.EqualTo("this comment"));
 
        Assert.That(mockViewEngine.ActualViewContext.ViewName, Is.EqualTo("confirm"));
        Assert.That(mockViewEngine.ActualViewContext.ViewData, Is.EqualTo(attendee));
    }
 
    private class MockViewEngine : IViewEngine
    {
        public ViewContext ActualViewContext;
 
        public void RenderView(ViewContext viewContext)
        {
            ActualViewContext = viewContext;
        }
    }
 
    private class FakeRepository : IAttendeesRepository
    {
        public Attendee SavedAttendee;
 
        public IEnumerable<Attendee> GetAttendees()
        {
            throw new NotImplementedException();
        }
 
        public void SaveNewAttendee(Attendee attendee)
        {
            SavedAttendee = attendee;
        }
    }
}

 

Note that my unit test is concerned with explicit dependencies and doesn't know or care that I'm making use of a PROTECTED method named "RenderView" inside my action.  That detail doesn't matter because the interaction with the IViewEngine is what is important. 

I also understand that I could use the Legacy code pattern of "Extract and Override Call" (Feathers, p348).  Microsoft has already provided the extracted method, RenderView.  I can override the call to break the dependency, but that pattern is meant to be used as a dependency-breaking technique with legacy code.  If you haven't read Michael Feathers' book, Working Effectively with Legacy Code, you should order it right now.  It talks about all kinds of dependency-breaking techniques in order to write unit tests on existing code.  My goal in providing feedback to the MVC Framework team (and I provide feedback, believe me) is to have this framework be something that I would want to use.  There is plenty of time to make the necessary changes, and the team is working hard.

I revised this post a bit after chatting on the phone with Scott Guthrie this evening.  We'll see some testability improvements in the next drop, and the team is aiming to make drops around every 6 weeks or so.

Note:  I'm playing around with using "var" for locals.  Not sure if I like it yet.  We'll see.  No need to comment on the "vars".  Comment on controllers, actions, and unit tests.


Posted Sun, Mar 9 2008 3:22 PM by Jeffrey Palermo

[Advertisement]

Comments

Justice~! wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 6:32 PM

You beat me to this post, but I'll probably post about it anyway.  This to me is one of the more frustrating choices and I totally agree.  We shouldn't have to try to find workaround for something designed fresh.  

It's more frustrating to me given we've already been through 6 mos worth.  I really hope it is fixed in the next drop, or at least a valid explanation given as to why it's not available.

Paul wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 6:44 PM

The var keyword should be banished from anything but Linq queries.

Notice how similar the following is to what we VB'ers do today:

"var" repository = new FakeRepository();

and the corresponding VB version....

"dim" respository "as" new FakeRepository()

Using the var in this manner makes a code smell to high heaven! What the heck is "repository" without using R# or go to definition. So much for self-documenting code......

Simone wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 7:02 PM

Now it will not work since if you that way you get a "null reference exception" because the controller context has not been set. Isn't it?

Scott Bellware wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 7:43 PM

Paul,

> What the heck is "repository" without using R# or go to definition.

> So much for self-documenting code

It's a repository.  I trust that Jeffrey will always use appropriate variable names, so I know that the variable name is likely the name of the type or a subtype (especially when mocking or stubbing).

If the variable has to be something other than the type, I still don't have to be interested in the type unless I can't derive the intention of the code from its use or literate style and appropriate language.

I agree that the "var" keyword should be struck from the language. I'd rather see a more Ruby-esque mode of variable declaration like:

repository = new Repository();

Explicit type declaration prefixes on variable declaration statements isn't a significant part of readability.  The extents of language-oriented programming that can be achieved in untyped and dynamic languages are a testament to this.

Scott Bellware wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 7:56 PM

ScottGu,

I don't believe Jeffrey's comments went to the MERE POSSIBILITY of testing this code, but the FRICTION involved in doing so.

TDD isn't about just being able to test, it's about being able to design clean, clear interfaces, and we look at the amount of ancillary dependencies showing up in test scripts as an unequivocal indicator of design problems.

Can this test be effected using a design that doesn't require these test helpers, or at least requires less of them?

Derik Whittaker wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 8:01 PM

Jeff,

What is the point of using var's for your locals?  You doing something where you need the variable to possibly be dynamic?  

Personally I am not a fan of using var's yet.  I don't think i see a point in not using a strongly typed variable unless you are doing something with linq or something where you need the variable to be dynamic.

Scott Bellware wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 8:02 PM

Jeff,

I'd like to see what you come up with after working with the controller testing abilities in RSpec for Rails.

I find it distressing that I would have to mock a view engine to test whether a controller asked for one view or another to be rendered.

I find the extend and override solution to lead to clearer test code, but I still don't feel it as a "good" or "right" solution.  I'm spoiled by Ruby.  My expectations have been set pretty high by what I see as a much better implementation of an MVC framework using a much friendlier language.

I think I'd rather see this done with a generic controller test base class that knows the type of the controller, can provide an instance of the controller to test and can prove that expectations have been met.  I think this is closer to a solution for good clear test code that you and Phil are dancing around the perimeter of.

Scott Bellware wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 8:13 PM

Jeff,

I know I'm on your back about these things all the time, but there's value in making controllers RESTful and resource-oriented.

You would end up with a RegistrationsController rather than a MainController.  The action to create a new registration would simply be: Create.  You'd POST to Registrations/Create.  To get the new registration form, you'd GET Registrations/New.

Resource controllers keep things simple by allowing you to have only a standard set of verbs that reflect the underlying resource-oriented protocols like HTTP:

- New (GET)

- Create (POST)

- Update (POST)

- Delete (or Destory) (POST)

- List (GET)

With a limited set of action names to work with, the work is in figuring out the resource names.  Meaningful and appropriate resource names help your addresses to be more discoverable and often more sensible.

ScottGu wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 8:26 PM

@Scott Bellware,

>>>>>>> Can this test be effected using a design that doesn't require these test helpers, or at least requires less of them?

Yes - of course it can.  You can replace this helper method:

          mocks.SetFakeControllerContext(controller);

with these three lines of more explicit code if you prefer:

          //

          // Construct mock HttpContext and ControllerContext and explictly inject onto Controller

          //

          HttpContextBase mockHttpContext = mocks.FakeHttpContext();

          ControllerContext mockControllerContext = new ControllerContext(new RequestContext(mockHttpContext, new RouteData()), controller);

          controller.ControllerContext = mockControllerContext;

All of the objects and methods (with one exception) above are with out of the box objects and methods in the shipping MVC Preview 2.  

The one exception is the mocks.FakeHttpContext() method.  This is a simple helper that constructs a mock HttpContext using RhinoMocks.  For completeness here is its complete implementation of it below:

       public static HttpContextBase FakeHttpContext(this MockRepository mocks)

       {

           HttpContextBase context = mocks.PartialMock<HttpContextBase>();

           HttpRequestBase request = mocks.PartialMock<HttpRequestBase>();

           HttpResponseBase response = mocks.PartialMock<HttpResponseBase>();

           HttpSessionStateBase session = mocks.PartialMock<HttpSessionStateBase>();

           HttpServerUtilityBase server = mocks.PartialMock<HttpServerUtilityBase>();

           SetupResult.For(context.Request).Return(request);

           SetupResult.For(context.Response).Return(response);

           SetupResult.For(context.Session).Return(session);

           SetupResult.For(context.Server).Return(server);

           mocks.Replay(context);

           return context;

       }

There is much more work planned to make the testing workflow simpler with future previews.  There are definitely certain testing aspects that the team feels needs to be improved (a point that all MVC preview 2 presentations from the MVC team members have explictly called out).  

The biggest work items and refactorings for MVC Preview 2 were in the routing engine layer - since other components in .NET will start relying on it shortly.  Now that that is done you'll see a bigger focus on refactoring and polishing the testing workflow of controllers with the preview releases ahead.

- Scott

Scott wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 8:59 PM

@Paul

> What the heck is "repository" without using R# or go to definition

How is:

 var repository = new FakeRepository();

less readable than:

 FakeRepository repository = new FakeRepository();

Do you need to see the type name twice to make sure it's *really* a fake repository?

P.S. var != dynamic or loose

jdn wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Sun, Mar 9 2008 11:46 PM

Definitely agree with Odetocode Scott (there's a proliferation of Scotts in this comment chaing).  I think var is definitely useful here in terms of cutting down on the amount of code you need to type, and it is strongly typed.

Simone wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Mon, Mar 10 2008 4:16 AM

@ScottGu,

probably already in the pipeline, but what I'd like to see is a TestViewEngine that is part of a System.MVC.Testing.dll that can be used for testing Controllers.

Helper methods are great, but feel like something you make because the library you are working with is suboptimal.

DotNetKicks.com wrote This is how ASP.NET MVC controller actions should be unit tested
on Mon, Mar 10 2008 4:54 AM

You've been kicked (a good thing) - Trackback from DotNetKicks.com

Dew Drop - March 10, 2008 | Alvin Ashcraft's Morning Dew wrote Dew Drop - March 10, 2008 | Alvin Ashcraft's Morning Dew
on Mon, Mar 10 2008 8:01 AM

Pingback from  Dew Drop - March 10, 2008 | Alvin Ashcraft's Morning Dew

Links Today (2008-03-10) wrote Links Today (2008-03-10)
on Mon, Mar 10 2008 11:19 AM

Pingback from  Links Today (2008-03-10)

Dan wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Mon, Mar 10 2008 12:58 PM

IMHO, I believe too many things are being tested under this one test case. This test is testing, the setting of the attendee properties, that save was called, and that RenderView was called appropriately. This test would look more simple/readable as a interaction based test  - rather than a state based test.

If you wanted to test that the attendee's property values were set as expected I believe that should be another test - or at least rename this test more appropriately.  Also, recommend using a mock framework such as NMock or RhinoMock.  I've found the overhead of creating and maintaining 'fake' or stub classes to be time consuming and troublesome when small changes occur.

This is more what I had in mind:

[SetUp]

public void setup()

{

 context = new ControllerContext(new RequestContext(new HttpContextStub(), new RouteData()), controller);

}

[Test]

public void ShouldSaveAttendee()

{    

 var mockRepository =  mocks.NewMock<IAttendeesRepository>();    

 var mockViewEngine = mocks.NewMock<IViewEngine>();  

 Expect.Method("SaveNewAttendee").On(repository);

 var controller = new MainController(mockRepository, mockViewEngine);

 controller.ControllerContext = context;

 controller.Register("Jeffrey.',", "www.jeffreypalermo.com, "this comment!?,'.");

 mocks.VerifyAllExpectationsHaveBeenMet();

}

To test that the attendee properties were actually set to the correct values should actually tested at the Attendee class level (since the attendee constructor is doing that work). Thoughts?

Jeffrey Palermo wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Mon, Mar 10 2008 3:16 PM

@Dan,

I use Rhino Mocks where it is easier than using nested classes.  In this case, I opted for nested classes.

I don't fall to either absolute on the spectrum.  I could break each assert out into its own test case, but in this particular case, I strike a balance based on reducing friction.  

Regarding the Attendee's constructor.  I need to test that the constructor is called with the appropriate arguments.

Jeremy Gray wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Mon, Mar 10 2008 3:20 PM

@Derik - var _is_ strong typed.

ScottGu wrote re: This is how ASP.NET MVC controller actions should be unit tested
on Tue, Mar 11 2008 3:47 AM

Hi Simone,

>>>>>> probably already in the pipeline, but what I'd like to see is a TestViewEngine that is part of a System.MVC.Testing.dll that can be used for testing Controllers.  Helper methods are great, but feel like something you make because the library you are working with is suboptimal.

One of the things the team is working on right now is some refactorings to not require the need to create or assign mocks for common scenarios (like the one Jeffrey was doing above).  Phil Haack is probably going to be blogging about some of the approaches they are looking to enable in the next few weeks.

Thanks,

Scott

php code and scripts » Blog Archive » This is how ASP.NET MVC controller actions should be unit tested wrote php code and scripts &raquo; Blog Archive &raquo; This is how ASP.NET MVC controller actions should be unit tested
on Sat, Mar 15 2008 5:43 PM

Pingback from  php code and scripts  &raquo; Blog Archive   &raquo; This is how ASP.NET MVC controller actions should be unit tested

SitePoint Blogs » .NET on the ‘NET March 10-17: SubSonic Rocks and MVC is Hawt wrote SitePoint Blogs &raquo; .NET on the &#8216;NET March 10-17: SubSonic Rocks and MVC is Hawt
on Wed, Mar 19 2008 10:11 PM

Pingback from  SitePoint Blogs &raquo; .NET on the &#8216;NET March 10-17: SubSonic Rocks and MVC is Hawt

Devlicio.us