Working With Assertions Made on Arguments in Rhino Mocks

 

Today when modifying what we call an “order notifier” (essentially observers that are notified when an order is placed), I was having trouble figuring out why my test was failing. The project is written in C# and this test was using Rhino Mocks to isolate the EmailService class. We obviously don’t want to test our code with an actual email implementation, so a mocking/isolation framework is a perfect tool for abstracting this out and making testing easier.

The TestFixture and Test I Wrote

Below is a very verbose version of the test class using NUnit. The actual tests use some shared methods for setting up the order and making assertions on the email service. I opted to show more code just so it’s easier to see all the setup and verification that was going on.

[TestFixture]

public class Tests

{

    private IEmailService emailService;

 

    [SetUp]

    public void SetUp()

    {

        emailService = MockRepository.GenerateMock<IEmailService>();

    }

 

    [Test]

    public void Notify_should_contain_OrderNumber_and_CustomerNumber_if_Order_NeedsShippingQuote()

    {

        const int customerNumber = 1337;

        const int orderNumber = 2401;

        var order = MockRepository.GenerateMock<IOrder>();

        order.Stub(x => x.OrderNumber).Return(orderNumber);

        order.Stub(x => x.CustomerNumber).Return(customerNumber);

        order.Stub(x => x.NeedsShippingQuote).Return(true);

 

        var notifier = GetNotifier();

 

        notifier.Notify(order);

 

        emailService.AssertWasCalled(x => x.SendOrderReceived(

            Arg<string>.Is.Anything, 

            Arg<string>.Matches(body => 

                body.Contains("This order needs a shipping quote.") &&

                body.Contains(customerNumber.ToString()) &&

                body.Contains(orderNumber.ToString())

            )));

    }

 

    public CustomerServiceNotifier GetNotifier()

    {

        return new CustomerServiceNotifier(emailService);

    }

}

The only additions to this test were those asserting that the email’s message body contained the customer number and the order number. With these new expectations set up on the CustomerServiceNotifier, it was time to write the code to make that expectation valid.

The Notifier Code

Some of this code was removed because it’s either not important to this post, or just too lengthy. This is a trimmed down version of the actual implementation.

public class CustomerServiceNotifier

{

    private readonly IEmailService emailService;

    private const string ORDER_NOTES_FORMAT = "{0}{1}";

    private const string SHIPPING_QUOTE_FORMAT = 

        "<br />This order needs a shipping quote:<br />Order #: {0}<br />Cust #:{1}";

 

    public CustomerServiceNotifier(IEmailService emailService)

    {

        this.emailService = emailService;

    }

 

    public void Notify(IOrder order)

    {

        var emailBuilder = new StringBuilder();

 

        if (order.HasNotes)

            emailBuilder.AppendFormat(ORDER_NOTES_FORMAT, order.OrderNumber, order.OrderNotes);

 

        if (order.NeedsShippingQuote)

            emailBuilder.AppendFormat(SHIPPING_QUOTE_FORMAT, order.OrderNumber, order.CustomerNumber);

 

        if(emailBuilder.Length > 0)

            emailService.SendOrderReceived(order.EmailAddress, emailBuilder.ToString());

    }

}

The notifier was changed to format the “needs shipping quote” message to contain the order number and the customer number. If you’ve already spotted the mistake, nice work, I didn’t find it so fast.

The failure stated the following:

IEmailService.SendOrderReceived(anything, body => ((body.Contains(“This order needs a shipping quote.”) && body.Contains(1337.ToString())) && body.Contains(2401.ToString()))); Expected #1, Actual #0.

I knew that the method was being called. I couldn’t figure out why it was giving me the error message at first. I decided to take advantage of Rhino Mocks’s WhenCalled method. This allows you to provide a delegate to do some fancy stuff when that method is called. I just decided to use some simple output so I could see the actual string that was being passed to the mock email service. That was done pretty easily:

[SetUp]

public void SetUp()

{

    emailService = MockRepository.GenerateMock<IEmailService>();

    emailService.Stub(x => x.SendOrderReceived(null, null)).IgnoreArguments()

        .WhenCalled(x => Console.WriteLine(x.Arguments[1]));

}

After I generate my mock, I just stub out the method, ignoring any arguments, and output the second argument to the console. I re-ran the test and it gave me the output like so:

<br />This order needs a shipping quote:<br />Order #: 2401<br />Cust #:1337

It only took me one look to see the problem and scream think: Doh!! The actual text contains a colon character after “quote” instead of my test which was expecting a period character. The character was changed, I removed the WhenCalled delegate set up and I was back to good. This idea seemed like a nice way to do some debugging without having to slowly step through the code.

Some Thoughts on the Problem

I’m all for writing good tests and adhering to best practices and while my unit test above technically only contains only one assertion from Rhino Mocks (a good idea), it’s actually making 3 assertions total (not so good). I’m checking that the email body contains some text, an order number and a customer number. I really should break this up into 3 separate tests since it’s checking 3 requirements within the email body. I guess I have something to do tomorrow morning. :)

Some Thoughts on Rhino Mocks WhenCalled Method

I’ve found myself using this more and more lately when stubbing out test setup. Primarily when using the WhenCalled method I’ve been thinking of it in my head as a “pass-through”. I want to ask for an object given some specific data and get back a fake object of that type that matches on the value I passed in. I’m not sure if the thought in my head is making sense, so I’ll give some sample code.

The following IScheduleResolver interface is meant to provide an ISchedule back when called. I don’t really want to set up a new fake object for all the variations passed in, I just want to return a schedule that is valid (or possibly invalid) for that time I provided. Here’s the setup of this idea:

scheduleResolver.Stub(x => x.GetCurrentSchedule(null))

    .IgnoreArguments().WhenCalled(a =>

        {

            var fakeSchedule = MockRepository.GenerateMock<ISchedule>();

            fakeSchedule.Stub(x => x.IsValid((IClock) a.Arguments[0]));

            a.ReturnValue = fakeSchedule;

        });

Essentially what this is saying:

No matter what IClock value you pass into GetCurrentSchedule, I’m going to return a fake ISchedule that is valid for that IClock.

You could do the opposite of this too, of course. This has been very helpful in reducing some extra set up I might have otherwise endured during testing.

Related:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Chris Missal

Oh hey, I'm a Senior Consultant for Headspring in Austin, TX. I've been working in software professionally since 2006 and I really, really love it. I'm mostly in the Microsoft world, but enjoy building computer things of all sorts (to be vague). When I'm not slinging code, I'm probably out and about slinging discs, bowling balls, or good beer with great friends.
This entry was posted in Best Practices, NUnit, RhinoMocks, Testing. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://blog.robustsoftware.co.uk Garry Shutler

    In this kind of scenario I find it much easier to hand-roll my mock object. Then you can expose a property like “LastSentOrderBody” which lets you split up the assertions. This gives you much finer grained assertions which prevents this sort of problem.

  • Greg Sohl

    I don’t have a problem with multiple Asserts in a test, as long as each produces a proper message. For example, in recently testing a thread-safe cache, after adding an item, there were multiple counters that should be set to particular values. The “Add” test verifies all those counters with multiple Asserts and clear messages on failure.

  • http://melioratingmonkey.blogspot.com/ MM

    The ironic thing is, multiple assertions would have probably allowed you to find the problem faster.

    emailService.AssertWasCalled(x => x.SendOrderReceived(
    Arg.Is.Anything,
    Arg
    .Matches(body => body.Contains(“This order needs a shipping quote.”))));

    emailService.AssertWasCalled(x => x.SendOrderReceived(
    Arg.Is.Anything,
    Arg
    .Matches(body => body.Contains(customerNumber.ToString()))));

    emailService.AssertWasCalled(x => x.SendOrderReceived(
    Arg.Is.Anything,
    Arg
    .Matches(body => body.Contains(orderNumber.ToString()))));

    This might make the test less readable but you would have seen which Contains was failing.

    I think it’s debatable as to whether you’re actually testing three things in this case (regardless of how many actual assertions you have). You’re really just checking the contents of one argument. Three separate tests might be overkill but I’m not sure.

  • http://www.lostechies.com/members/chrismissal/default.aspx Chris Missal

    @Garry

    I think you might be right, a hand-rolled mock might make things a bit more concise here. I use them sometimes, usually when subclassing an object to test it.

  • http://www.lostechies.com/members/chrismissal/default.aspx Chris Missal

    @MM

    Correct, multiple assertions would have pointed to a better failure sooner. I’m just thinking that taking it one step further and breaking them into their own tests would be an even better step. After all, they are individual requirements we want to ensure are working as expected.

  • http://www.lostechies.com/members/chadmyers/default.aspx chadmyers

    Looking through this, it feels like you’ve got an OCP and maybe an SRP issue, which is why you’re feeling the pain of the mock troubles here.

    The “if (order.HasNotes)” code feels like OCP violation as it’s somewhat similar to a switch/case statement block. Perhaps you should consider having strategies with a Match() method and a BuildNotification() method. The strategies get injected via IoC and your Notify() method just iterates the strategies and if they Match() on the current Order, then you call BuildNotification() and the strategy will append whatever it needs to the notification.

  • http://www.lostechies.com/members/chrismissal/default.aspx Chris Missal

    @Chad

    You’re right on the SOLID issues, but I wouldn’t say that they’ve caused pain. I just had the one hiccup that I mentioned above. I could actually modify these pretty easily. The code doesn’t show it above (cause I accidentally omitted it), but the CustomerServiceNotifier is an INotifier, which are injected via our container. It would pretty easy to add the idea of Match() and BuildNotification() methods. We only have a few of these right now, but if/when that area grows, that could be really helpful. Good thoughts!