views:

352

answers:

2

Hello,

I'm new to MSpec and would like to know if the way I wrote my test for ASP.NET MVC is correct. The test passes but I don't really like the way it's written and it seems awkward. I'm certainly missing something.

public class AccountControllerTests3
{
    protected static AccountController controller;
    static IFormsAuthenticationService formsService;
    static IMembershipService membershipService;
    protected static ActionResult result;
    protected static LogOnModel model;

    Establish context = () =>
    {
        var controllerBuilder = new TestControllerBuilder();

        formsService = MockRepository.GenerateStub<IFormsAuthenticationService>();
        membershipService = MockRepository.GenerateStub<IMembershipService>();
        model = MockRepository.GenerateStub<LogOnModel>();

        controller =
            controllerBuilder.CreateController<AccountController>(new object[]
                                                                    {
                                                                        formsService,
                                                                        membershipService
                                                                    });
    };

    Because user_logs = () =>
    {
        bool rememberMe = false;

        membershipService.Stub(
            x => x.ValidateUser("bdd", "mspec")).Return(true);
        formsService.Stub(x => x.SignIn("bdd", rememberMe));

        controller.ModelState.IsValid.ShouldBeTrue();
    };
}

[Subject(typeof(AccountController), "LogInTests")]
public class When_logging_into_application_with_good_login_and_password : AccountControllerTests3
{
    private It user_should_be_redirected_to_the_home_page = () =>
                                                                {
                                                                    model.UserName = "bdd";
                                                                    model.Password = "mspec";

                                                                    result = controller.LogOn(model, string.Empty);

                                                                    result.AssertActionRedirect().ToAction<HomeController>(
                                                                        x => x.Index());
                                                                };
}

[Subject(typeof(AccountController), "LogInTests")]
public class When_logging_into_application_with_bad_login_and_password : AccountControllerTests3
{
    It The_error_message_should_be_shown = () =>
                                            {
                                                model.UserName = "BAD";
                                                model.Password = "BAD";

                                                result = controller.LogOn(model, string.Empty);

                                                controller.ModelState[""].Errors[0].ErrorMessage.ShouldEqual(
                                                    "The user name or password provided is incorrect.");
                                            };
}

Thanks in advance,

Thomas

+1  A: 

Here's a remark: instead of using CreateController method use InitializeController, because it is compile-time safer and refactor friendlier.

Instead of:

controller = controllerBuilder.CreateController<AccountController>(
    new object[] { formsService, membershipService });

Do:

controller = new AccountController(formsService, membershipService);
controllerBuilder.InitializeController(controller);

The first will still compile if you change the controller constructor arguments and it will blow at runtime, while the second will generate a compile-time error.

Darin Dimitrov
Thanks dor the tip. And for the rest of the test. Is it correct from the point of view of MSpec ?
Thomas Jaskula
+3  A: 

Hi Thomas,

One of my goals when I write tests with MSpec is to get the "Assert" or the "It" down to one line. MSpec is not like NUnit in that it only executes the Context (made up of the Establish clauses from the current class and all base classes and the Because clause) once followed by all of the Specifications (It clauses).

This is designed explicitly to force you to not perform any behavior in the It clauses, but rather observe it.

What you're actually doing here is using MSpec like NUnit. Try and rewrite the tests in a single class (using no inheritance). Work backwards... in the It, place a single line that asserts what you want to assert. Perhaps the AssertRedirect. In the Because, try and put a single line that causes the observations to be observable. This would probably be your call to the controller's logon method. Finally, in the "Establish context" you'd want to put everything else.

After a while, you may want to pull some of the things in the Establish context only into a base class, but in doing so, be sure that your entire subclass stands alone in terms of understanding. A reader shouldn't need to read the base class in order to understand what the actual spec is doing. It's ok to hide ceremonial implementation details, but be sure to hide them behind descriptive method names.

There's another line I'm not sure about:

controller.ModelState.IsValid.ShouldBeTrue();

If this is a test, it should probably be in its own It clause. Though really, do you want to test this? What are you testing here? Shouldn't your controller take an action based on whether or not the model is valid? Shouldn't the result of that action be observable (validation error instead of login error). I just wonder if you really need to test this.

A few other things to check out, first for styling with R#, it seems your tests are falling victim to R#'s defaults. I posted about how to fight this here:

http://codebetter.com/blogs/aaron.jensen/archive/2008/10/19/getting-resharper-and-vs-to-play-nice-with-mspec.aspx

Also, James Broome has some nice MVC MSpec extensions that are worth checking out:

http://jamesbroo.me/introducing-machinespecificationsmvc/

Good luck and Enjoy! Feel free to ping me on twitter if you have any other unrelated questions.

aaronjensen
Thanks for your response.I agree with you that I can get rid of the line controller.ModelState.IsValid.ShouldBeTrue();You say that the base class is to avoid. So should I place Because in two separates classes ?Thanks
Thomas Jaskula
Without a base class you have two different contexts. So yes, there would be two different Because clauses and two different Establish context clauses.
aaronjensen
Ok, so where I should define common configuration code as I did in Establish delegate ?
Thomas Jaskula
Please reread the paragraph beginning with "After a while...", I attempt to explain a more advanced scenario in which you start to use base classes.
aaronjensen