Mocking expensive template methods
Posted by Davy Brion on July 11th, 2008
One approach to implement business logic that i’ve seen a couple of times now, is to use a class that implements an execution pipeline, which triggers virtual methods in a specific order and provides general error handling or transaction management for that pipeline. The idea is usually to get most developers to write their code in a similar manner, while trying to relieve them of the burden of exception handling, transaction management or whatever else you want to provide in the pipeline execution. This approach is far from ideal (personally i think it sucks), but when you’re facing a large application with a bunch of code that already uses this, you pretty much gotta live with it.
Here’s a simplified example of a class implementing such an execution pipeline:
public abstract class Command
{
private readonly List<string> errorMessages = new List<string>();
public void Execute()
{
try
{
CheckAuthorization();
CheckErrorMessages();
ValidateInput();
CheckErrorMessages();
ProcessInput();
CheckErrorMessages();
}
catch (Exception e)
{
Logger.Log(e);
// do something clever that developers supposedly don’t think of
// …
}
}
protected virtual void CheckAuthorization() { }
protected virtual void ValidateInput() { }
protected virtual void ProcessInput() {}
protected void AddErrorMessage(string errorMessage)
{
errorMessages.Add(errorMessage);
}
private void CheckErrorMessages()
{
if (errorMessages.Count > 0)
{
// throw some kind of exception which contains all of the messages
}
}
}
There are many problems with this approach, the most important one being that most developers usually put too much code directly within the virtual methods and usually even using concrete dependencies, which leads to all kinds of testing difficulties. I hope you spotted the outdated exception handling approach, but i’m not even gonna get into that in this post.
Here’s an example of how a small piece of business logic might be implemented with this approach:
public class DeleteCustomerCommand : Command
{
private Customer customer;
private CustomerDataLayerComponent customerDAL;
public void Execute(Customer customer)
{
this.customer = customer;
customerDAL = new CustomerDataLayerComponent();
Execute();
}
protected override void CheckAuthorization()
{
if (!AuthorizationManager.IsAllowedToAccess<DeleteCustomerCommand>())
{
AddErrorMessage(“sorry buddy, no access”);
}
}
protected override void ValidateInput()
{
if (customerDAL.GetOutstandingOrderCount(customer.Id) > 0)
{
AddErrorMessage(“Customer can’t be deleted when there are outstanding orders”);
}
}
protected override void ProcessInput()
{
try
{
customerDAL.Delete(customer);
}
catch (Exception e)
{
AddErrorMessage(e.Message);
}
}
}
If you’ve been reading this blog for a while, then i hope you can spot the 2 biggest problems already. The first is this line:
customerDAL = new CustomerDataLayerComponent();
And the second is this one:
if (!AuthorizationManager.IsAllowedToAccess<DeleteCustomerCommand>())
These are 2 concrete dependencies which can’t easily be replaced with mock implementations while testing this code. Basically, any code that is performed by the data access layer component, or the AuthorizationManager’s static IsAllowedToAccess method will have to be set up properly in every test that you write for this class. Depending on what needs to be done during that set up, this can really become a major pain in the ass. For instance, if you wanted to write a test that would check if the right error message was created when the user is not allowed to access this command, then you have to make sure that the call to IsAllowedToAccess method actually fails. Depending on the implementation of the AuthorizationManager, this can be quite tedious work. Keep in mind that because of the template method pipeline approach that is used here, you need to set this up for every test for this class. The same thing goes for the call to the GetOutstandingOrderCount method of the data access layer component that is being performed in the ValidateInput method.
Suppose you have the following tests for this class:
AddsErrorMessageWhenUserDoesntHaveAccessToThisCommand
DoesNotAddErrorMessageWhenuserDoesntHaveAccessToThisCommand
AddsErrorMessageWhenCustomerHasOutstandingOrders
DoesNotAddErrorMessageWhenCustomerHasNoOutstandingOrders
DeletesCustomerWhenThereAreNoErrorMessages
DoesNotDeleteCustomerWhenThereAreNoErrorMessages
For each test, the entire pipeline is executed. That means that for each test, you need to set up data that is not always relevant to the test itself. You’re actually doing more setting up than you really need to do for that small piece of code you’re trying to test. You can see where this is going right? These 6 tests would basically lead to unecessary expensive setup.
Now suppose you have an application where you have hundreds of these command classes, and thousands of tests. Running all the tests becomes so slow it’s unbearable, not to mention all the wasted effort that has been spent writing all that unnecessary setup, and maintaining it as well because you can be pretty sure that a lot of tests are pretty fragile.
Now, ideally you’d want to make sure that you could talk to a mocked AuthorizationManager and to a mocked data access layer component when this class is being tested. After all, these are dependencies of this class, and thus, the functionality offered by those dependencies should be covered by their own tests. So to correctly test the functionality of this class, you really don’t need to use the actual implementations. In the tests of this command class, you need to verify that it reacts properly to the possible return values of the dependencies.
Unfortunately, refactoring to the approach i just mentioned is not always feasible. So how do you try to minimize the pain? A coworker of mine actually pointed out an approach that allows us to at least minimize the set up that is needed for some tests, without having to redesign the existing code. His suggestion was to mock template methods in the pipeline that were unrelated to the current test. If you can’t redesign the existing code, this is probably the next best thing.
Let’s give it a shot. Suppose we want to test the CheckAuthorization method. We basically don’t need anything to happen in the other methods when we’re testing the authorization, so we’ll try to mock those with empty implementations:
[Test]
public void AddsErrorMessageWhenUserDoesntHaveAccessToThisCommand()
{
var mocks = new MockRepository();
var command = mocks.PartialMock<DeleteCustomerCommand>();
PrepareAuthorizationManagerToDenyAccess();
command.Stub(c => c.ValidateInput()).Do(new Action(EmptyMethodThatDoesntDoAnyting));
command.Stub(c => c.ProcessInput()).Do(new Action(EmptyMethodThatDoesntDoAnyting));
mocks.ReplayAll();
AssertThrows<Exception>(“sorry buddy, no access”, () => command.Execute(new Customer()));
}
Note that in order for this to work, you have to change the protected virtual methods to internal protected (and use the InternalsVisibleTo attribute if your tests are in a different assembly). For this test, we only had to set up the authorization manager, nothing else.
If you want to test the ValidateInput method, you could do something like this:
[Test]
public void AddsErrorMessageWhenCustomerHasOutstandingOrders()
{
var mocks = new MockRepository();
var command = mocks.PartialMock<DeleteCustomerCommand>();
var customer = new Customer();
CreateOutstandingOrderFor(customer);
command.Stub(c => c.CheckAuthorization()).Do(new Action(EmptyMethodThatDoesntDoAnyting));
command.Stub(c => c.ProcessInput()).Do(new Action(EmptyMethodThatDoesntDoAnyting));
mocks.ReplayAll();
AssertThrows<Exception>(“Customer can’t be deleted when there are outstanding orders”,
() => command.Execute(customer));
}
In this test, we only had to set up the order for the customer, but we didn’t have to set up the authorization manager. If you implement your tests this way instead of going through the entire pipeline each time, and thus, taking the performance hit of all the unnecessary set up every time, you can probably cut a lot of time off of those test runs.
Thanks to Joel for giving me the idea 
Note: again, i want to make it clear that i don’t advocate using the template method pipeline approach. If you absolutely want to use it, at least inject each dependency in a way that allows easy mocking. If you can’t modify existing code that already is implemented like this, the partial mocking technique in this post is a good idea, but ideally, this entire thing should be avoided.
Note: i just spotted the EmptyMethodThatDoesntDoAnyting typo… it’s getting late so i’m not even gonna bother fixing it 
July 14th, 2008 at 6:58 pm
What is a preferable pattern for implementing the skeleton of an execution pipeline? (I’ve been using template methods without realizing they were bad
)
July 14th, 2008 at 7:05 pm
the question you need to ask is: why would i need an execution pipeline?
I really don’t see the point in them… code is often simpler without them, and if you want to offer standardized logging or exception handling or transaction management or whatever within that pipeline, you can usually just put that behavior in an aspect and apply it to a simple method which only contains the code that normally would’ve been in the pipeline.
Btw, i don’t think template methods in general are bad… i do use them when they come in handy, but i only use it when i want to delegate a small piece of behavior to a derived class. Never to ‘force’ a specific execution pipeline. When i need to implement too many template methods, i always feel there’s gotta to be a better way and that the necessity of the template methods might indicate that i’m working with a wrong level of abstraction.