Injecting behaviour for static method calls (filesystem mocking redux)

This is a direct evolution of my last post, which dealt with decisions faced in testing code that hits the filesystem. The solution I posted was a bit clunky, and inconsistent; this is better, and works generally if you’re faced with a static method whose behaviour you want to change but can’t, immediately.

A lot of the following process has been informed by Michael Feathers’ “Working Effectively With Legacy Code”. Go read it if you haven’t.

So, imagine you have this idealized legacy code:

using System.IO;
using System.Xml.Linq;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml);
        }

        public Trade(XDocument xml) { /* ... */ }
    }
}

I want to get it under test, changing as little of the existing code as possible in the process.

The first thing to look at is the use of Path, and then File, as they can both be treated the same way. Path and File are static, and I don’t have the code; this means I can't just derive from them. What I want to do is create a class that provides a pass-through to the existing IO code, without modifying any parameters or behaviour in flight by default.

So, I mimic the existing Path class in a new namespace. In the new class I provide delegates for desired behaviour that defaults to the original System.IO behaviour. First I write a few simple tests like:

    [TestFixture]
    public class Tests
    {
        [Test]
        public void PathTest()
        {
            var path = @"root\file";

            var actual = IO.Abstractions.Path.Combine("root", "file");

            Assert.AreEqual(path, actual);
        }
    }

and then the new classes:

using System;
using System.IO;

namespace IO.Abstractions
{
    public static class Path
    {
        public static Func<string, string, string> CombineImpl = Path.Combine;

        public static string Combine(string first, string second)
        {
            return CombineImpl(first, second);
        }
    }

    public static class File
    {
        public static Func<string, bool> ExistsImpl = File.Exists;

        public static bool Exists(string file)
        {
            return ExistsImpl(file);
        }
    }
}

Now I can introduce these new classes along this seam by just swapping the namespace:

//using System.IO;  // no longer needed
using System.Xml.Linq;
using IO.Abstractions;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml);
        }

        public Trade(XDocument xml) { /* ... */ }
    }
}

The Trade-related changes are slightly more complicated – because we want to replace XDocument. XDocument is not a static class, and we do actually use instances of it.

If I want to avoid all changes to the existing code then I’ll need to turn my as-yet unwritten IO.Abstractions.XDocument into a proxy for System.Xml.Linq.XDocument. Unfortunately System.Xml.Linq.XDocument doesn’t implement an interface directly; fortunately it isn’t sealed, so I can derive from it. Even better, System.Xml.Linq.XDocument also provides a copy constructor so I don’t need to wrap an instance and present the correct methods myself. I just need to derive from System.Xml.Linq.XDocument, and construct using the copy constructor.

namespace Xml.Abstractions
{
    public class XDocument : System.Xml.Linq.XDocument
    {
        public XDocument(System.Xml.Linq.XDocument baseDocument) : base(baseDocument)
        {
        }

        public static Func<string, XDocument> LoadImpl = 
            f => new XDocument(System.Xml.Linq.XDocument.Load(f));

        public new static XDocument Load(string file)
        {
            return LoadImpl(file);
        }
    }
}

I’m a little concerned with the amount of work going on in construction here; I’ll have to take a look at how the copy constructor behaves.

Finally I can introduce the new XDocument along the seam in TradeFileLoader, so we end up with exactly what we started with, except for the changed namespace directives:

//using System.IO;
//using System.Xml;
using IO.Abstractions;
using Xml.Abstractions;

namespace AbstractionExamples
{
    public static class TradeFileLoader
    {
        public static ITrade Load(string rootFolder, string file)
        {
            ITrade trade = null;
            var filePath = Path.Combine(rootFolder, file + ".trade");
            if (File.Exists(filePath))
            {
                trade = Trade.Load(filePath);
            }
            return trade;
        }
    }

    public class Trade : ITrade
    {
        public static ITrade Load(string file)
        {
            var xml = XDocument.Load(file);
            return new Trade(xml); 
        }

        public Trade(XDocument xml) { /* ... */ } 
    }
}

So, the process I've gone through when faced with these is:

1. Trace static calls until you run out of code you own (e.g. to Path.Combine, XDocument.Load).
2. Create an abstracted class in a new namespace that mimics the existing static interface.
3. Provide the new class with delegated behaviour that defaults to passing through to the original.
4. (And naturally you'll have tests for this new code to verify that).
5. If the class you’re overriding is not static, derive from the original class and hope that there’s a copy constructor, or be ready for potentially lots more work.
6. Swap in new behaviour on the seam by just replacing old namespace directives (using System.IO, &c) with the new.

Finally, if I want to test the new class I just need to inject my desired test behaviour into Path, File and XDocument -- after adding a method to ITrade and Trade to return the root element of the trade XML. It turns out that the arrange, act, assert flow looks quite natural:

namespace AbstractionExamples.Tests
{
    [TestFixture]
    public class Tests
    {
        [Test]
        public void ReturnsValidTrade_WhenGivenAValidFile()
        {
            File.ExistsImpl = 
                f => { if (f.Equals(@"root\file.trade")) return true; return false; };
            XDocument.LoadImpl = 
                f =>
		{
                    var doc = new XDocument(
                        System.Xml.Linq.XDocument.Parse("<root></root>"));
                    return doc;
                };

            var trade = TradeFileLoader.Load("root", "file");

            Assert.AreEqual("root", trade.Root.LocalName);
    }
}

Rhino.Mocks and manual fakes

Habitually, I use Rhino Mocks. Chris Oldwood and I were discussing fakes and mocks, and he commented that he strongly disliked the Rhino syntax. He much preferred writing manual stubs with delegates for test-specific behaviour because the syntax was clearer and more portable.

He may have a point.

To compare, I knocked up an example test; it just checks the number of times that a method is called on some mocked component:

[Test]        
public void UsingRhinoMocks()        
{
    var mockComponent = MockRepository.GenerateMock<component>();
    
    int expect = 10;            
    mockComponent.Expect(x => x.Operation(null))
        .IgnoreArguments()                
        .Return(new Result())
        .Repeat.Times(expect);                        

    var svc = new Service(mockComponent);            
    svc.ServiceMethod(expect);             

    mockComponent.VerifyAllExpectations();        
}

It’s brief, but somewhat hard to parse. In contrast, here’s the same test as Chris would have it:

[Test]        
public void UsingAManualFake()        
{            
    var fakeComponent = new FakeComponent();             

    int expect = 10;            
    bool called = false;            
    int calls = 0;            
    fakeComponent.Operation = () =>              
        {                  
            called = true;                  
            calls++;                  
            return new Result();              
        };             
    
    var svc = new Service(fakeComponent);            
    svc.ServiceMethod(expect);             

    Assert.IsTrue(called);            
    Assert.AreEqual(expect, calls);        
}

The points here are that the specific tests are clear – you don’t need to reparse the expectations by eye to see what the result should be; and that the setting of expectations is clearer – it looks like a coherent body of code, rather than the notionally fluent style used by Rhino.Mocks. Clearly I’ve left out the definition of the FakeComponent class, among other things – and there lies the principle problem. Fakes can become brittle, especially when shared between different test sets. You have to maintain that class as well as any callsites in the tests as the interface changes.

I took the point about clarity and thought: most of the FakeComponent is actually boilerplate on top of the interface. It’d be easy to dynamically instantiate something using our own dynamic proxy.

But then, Rhino.Mocks is basically creating a dynamic proxy for us.

It turns out we can use Rhino.Mocks to do the same thing, with a helper method to instantiate the mock instance:

private static T GetMock<T,A>(Action<T> method, A del) where T : class        
{   
    // yuck ... but we don't have generic delegate constraints!         
    var action = del as Delegate;              
    
    var mockComponent = MockRepository.GenerateStub<T>();            
    mockComponent.Stub(method).IgnoreArguments().Do(action);            

    return mockComponent;        
}

and then the modified test looks like this, with specific asserts at the end, and a coherent non-fluent code body:

         
[Test]        
public void UsingRhinoMocksLikeAFake()        
{            
    int expect = 10;            
    bool called = false;            
    int calls = 0;            
    var method = new Action<IComponent>(x => x.Operation(null));
    var action = new Func<string, Result>(
        (x) => {
            called = true;
            calls++;
            return new Result();
         });             

    var mockComponent = GetMock(method, action);

    var svc = new Service(mockComponent);            
    var ret = svc.ServiceMethod(expect);             

    Assert.IsTrue(called);            
    Assert.AreEqual(expect, calls);        
}

One wrinkle is that now we need to explicitly define the method that will be called on IComponent, and also the behaviour we want to see when the method is called; it’s like a union of the Rhino and Fake approaches. It’s not quite as clear – but it’s only a line longer.

Mocks and dependencies

On rationalizing class dependencies -- John Sonmez is exploring a number of these ideas at the moment, and is building a useful list of patterns for dealing with common scenarios.

First, check your dependencies; avoid breaking encapsulation

Recently I was bogged down attempting to mock out half the world when testing a method, only to find that a little thought would have made it irrelevant.

You don’t have to mock to isolate – sometimes you just need to understand your dependencies better.

This example is a case in point. I went down a couple of blind alleys because I didn’t consider the coupling between the method under test and other classes. It’s this one:

public void MethodUnderTest()
{
    UtilityClassProvider.Get<UtilityClass>().SomeUsefulMethod();
}

In this abstracted example the problem’s pretty easy to spot. I started mocking a UtilityClassProvider and a UtilityClass, which was all rather more involved than necessary because of the way UtilityClassProvider instantiates a class, and because UtilityClass was sealed, and couldn’t be unsealed. Rhino Mocks complains if you attempt to mock a sealed class, and I couldn’t extract an interface and use that as UtilityClass needs an instantiatable type to create – and so on.

But then -- UtilityClassProvider looks like it’s meant to be a builder. Surely it could be refactored into one, standardising the code a bit too. Refactoring UtilityClassProvider to a builder pattern would be worthwhile, even if it did mean the refactoring started to affect more than the class under test.

Probably best to limit the refactoring to begin with.

I looked at it and realised that this method didn’t depend on UtilityClassProvider at all. At worst, it relies on a UtilityClass, and at best only on the effects of SomeUsefulMethod. The encapsulation-breaking of UtilityClassProvider.Get<UtilityClass>.SomeUsefulMethod() is simply ugly, and let me lead myself up the garden path.

So, in this case I extracted an IUtilityClass property with public accessors, and extracted an IUtilityClass interface from UtilityClass. This shifts the emphasis for creating (and injecting) the IUtilityClass instance to the calling code.

public sealed class UtilityClass : IUtilityClass
{
    public void SomeUsefulMethod()
    {
    }
}

public interface IUtilityClass
{
    void SomeUsefulMethod();
}

public class ClassUnderTest
{
    public IUtilityClass UtilityClass { get; set; }

    public void MethodUnderTest()
    {
        UtilityClass.SomeUsefulMethod();
    }
}

I could have gone a step further and removed the dependency on UtilityClass, and just extracted a method that proxied SomeUsefulMethod instead – but it would have required a call to an IUtilityClass anyway, and going this far removes the more awkward problems associated with the dependency on UtilityClassProvider.

With either solution I’ve removed direct dependencies from the method under test, meaning that the method is a) simpler and b) easier to isolate. The extracted method(s) represent logic that might be better provided by another object or structure – but that’s another step. I might recover a UtilityClassProvider from a container like Unity, for example.

Finally, now I can test the method either with a mock IUtilityClass or an actual UtilityClass, if the class is amenable to it (in this simple example it is; in real life it wasn’t, so mocking helped). I can then inject the mocked IUtilityClass, call MethodUnderTest and voila.

[TestFixture]
public class Tests
{
    [Test]
    public void Test()
    {
        var mockUtilityClass = MockRepository
            .GenerateMock<IUtilityClass>();
        mockUtilityClass
            .Expect(o => o.SomeUsefulMethod());
        
        var cut = new ClassUnderTest();
        cut.UtilityClass = mockUtilityClass;

        cut.MethodUnderTest();

        mockUtilityClass.VerifyAllExpectations();
    }
}

So, the moral is: if you find yourself refactoring half the world just so you can then mock it – check your dependencies first, as you might not be dependent on half of what you think you are.

Mocking dependencies

Given the following UtilityClassProvider and UtilityClass classes, what are suitable ways of mocking out the call to SomeUsefulMethod() and isolating the MethodUnderTest()? Wrinkle: UtilityClass needs to stay sealed.

public class UtilityClassProvider
{
    public T Get<T>() where T : new()
    {
        return new T();
    }
}

public sealed class UtilityClass
{
    public void SomeUsefulMethod()
    {
    }
}

public class ClassUnderTest
{
    public UtilityClassProvider UtilityClassProvider { get; set; }

    public void MethodUnderTest()
    {
        UtilityClassProvider.Get<UtilityClass>().SomeUsefulMethod();
    }
}

[TestFixture]
public class Tests
{
    // Fails; Rhino can't mock a sealed class
    [Test]
    public void Test()
    {
        var mockUtilityClass = MockRepository
            .GenerateMock<UtilityClass>();
        mockUtilityClass
            .Expect(o => o.SomeUsefulMethod());
        var mockUtilityClassProvider = MockRepository
            .GenerateMock<UtilityClassProvider>();
        mockUtilityClassProvider
            .Expect(o => o.Get<UtilityClass>())
            .Return(mockUtilityClass);

        var classUnderTest = new ClassUnderTest() { 
            UtilityClassProvider = mockUtilityClassProvider };
        classUnderTest.MethodUnderTest();

        mockUtilityClassProvider.VerifyAllExpectations();
        mockUtilityClass.VerifyAllExpectations();
    }
}

Unsealing UtilityClass would make mocking easier – but what are the alternatives?

10 minute workout: Iterator pattern

10 minute workout: Visitor pattern