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:
- Trace static calls until you run out of code you own (e.g. to Path.Combine, XDocument.Load).
- Create an abstracted class in a new namespace that mimics the existing static interface.
- Provide the new class with delegated behaviour that defaults to passing through to the original.
- (And naturally you'll have tests for this new code to verify that).
- 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.
- 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);
}
}
