Are you Mocking Me?


Jimmy has had a couple posts (mocks, mocks, and less mocks) that prompted Derick to post this on his experience with tests – I’d like to add my thoughts to mix.  First, let me say I’m not offering an answer to Derick’s question (sorry Derick), simply some ideas to consider.  If they help you, great.  If not, please join the mix.

Not All Tests are created equal

If I told you there are only X types of programs you can write to solve any problem and they could only be created a certain way, etc. you’d probably tell me to go do something rather unflattering and walk away.  Yet when someone talks rigidly about Unit Tests and Integration Tests as solemnly as one might discuss religious rites our incredulity is significantly diminished.  Is that a good thing?  What are automated tests after all?  Aren’t they applications we create to achieve a specific goal?  Is that goal always the same?  I hosted a discussion on testing a few months ago at work.  I began the session with a simple question, “Why do we test?”  The answers from that group were fairly broad – from the expected “to validate the expectations were met” to “to provide immediate feedback on an action we took”.  The next question was “What kind of tests can we do?”  And again the responses were varied.  I believe this is very healthy.

My in my current position I find I have the following goals for tests:

  • When designing the roles in a new area of the application
    • Provide immediate feel for how my design choice shapes the behavior of the application
    • Provide immediate feedback that I am meeting the expectations
    • Keep my design focused on meeting the expectations for the Feature and no more
  • When creating a new feature using existing roles
    • Provide immediate feedback that I am meeting the expectations
    • Warn me if I’ve introduced a new role, altered an existing role, or broken existing features
  • When deploying my application to a CI or other environment
    • Provide assurance the environment is configured correctly
    • Provide immediate feedback about my interaction with external dependencies
    • Provide immediate feedback about how well my system meets the expected acceptable behavior

There are a couple ideas I believe are valuable here:

  • Context – My goals and expectations vary by context; the context of what I’m doing and even the context of where I’m doing it (where I’m employed)
  • Roles – The behavior of a system is ultimately a collection of the data (nouns) and the roles (verbs) of a given feature.  If those roles are not being changed, I’m not designing anything new.  If they do, then I am in a design context
  • Scope – The scope of each context is important and should influence the scale of what I’m trying to do.  Being very specific in a broadly scoped context is an invitation for pain

Because I have different contexts and goals I would be foolish to use the same tool for all of them, but isn’t that often the trap we get into with testing?  We say we follow a TDD, BDD, CYA, etc. style of testing and leave the conversation (and our minds) locked in a monolithic hell where nothing works quite right.  What if we left the grand ideas behind and just stated simply what we want.  When I’m designing something new I want tests to help me explore the roles I’m creating and how well they work together to create the expected behavior.  When I’m simply creating a new feature by composing the existing roles I just need to know I’m creating the appropriate behavior and staying within my expected architecture.  When I deploy, I want to see how the system behaves in it’s environment and how the features flow.  So, what does that look like?  Let’s take the code Jimmy posted in his original article and see.

Design Mode

public class OrderProcessor
{
    private readonly ISmtpClient _client;
    private readonly IOrderSpec _spec;

    public OrderProcessor(ISmtpClient client, IOrderSpec spec)
    {
        _client = client;
        _spec = spec;
    }

    public void PlaceOrder(Order order)
    {
        if (_spec.IsMatch(order))
        {
            string body = “Huge frickin’ order alert!!!rnOrder #:” + order.OrderNumber;

            MailMessage message =
                new MailMessage(“sender@email.com”, “salesdude@email.com”,
                            “Order processed”, body);

            _client.Send(message);
        }

        order.Status = OrderStatus.Submitted;
    }
}
When in design mode I want to start with a test that is focused on the expected behavior of the feature.  Jimmy didn’t supply that expectation so I’m going to make one up.  This feature is part of our commerce MVC application.  The expected behavior is that when a large order is placed, sales will be notified of the order and the order will be placed.  A controller action will take an order object and call our Order Processor object, save the order object, and decide which view to return based on order’s status.  For our purpose we’ll hold a notion of the system beginning at the controller.  Therefore, the behavior I want to create should be exposed solely by the OrderProcessor (let’s assume persistence is cross-cutting concern and not unique or valuable to this test).  Since it is the top level of our system I’d start by creating a feature test that instantiates the controller with a fake persistence object that allows me to control the order object that is passed into the action method and read the ‘saved’ order out.  I’ll let the IoC build the rest from the real objects.  If I was doing this with NUnit it might* look like this:

using Autofac;
using Moq;
using NUnit.Framework;
using Should;

namespace FakeOrderSystem
{
    [TestFixture]
    public class BehaviorProcessOrder
    {
        protected OrderController SUT;
        protected Mock MockSmtpClient;         protected IRepository FakeRepository;

        [TestFixtureSetUp]
        public void CreateSystem()
        {
            var builder = new ContainerBuilder();
            builder.RegisterType().As();             builder.RegisterType().As();             FakeRepository = new AFakeRepository();             MockSmtpClient = new Mock(MockBehavior.Loose);             builder.Register(s => MockSmtpClient.Object).As().SingleInstance();             var container = builder.Build();             SUT = new OrderController(FakeRepository, container.Resolve());         }

        [Test]
        public void Should_Notify_Sales_When_a_Large_Order_is_Placed()
        {
            //Arrange
            var largeOrder = new Order {OrderNumber = “O1234485”, Total = 300, Status = OrderStatus.New};

            //Act
            SUT.PlaceOrder(largeOrder);

            //Assert
            MockSmtpClient.Verify(c => c.Send(It.IsAny()));             FakeRepository.GetById(largeOrder.OrderNumber).Status.ShouldEqual(OrderStatus.Submitted);         }             } }

*confession here – I didn’t spend the time to get this passing.  Do not consider this a ‘real’ example.

I few things to point out here:

  1. The only test doubles here are for the SmtpClient (a mock because I only need to know it was called) and the Respository (a fake because I want more control)
  2. I could’ve used scanning to avoid specifying the interfaces (roles) involved and the objects acting in those roles.  I like being specific when I’m in design mode.  Once I’m done, I can always go back and replace this code with scanning/autowiring
  3. I want the system to remain the same for each test – the inputs and results are what define the context and expectations

What does this give me?  For starters, it helps to clarify what I consider my system, what my expectations are of that system’s behavior in a given context, and what roles I believe should be enacted by the system to fulfill those expectations.  Once the design is complete (e.g. all expectations are met), I can re-use this test to confirm the system is unharmed by future additions to the application.  Indeed, if I remove the explicit registration, I could potentially replace the implementations below the controller and re-use this test.  It also helps me see the results of my design choices.  A few thoughts there – IOrderSpec is a bit too vague for my taste.  I’d want to explore that bit of the system more.  What is it’s role in processing an order and how can that role be more clearly expressed?  Second, I really don’t like where the ISmtpClient sits.  Even without seeing the implementation the order processor, I can spot a rub.  Did you see it too?  My expectation is that sales is notified, a behavior that is never expressed by an interface (role) in my system.  I believe I’m missing an abstraction – INotify.  Doesn’t change the need to use the mock for the mail client, but it does help to clarify how the expectation is met while providing more flexibility in the architecture for how that expectation is implemented (and maybe expanded in the future).  As I implemented each role, I would create unit tests for that single class using test doubles for all dependencies.  I would probably use an Automocking container (StructureMap has one, as does Autofac via the contrib project).  I would do this to allow me to be specific about the interaction (how the roles behave) as I design the system.  I might delete these tests after the design is finished, leave them alone as a warning for when my design has changed, or keep them in a separate test project I use when I’m re-designing my system.

After re-factoring our system design test looks like this:

using Autofac;
using NUnit.Framework;
using Should;

namespace FakeOrderSystem
{
    [TestFixture]
    public class BehaviorProcessOrder
    {
        protected OrderController SUT;
        protected AFakeMailClient FakeSmtpClient;  <——-  Now a Fake
        protected IRepository FakeRepository;

        [TestFixtureSetUp]
        public void CreateSystem()
        {
            var builder = new ContainerBuilder();
            builder.RegisterType().As();             builder.RegisterType().As();   <—- New Role that was ‘found’ by exploring the IOrderSpec role             builder.RegisterType().As(); <— The re-factored IOrderSpec             builder.RegisterType().As(); <—- Also ‘had’ a requirement for this state so put it in here to see how the design behaves
            builder.RegisterType().As(); <— we now have an abstraction that fulfills our missing ‘notify’ role             FakeRepository = new AFakeRepository();             FakeSmtpClient = new AFakeMailClient();             builder.Register(s => FakeSmtpClient).As().SingleInstance();             builder.Register(r => FakeRepository).As<IRepository>().SingleInstance();             var container = builder.Build();             SUT = new OrderController(container.Resolve());         }

        [Test]
        public void Should_Notify_Sales_When_a_Large_Order_is_Placed()
        {
            //Arrange
            var largeOrder = new Order {OrderNumber = “O1234485”, Total = 300, Status = OrderStatus.New};
           
            //Act
            SUT.PlaceOrder(largeOrder);

            //Assert
            FakeSmtpClient.VerifyMessageSentTo(“sales@fakeemail.com”).ShouldBeTrue();
            FakeRepository.GetById(largeOrder.OrderNumber).Status.ShouldEqual(OrderStatus.Submitted);
        }

        [Test]
        public void Should_Notify_Manufacturing_When_a_Rush_Order_is_Placed()
        {
            //Arrange
            var rushOrder = new Order {OrderNumber = “O1234486”, Total = 30, Status = OrderStatus.NewRush};
           
            //Act
            SUT.PlaceOrder(rushOrder);

            //Assert
            FakeSmtpClient.VerifyMessageSentTo(“manufacturing@fakeemail.com”).ShouldBeTrue();
            FakeRepository.GetById(rushOrder.OrderNumber).Status.ShouldEqual(OrderStatus.Submitted);
        }
    }

A consequence of this exercise is that I’ve decided (and expressed in the test) that my system has three top level responsibilities (roles): 1) to evaluate which actions should be taken based on the order’s state, 2) to set the status to submitted, and 3) to save the order (which will prompt other parts of the application to do things).  I refactored the OrderProcessor to perform these roles and added other dependencies (IEvaluateOrdersForActions) to help clarify the roles of the system.  I’ve also decided that the truly meaningful part of the email client is that the message was sent to the right person (I could also examine other aspects of the message).  This has removed my use of mocks completely as I need to retrieve the message from the client.  I’m now using two simple fake objects for more external dependencies.  How about the class that started this whole thing?  Our old friend OrderProcessor looks like this:

public class OrderProcessor : IProcessAnOrder
{
    private readonly IEvaluateOrdersForActions _evaluator;
    private readonly IRepository _repository;

    public OrderProcessor(IEvaluateOrdersForActions evaluator, IRepository repository)     {         _evaluator = evaluator;         _repository = repository;     }

    public void PlaceOrder(Order order)
    {
        _evaluator.Evaluate(order);
        order.Status = OrderStatus.Submitted;
        _repository.Update(order);
    }
}

Revisiting Jimmy’s statement on the value of test doubles I’d say this:  Mocks, stubs, fakes and the like are most valuable when they replace external dependencies.  Mocks, stubs, and the like are most valuable when they play a role in achieving the purpose for which you wrote the test.  I’m going to stop there for now.  I hope this gives folks some ideas to chew on.  A reminder, I’m not saying this is The Right Way or even A Right Way.  These are some ideas.  They need to worked and challenged if they are to have any value for you (or me for that matter).  I know I didn’t provide examples for deployment testing.  If I can find some time (and motivation) I’ll try to post some ideas on those as well.  Discuss.  (ala CoffeeTalk)

Pablo’s Fiesta Preparation