Mock To Test the Outcome, Not the Implementation

The other day a colleague came to me with a question about a problem they had trying to verify an expectation through a mock – they were trying to mock the invocation of an indexer property in C# with Rhino Mocks. Not knowing Rhino Mocks at all well I had to pass on the specific question, but it did raise other questions and it seems there was a ‘smell’ emanating from the test & code that my colleague had also picked up on…

The Feature

The code they were trying to test was a factory method that was going to be invoked by some 3rd party application framework. The method it seems has more than one responsibility forced upon it by the framework. The code  looked something like this:-

public IView CreateView(IState state, string viewName,
                        string areaName)
{ 
  var view  = IoC.CreateObject<TView>(); 
  var viewModel = IoC.CreateObject<TViewModel>(); 

  viewModel.Load(state); 
  . . . 
  view.ViewModel = viewModel; 
  . . . 
  var area = ViewManager.Areas[areaName]; 
  area.AddView(viewName, view); 
  . . . 
  return view;
}

Now, the code is already doing quite a bit more than I would expect a factory method to do, but that is down to the framework having what I consider excessive demands.

The Test

As I said above I’ve had very little experience with Rhino Mocks, and this post is not about that product which is undoubtedly very clever and very useful. No, it’s about what is being tested with it (the syntax is probably slightly wrong but I hope you get the idea):-

[Test]
public void testCreateView()
{ 
  . . . 
  Expect.Call( viewModel.Load ); 
  Expect.Call( ViewManager.Areas[areaName] ); 
  . . . 
  var view = factory.CreateView(. . .); 

  Assert.That(view, Is.Not.Null); 
  . . .
}

Let’s put aside test naming conventions for now and focus on what’s inside the curly braces. The test is attempting to verify (amongst other things) the following two behaviours:-

• The state is loaded into the ViewModel object
• The view is registered with the view manager

The big problem I have with the test is that it is not verifying that the outcome of the test matches these expectations but that it is verifying exactly how those expectations are being met. In essence the feature specification appears to be saying this:-

• The state is loaded into the ViewModel object by calling the Load() method
• The view is registered with the view manager by associating the view with named area accessed via the indexer property

In short, why should the test prescribe the mechanism used to implement the feature?

Variations On a Theme

I’m going to discuss the “view registration” expectation first because I have another thing to say about the “state loading” that is more design related.

The expectation of the application framework is that the created view will already be registered with the “view manager” by the time the factory method returns. But we may be able to implement that in more than one way depending on what the interface to the view manager looks like. It’s not uncommon to augment a simple interface with different overloads (and extension methods) to allow the caller to write simpler code:-

viewManager.AddView(area, view);

vs

viewManager[area] = view;

You may also choose to introduce your own facade over the abstraction and therefore invoke it indirectly:-

// Does all 3 necessary steps
myViewManagerFacade.Register(area, name, view);

I’ll leave it up to you to decide which you prefer, but my point is that your test should not care which of these many forms are used. In fact it should be possible to switch between them so long as they still adhere to the contract. By not being overly specific in your tests you stand a better chance of not writing brittle tests that break in the face of minor refactorings. One such refactoring that I’ve done in the past is to simplify an interface by introducing extension methods to add syntactic sugar so that the interface remains as simple as possible; should I change the expectation to now be on the extension method or the new interface method? I don’t believe that you can mock extension methods and so now your test code looks inconsistent with your production code. If you introduce a facade, that itself is mockable, do you verify that interface call or the underlying one? From the test’s perspective the outcome is the same and that is what ultimately matters.

So what would my test be? Well, if the real view manager type brings too much baggage to allow me to use it directly in the test I’ll create a mock that will implement the bits I need, i.e. record the results of the interaction:-

public Dictionary<…> m_views = new Dictionary<…>();

public void AddView(string area, IView view)
{ 
  Views.Add(area, view);
}

In mocks I have absolutely no qualms about using public fields because the test is the important thing and the mock is a means to a different end than production code. Then in the test I just assert that the view was registered with the correct name:-

{ 
  var viewManager = new MockViewManager(); 
  var factory = new Factory(viewManager, . . .); 
  . . . 
  var view = factory.CreateView(state, . . .); 

  Assert.That(view, Is.Not.Null);  
  Assert.That(viewManager.m_views[area],
                                 Is.EqualTo(view)); 
  . . .
}

This is a manual mock and no doubt Rhino Mocks makes this kind of thing even easier to write if I just bothered to RTFM. The key point is that I’m only testing the outcome, not the means by which that outcome is achieved. I can change the factory implementation in the ways mentioned earlier and it in no way invalidates the test because from the application framework’s perspective I have met the contract.

Reinventing The Wheel

One thing that worries some people is that I’ve just re-implemented the ViewManager purely to test some other code and have therefore wasted my time re-inventing the wheel and created even more code that needs to be maintained. Well, yes and no. Note that I said I’d only do it if I couldn’t reuse the real ViewManager in my tests. Unit testing is about isolation and determinism – you don’t want a lot of dependencies making your tests brittle. If the real ViewManager cannot easily be plugged in by mocking its dependencies then yes I’d mock it, but if I can reuse it then no I wouldn’t.

When it comes to integration and system testing things get harder and at that point you may have to substitute more fully featured mocks to avoid taking a troublesome dependency on, say, an externally owned service. I may provide many mock implementations of the same service to suit the different types of automated tests being run.

Flexible Designs

So back to my other issue. The reason my colleague added the Load method was in part so that he could verify that the ViewModel object had been initialised from the persistent state. He suggested that in real world use the view model is unusable if this hasn’t happened and I get his point. But I would argue that it is not necessarily the responsibility of the factory to ensure that this happens. Once again we have to put ourselves in the shoes of the client (the application framework) and think how this would be observed – how can we interact with the resulting object in a way that depends on this interaction having being played out?

There are two ways to ensure that this orchestration happens:-

1. Explicitly, by the factory itself or some other mediator in the chain
2. Implicitly, as part of the ViewModel object’s construction

The first approach, which is the one my colleague had chosen, raises other questions such as what happens if someone calls Load a second time later? Or what behaviour can be relied upon between construction and Load being called? Two-phase construction is something I really try and avoid because when somebody does do the unexpected it can manifest as one of those hard to find bugs. And so I favour that latter, after all it’s called a constructor for a reason because it’s expected to construct an object :-).

The obvious follow-on question is “if I delegate responsibility of state loading to the ViewModel constructor then do I still need to test if the object returned from the factory is fully constructed?” Almost definitely “Yes”. There is a reason the method takes arguments and so you are probably expected to use them. If you have overloads of a factory method that do different things, say, one default constructs a view and the other constructs from an external resource you’d want to know that the objects were constructed in the appropriate way.

Now let’s add another common design choice into the mix – lazy loading. Does it matter if the state loading happens directly during construction, asynchronously in the background or even synchronously as some by-product of a method call that depends on the object having been initialised? The test should succeed either way because the object is either fully constructed up front or by the time the assertions attempt to verify the expectations.

Once again let’s compare the two approaches. The original test constrains the implementation into calling a specific method called Load, whereas the latter allows for a number of different implementations. It also allows the code to be refactored between the various implementations without affecting the tests.

Test Driven (Development|Design)

I think this kind of problem highlights the difference between the last word in TDD being interpreted as [D]evelopment or [D]esign. In the former you know what production code you’re going to write and therefore you bias the test towards verifying what you’re going to implement. But in the latter you don’t really know or care how it’s going to be implemented and so the test only verifies that the design allows for it to be done in some way. And the best way to achieve that is to only verify the results of any interactions and not the interactions themselves.