My apologies. I meant to be a little more regular in this series, but I stumbled a bit out of the gate, as I got into the home stretch of my next Pluralsight course. Now that the course is delivered (not released yet – in the review/edit phase), I have some more time, so I’m planning to pick this back up and go with it a little more regularly.
One interesting thing that arises out of these “fits and starts” kind of passes at it is that it mimics an actual, common development scenario: spotty maintenance coding. What I mean is, so many TDD series that you’ll watch or coding dojo/exercises in which you’ll participate have a premise that you have some fixed length of time during which to pay complete attention. But in this series, I’m sort of poking at it for 10 minutes here and 20 minutes there, very seriously mimicking an environment where you’re plugging a lot of holes, thrashing a bit and saying, “where was I and what was I dong here?”
That’s evident in this clip, probably a little too much for me really to call it polished. And as such, I don’t accomplish a ton, but here’s what I did accomplish (not necessarily in order):
Tied up a loose end by getting rid of the last of the primitive obsession passing of x and y coordinate ints.
Implemented sanity precondition checks for the input Board’s AddPiece() method, in terms of where pieces could be placed.
Pushed functionality for validating coordinates into the coordinate itself.
Eliminated duplication in the validation with a refactoring.
And, here are some lessons to take away from this, both instructional from me and by watching me make mistakes:
After a conceptual refactoring, such as replacing multiple primitives with a type, take a look around to make sure you cleaned up all instances of the former.
When you’re not really sure what to do next (i.e. “coder’s block” or “paralysis by analysis”), implement some sanity checks for preconditions/invariants. This might jolt you into some next steps as you do it.
Make ABSOLUTELY SURE that a test goes red when you think it should go red. Not understanding why a unit test is passing is just as bad as not understanding why it’s failing. In both cases, it means you don’t understand what your code is doing. Stop everything and get your brain in sync with the code immediately to save yourself a lot of frustration later. (See “programming by coincidence,” which I saw coined in the book “The Pragmatic Programmer” — and then, don’t do it!)
You’re going to make mistakes. Often dumb ones. The beauty of TDD and its fast feedback loop is to prevent them from festering and being worse later.
This is more of an editorial/opinion take, but I’ve more recently gravitated toward allowing my TDD to include what might be called “integration tests” (tests that exercise the interaction between two classes). As long as the test makes sense from a behavioral standpoint and provides clarity, I think it’s fine. Some, particularly those in the BDD camp, even argue that this is preferred, and that your tests should really only go through the outer API of your module/application.
Eliminate duplication, however trivial and however subtle. If you see repetition of any kind, you can probably extract a method. Some productivity tools and IDEs will even help you locate possible duplication.
Finally, a few notes on the video itself (and resultant code):
For those of you who suggested a larger font size, look for that in part 4. I apologize, but I had actually recorded the video for this already when I was taking suggestions. In the production, I did zoom to a slightly smaller area, so we’ll see if that helps any.
I had one commenter express a preference for a white background instead of the VS Dark theme that I use. White work-spaces give me a headache, so I darken all IDEs and things that I work in. For one person, I don’t think I’ll pull the trigger, but if more people start responding and expressing that preference, I’ll agree to suck it up and change colors.
The code is now on github. I’ll commit the code each time I record the video and tag it with a comment corresponding to the part of the series in question. The initial push to master just reads “Initial publish to Github” but it corresponds to the code at the end of this clip. From here forward, I’ll sync them, though if you check the repo, it’ll probably run slightly ahead of me publishing the videos because I record the audio and do these writeups after the fact.
Again, the higher res you view this in the better. I’d go for 1440P if you can.
In the last two posts in this series, I talked about how to test new code in your code base and then how to bring your legacy code under test. Toward the end of the last chapter in this series, I talked a bit about the concept of test doubles. The example I showed was one in which I used polymorphism to create a “dummy” class that I used in a test to circumvent otherwise untestable code. Here, I’ll dive into a lot more detail on the subject, starting out with a much simpler example than that and building to a more sophisticated way to handle the management of your test doubles.
First, a Bit of Theory
Before we get into test doubles, however, let’s stop and talk about what we’re actually doing, including theory about unit tests. So far, I’ve showed a lot of examples of unit tests and talked about what they look like and how they work (for instance, here in post two where I talk about Arrange, Act Assert). But what I haven’t addressed, specifically, is how the test code should interact with the production code. So let’s talk about that a bit now.
By far the most common case when unit testing is that you instantiate a class under test in the “arrange” part of your unit test, and then you do whatever additional setup is necessary before calling some method on that class. Then you assert something that should have happened as a result of that method call. Let’s return to the example of prime finder from earlier and look at a simple test:
[TestMethod]
public void Returns_False_For_One()
{
var primeFinder = new PrimeFinder(); //Arrange
bool result = primeFinder.IsPrime(1); //Act
Assert.IsFalse(result); //Assert
}
This should be reviewed from the perspective of “arrange, act, assert,” but let’s look specifically at the “act” line. Here is the real crux of the test; we’re writing tests about the IsPrime method and this is where the action happens. In this line of code, we give the method an input and record its output, so it’s the perfect microcosm for what I’m going to discuss about a class under test: its interactions with other objects. You see, unit testing isn’t about executing your code — you can do that with integration tests, console apps, or even just by running the application. Unit testing, at its core, is about isolating your classes and running experiments on them, as if you were a scientist in a lab. And this means controlling all of the inputs to your class — stimulus, if you will — so that you can observe what it puts out.
Controlling the inputs in the PrimeFinder class is simple. Because I’m telling you that there are no invocations of global/static state (which will become an important theme as we proceed). You can see by looking at the unit test that the only input to the class under test (CUT) is the integer 1. This means that the only input/stimulus that we supply to the class is a simple integer, making it quite easy to make assertions about its behavior. Generally speaking, the simpler the inputs to a class, the easier that class is to test.
There are Inputs and There are Inputs
Omitting certain edge cases I can think of (and probably some that I’m not thinking of), let’s consider a handful of relatively straightforward ways that a class might get ahold of input information. There is what I did above — passing it into a method. Another common way to give information to a class is to use constructor parameters or setter methods/properties. I’ll refer to these as “passive collaboration” from the perspective of the CUT, since it’s simply being given the things that it needs. There is also what I’ll call “semi-passive collaboration,” which is when you pass a dependency to the CUT and the CUT interacts in great detail with that dependency, mutating its state and querying it. An example of this would be “Car theCar = new Car(new Engine())”, in which performing operations on Car related to starting and driving result in rather elaborate modifications to the state of Engine. It’s still passive in the sense that you’re handing the Engine class to the car, but it’s not as passive as simply handing it an integer. In general, passive input is input that the scope instantiating the CUT controls — constructor parameters, method parameters, setters, and even things returned from methods of objects passed to the CUT (such as the Car class calling _engine.GetTemperature() in the example in this paragraph).
In contrast, there is also “active collaboration,” which is when the CUT takes responsibility for getting its own inpu. This is input that you cannot control when instantiating the class. An example of this is a call to some singleton or public static method in the CUT. The only way that you can reassume control is by not calling the method in which it occurs. If static/singleton calls occur in the constructor, you simply cannot test or even instantiate this class without it doing whatever the static code entails. If it retrieves values from static state, you have no control over those values (short of mocking up the application’s global state).
A second form of active collaboration is the “new” operator. This is very similar to static state in that when you create the CUT, you have no control over this kind of input to the CUT. Imagine if Car new-ed up its own Engine and queried it for temperature. There would be absolutely no way that you could have any effect on this operation in the Car class short of not instantiating it. Like static calls, object instantiation renders your CUTs a non-negotiable, “take it or leave it” proposition. You can have them with all of their instantiated objects and global state or you can write your own, buddy.
Not all inputs to a class are created equal. There are a CUT’s passive inputs, in which the CUT cedes control to you. And then there are the CUT’s active inputs that it controls and on which it does not allow you to interpose in any way. As it turns out, it is substantially easier to test CUTs with exclusively passive collaboration/input and difficult or even impossible to test CUTs with active collaboration. This is simply because you cannot isolate actively collaborating CUTs.
Literals: Too Simple to Need Test Doubles
There’s still a little bit of work to do before we discuss test doubles in earnest. First, we have to talk about inputs that are too simple to require stand-ins: literals. The PrimeFinder test above is the perfect example of this. It’s performing a mathematical operation using an integer input, so what we’re interested in testing is known input-output pairs in a functional sense. As such, we just need to know what to pass in, to pass that value in, and then to assert that we get the expected return value.
In a strict sense, we could refer to this as a form of test double. After all, we’re doing a non-production exercise with the API, so the value we’re passing in is fake, in a sense. But that’s a little formal for my taste. It’s easier just to think in terms of literals almost always being too simple to require any sort of substitution of behavior.
An interesting exception to this the null literal (of null type) or the default value of a non-nullable type. In many cases, you may actually want to be testing this as an input since null and 0 tend to be particularly interesting inputs and the source of corner cases. However, in some cases, you may be supplying what is considered the simplest form of test double: the dummy value. A dummy value is something you pass into a function to say, “I don’t care what this is and I’m just passing in something to make the compiler happy.” An example of where you might do this is passing null to a constructor of an object instance when you just want to make assertions as to what some of its property values initialize to.
Simple/Value Objects and Passing in Friendlies
Next up for consideration is the concept of a “test stub,” or what I’ll refer to in the general sense as a “friendly.”
Take a look at this code:
public class Car
{
public int EngineTemperature { get; private set; }
public Car(Engine engine)
{
EngineTemperature = engine.TemperatureInFahrenheit;
}
}
public class Engine
{
public int TemperatureInFahrenheit { get; set; }
}
Here is an incredibly simple implementation of the Car-Engine pair I described earlier. Car is passed an Engine and it queries that Engine for a local value that it exposes. Let’s say that I now want to test that behavior. I want to test that Car’s EngineTemperature property is equal to the Engine’s temperature in fahrenheit. What do you think is a good test to write? Something like this, maybe —
[TestMethod]
public void EngineTemperature_Initializes_Value_Returned_By_Engine()
{
const int engineTemperatureFromEngine = 200;
Engine engine = new Engine() { TemperatureInFahrenheit = engineTemperatureFromEngine };
var car = new Car(engine);
Assert.AreEqual(engineTemperatureFromEngine, car.EngineTemperature);
}
Here, we’re setting up the Engine instance in such a way as that we control what it provides to Car when Car uses it. We know by inspecting the code for Car that Car is going to ask Engine for its TemperatureInFahrenheit value, so we set that value to a known commodity, allowing us to compare in the Assert. To put it another way, we’re supplying input indirectly to Car by setting up Engine and telling Engine what to give to Car. It’s important to note that this is only possible because Car accepts Engine as an argument. If Car instantiated Engine in its constructor, it would not be possible to isolate Car because any test of Car’s initial value would necessarily also be a test of Engine, making the test an integration test rather than a unit test.
Creating Bonafide Mocks
That’s all well and good, but what if the Engine class were more complicated or just written differently? What if the way to get the temperature was to call a method and that method went and talked to a file or a database or something? Think of how badly the testing for this is going to go:
public class Car
{
public int EngineTemperature { get; private set; }
public Car(Engine engine)
{
EngineTemperature = engine.TemperatureInFahrenheit;
}
}
public class Engine
{
public int TemperatureInFahrenheit
{
get
{
var stream = new StreamReader(@"C:\whatever.txt");
return int.Parse(stream.ReadLine());
}
}
}
Now, when we instantiate a Car and query its engine temperature property, suddenly file contents are being read into memory and, as I’ve already covered in this series, File I/O is a definite no-no in a unit test. So I suppose we’re hosed. As soon as Car tries to read Engine’s temperature, we’re going to explode — or we’re going to succeed, which is even worse because now you’ll have a unit test suite that depends on the machine it’s running on having the file C:\whatever.txt on it and containing an integer as its first line.
But what if we got creative the way we did at the end of the last episode of this series? Let’s make the TemperatureInFahrenheit property virtual and then declare the following class:
public class FakeEngine : Engine
{
private int _temperature;
public override int TemperatureInFahrenheit
{
get { return _temperature; }
}
public FakeEngine(int temperature)
{
_temperature = temperature;
}
This class is test-friendly because it doesn’t contain any file I/O at all and it inherits from Engine, overriding the offending methods. Now we can write the following unit test:
[TestMethod]
public void EngineTemperature_Initializes_Value_Returned_By_Engine()
{
const int engineTemperatureFromEngine = 200;
Engine engine = new FakeEngine(engineTemperatureFromEngine);
var car = new Car(engine);
Assert.AreEqual(engineTemperatureFromEngine, car.EngineTemperature);
}
If this seems a little weird to you, remember that our goal here is to test the Car class and not the engine class. All that the Car class knows about Engine is that it wants its TemperatureInFahrenheit property. It doesn’t (and shouldn’t) care how or where this comes from internally to Engine — file I/O, constructor parameter, secret ink, whatever. And when testing the Car class, you certainly don’t care. Another way to think of this is that you’re saying, “assuming that Engine tells Car that the engine temperature is 200, we want to assert that Car’s EngineTemperature property is 200.” In this fashion, we have isolated the Car class and are testing only its functionality.
This kind of test double and testing technique is known as a Fake. We’re creating a fake engine to stand-in for the real one. It’s not simple enough to be a dummy or a stub, since it’s a real, bona-fide different class instead of a doctored version of an existing one. I realize that the terminology for the different kinds of test doubles can be a little confusing, so here’s a helpful taxonomy of them.
Mocking Frameworks
The last step in the world of test doubles is to get to actual mock objects. If you stop and ponder the fake approach from the last section a bit, a problem might occur to you. The problem has to do with long-term maintenance of code. I remember, many moons ago when I discovered the power of polymorphism for creating fake objects, that I thought it was the greatest thing under the sun. Obviously there was at least one fake per test class with dependency, and sometimes there were multiple dependencies. And I didn’t always stop there — I might define three or four different variants of the fake, each having a method that behaved differently for the test in question. In one fake, TemperatureInFarenheit would return a passed in value, but in another, it would throw an exception. Oh, there were so many fakes — I was swimming in fakes for classes and fakes for interfaces.
And they were awesome… until I added a method to the interface they implemented or changed behavior in the class they inherited. And then, oh, the pain. I would have to go and change dozens of classes. And then there was also the fact that all of this faking took up a whole lot of space. My test classes were littered with nested classes of fakes. It was fun at first, but the maintenance became a drudgery. But don’t worry, because my gift to you is to spare you that pain.
What if I told you that you could implement interfaces and inherit from classes anonymously, without actually creating source code that did this? I’d be oversimplifying a bit, but once you got past that, you’d probably be pretty excited. I say this because, as you start to grasp the concept of mocking frameworks, this kind of “dynamic interface implementation/inheritance” is the easiest way to reason about what it’s doing, from a practical perspective, without getting bogged down in more complicated concepts like reflection and direct work with byte-code and other bits of black magic.
As an example of this in action, take a look at how I go about testing the Car and Engine with the difficult dependency. The first thing that I do is delete the Fake class because there’s no need for it. The next thing I do is write a unit test, using a framework called JustMock by Telerik (this is currently my preferred mocking framework for C#).
[TestMethod]
public void EngineTemperature_Initializes_Value_Returned_By_Engine()
{
const int engineTemperatureFromEngine = 200;
var engine = Mock.Create();
engine.Arrange(e => e.TemperatureInFahrenheit).Returns(engineTemperatureFromEngine);
var car = new Car(engine);
Assert.AreEqual(engineTemperatureFromEngine, car.EngineTemperature);
}
Notice that instead of instantiating an engine, I now invoke a static method on a class called Mock that takes care of creating my dynamic inheritor for me. Mock.Create() is what creates the equivalent of FakeEngine. On the next line, I invoke an (extension) method called Arrange that creates an implementation of the property for me as well. What I’m saying, in plain English, is “take this mock engine and arrange it such that the TemperatureInFahrenheit property returns 200.” I’ve done all of this in one line of code instead of adding an entire nested class. And, best of all, I don’t need to change this mock if I decide to change some behavior in the base class or add a new method.
Truly, once you get used to the concept of mocking, you’ll never go back. It will become your best friend for the purposes of mocking out dependencies of any real complexity. But temper your enthusiasm just a bit. It isn’t a good idea to use mocking frameworks for simple dependencies like the PrimeFinder example. The lite version of JustMock that I’ve used and many others won’t even allow it, and even if they did, that’s way too much ceremony — just pass in real objects and literals, if you can reasonably.
The idea of injecting dependencies into classes (what I’ve called “passive” and “semi-passive” collaboration) is critical to mocking and unit testing. All basic mocking frameworks operate on the premise that you’re using this style of collaboration and that your classes are candidates for polymorphism (either interfaces or overridable classes). You can’t mock things like primitives and you can’t mock sealed/final classes.
There are products out there called isolation frameworks that will grant you the ability to mock pretty much everything — primitives, sealed/final classes, statics/singletons, and even the new operator. These are powerful (and often long-running, resource-intensive) tools that have their place, but that place is, in my opinion, at the edges of your code base. You can use this to mock File.Open() or new SqlConnection() or some GUI component to get the code at the edge of your application under test.
But using it to test your own application logic is a path that’s fraught with danger. It’s sort of like fixing a broken leg with morphine. Passively collaborating CUTs have seams in them that allow easy configuration of behavior changes and a clear delineation of responsibilities. Actively collaborating CUTs lack these things and are thus much more brittle and difficult to separate and modify. The fact that you can come up with a scheme allowing you to test the latter doesn’t eliminate these problems — it just potentially masks them. I will say that isolating your coupled, actively collaborating code and testing it is better than not testing it, but neither one is nearly as good as factoring toward passive collaboration.
If, in the movie Braveheart, the Scots had been battling a nasty legacy code base instead of the English under Edward Longshanks, the conversation after the battle at Stirling between Wallace and minor Scottish noble MacClannough might have gone like this:
Wallace: We have prevented new bugs in the code base by adding new unit tests for all new code, but bugs will still happen.
MacClannough: What will you do?
Wallace: I will invade the legacy code, and defeat the bugs on their own ground.
MacClannough (snorts in disbelief): Invade? That’s impossible.
Wallace: Why? Why is that impossible? You’re so concerned with squabbling over the best process for handling endless defects that you’ve missed your God-given right to something better.
Goofy as the introduction to this chapter of the series may be, there’s a point here: while unit testing brand new classes that you add to the code base is a victory and brings benefit, to reap the real game-changing rewards you have to be a bit of a rabble-rouser. You can’t just leave that festering mass of legacy code as it is, or it will generate defects even without you touching it. Others may scoff or even outright oppose your efforts, but you’ve got to get that legacy code under test at some point or it will dominate your project and give you unending headaches.
So far in this series, I’ve covered the basics of unit testing, when to do it, and when it might be too daunting. Most recently, I talked about how to design new code to make it testable. This time, I’m going to talk about how to wrangle your existing mess to start making it testable.
Easy Does It
A quick word of caution here before going any further: don’t try to do too much all at once. Your first task after reading the rest of this post should be selecting something small in your code base to try it on if you want to target production and get it approved by an architect or lead, if that’s required. Another option is just to create a playpen version of your codebase to throw away and thus earn yourself a bit more latitude, but either way, I’d advise small, manageable stabs before really bearing down. What specifically you try to do is up to you, but I think it’s worth proceeding slowly and steadily. I’m all about incremental improvement in things that I do.
Also, at the end of this post I’ll offer some further reading that I highly recommend. And, in fact, I recommend reading it before or as you get started working your legacy code toward testability. These books will be a great help and will delve much further into the subjects that I’ll cover here.
Test What You Can
Perhaps this goes without saying, but let’s just say it anyway to be thorough. There will be stuff in the legacy code base you can test. You’ll find the odd class with few dependencies or a method dangling off somewhere that, for a refreshing change, doesn’t reference some giant singleton. So your first task there is writing tests for that code.
But there’s a way to do this and a way not to do this. The way to do it is to write what’s known as characterization tests that simply document the behavior of the existing system. The way not to do this is to introduce ‘corrections’ and cleanup as you go. The linked post goes into more detail, but suffice it to say that modifying untested legacy code is like playing Jenga — you never really know ahead of time which brick removal is going to cause an avalanche of problems. That’s why legacy code is so hard to change and so unpleasant to work with. Adding tests is like adding little warnings that say, “dude, not that brick!!!” So while the tower may be faulty and leaning and of shoddy construction, it is standing and you don’t want to go changing things without putting your warning system in place.
So, long story short, don’t modify — just write tests. Even if a method tells you that it adds two integers and what it really does is divide one by the other, just write a passing test for it. Do not ‘fix’ it (that’ll come later when your tests help you understand the system and renaming the method is a more attractive option). Iterate through your code base and do it everywhere you can. If you can instantiate the class to get to the method you want to test and then write asserts about it (bearing in mind the testability problems I’ve covered like GUI, static state, threading, etc), do it. Move on to the next step once you’ve done the easy stuff everywhere. After all, this is easy practice and practice helps.
Go searching for extractable code
Now that you have a pretty good handle on writing testable code as you’re adding it to the code base and getting untested but testable code under test, it’s time to start chipping away at the rest. One of the easiest ways to do this is to hunt down methods in your code base that you can’t test but not because of the contents in them. Here are two examples that come to mind:
public class Untestable1
{
public Untestable1()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
}
public int AddTwoNumbers(int x, int y)
{
return x + y;
}
}
public class Untestable2
{
public void PerformSomeBusinessLogic(CustomerOrder order)
{
Console.WriteLine("Total is " + AddTwoNumbers(order.Subtotal, order.Tax));
}
private int AddTwoNumbers(int x, int y)
{
return x + y;
}
}
The first class is untestable because you can’t instantiate it without kicking off global state modification and who knows what else. But the AddTwoNumbers method is imminently testable if you could remove that roadblock. In the second example, the AddTwoNumbers method is testable once again, in theory, but with a roadblock: it’s not public.
In both cases, we have a simple solution: move the method somewhere else. Let’s put it into a class called “BasicArithmeticPerformer” as shown below. I do realize that there are other solutions to make these methods testable, but we’ll talk about them later. And I’ll tell you what I consider to be a terrible solution to one of the testability issues that I’ll talk about now: making the private method public or rigging up your test runner with gimmicks to allow testing of private methods. You’re creating an observer effect with testing when you do this — altering the way the code would look so that you can test it. Don’t compromise your encapsulation design to make things testable. If you find yourself wanting to test what’s going on in private methods, that’s a strong, strong indicator that you’re trying to test the wrong thing or that you have a design flaw.
public class BasicArithmeticPerformer
{
public int AddTwoNumbers(int x, int y)
{
return x + y;
}
}
Now that’s a testable class. So what do the other classes now look like?
public class Untestable1
{
public Untestable1()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
}
private int AddTwoNumbers(int x, int y)
{
return new BasicArithmeticPerformer().AddTwoNumbers(x, y);
}
}
public class Untestable2
{
public void PerformSomeBusinessLogic(CustomerOrder order)
{
Console.WriteLine("Total is " + AddTwoNumbers(order.Subtotal, order.Tax));
}
public int AddTwoNumbers(int x, int y)
{
return new BasicArithmeticPerformer().AddTwoNumbers(x, y);
}
}
Yep, it’s that simple. In fact, it has to be that simple. Modifying this untestable legacy code is like walking a high-wire without a safety net, so you have to change as little as possible. Extracting a method to another class is very low risk as far as refactorings go since the most likely problem that could possibly occur (particularly if using an automated tool) is non-compiling. There’s always a risk, but getting legacy code under test is lower risk in the long run than allowing it to continue rotting and the risk of this particular approach is minimal.
On the other side of things, is this a significant win? I would say so. Even ignoring the eliminated duplication, you now have gone from 0 test coverage to 50% in these classes. Test coverage is not a goal in and of itself, but you can now rest a little easier knowing that you have a change warning system in place for half of your code. If someone comes along later and says, “oh, I’ll just change that plus to a minus so that I can ‘reuse’ this method for my purposes,” you’ll have something in place that will throw up a bid red X and say, “hey, you’re breaking things!” And besides, Rome wasn’t built in a day — you’re going to be going through your code base building up a test suite one action like this at a time.
Code that refers to no class fields is easy when it comes to extracting functionality to a safe, testable location. But what if there is instance-level state in the mix? For example…
public class Untestable3
{
int _someField;
public Untestable3()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
_someField = TestabilityKiller.Instance.GetSomeGlobalVariableValue();
}
public int AddToGlobal(int x)
{
return x + _someField;
}
}
That’s a little tougher because we can’t just pull _someField into a new, testable class. But what if we made a quick change that got us onto more familiar ground? Such as…
public class Untestable3
{
int _someField;
public Untestable3()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
_someField = TestabilityKiller.Instance.GetSomeGlobalVariableValue();
}
public int AddToGlobal(int x)
{
return AddTwoNumbers(x, _someField);
}
private int AddTwoNumbers(int x, int y)
{
return x + y;
}
}
Aha! This looks familiar, and I think we know how to get a testable method out of this thing now. In general, when you have class fields or local variables, those are going to become arguments to methods and/or constructors of the new, testable class that you’re creating and instantiating. Understand going in that the more local variables and class fields you have to deal with, the more of a testing headache the thing you’re extracting is going to be. As you go, you’ll learn to look for code in legacy classes that refers to comparably few local variables and especially fields in the current class as a refactoring target, but this is an acquired knack.
The reason this is not especially trivial is that we’re nibbling here at an idea in static analysis of object oriented programs called “cohesion.” Cohesion, explained informally, is the idea that units of code that you find together belong together. For example, a Car class with an instance field called Engine and three methods, StartEngine(), StopEngine( )and RestartEngine() is highly cohesive. All of its methods operate on its field. A class called Car that has an Engine field and a Dishwasher field and two methods, StartEngine() and EmptyDiswasher() is not cohesive. When you go sniping for testable code that you can move to other classes, what you’re really looking for is low cohesion additions to existing classes. Perhaps some class has a method that refers to no instance variables, meaning you could really put it anywhere. Or, perhaps you find a class with three methods that refer to a single instance variable that none of the other 40 methods in a class refer to because they all use some other fields on the class. Those three methods and the field they use could definitely go in another class that you could make testable.
When refactoring toward testability, non-cohesive code is the low-hanging fruit that you’re looking for. If it seems strange that poorly designed code (and non-cohesive code is a characteristic of poor design) offers ripe refactoring opportunities, we’re just making lemonade out of lemons. The fact that someone slammed unrelated pieces of code together to create a franken-class just means that you’re going to have that much easier of a time pulling them apart where they belong.
Realize that Giant Methods are Begging to be Classes
It’s getting less and less common these days, but do you ever see object-oriented code which you can tell that the author meandered his way over to from writing C back in the one-pass compiler days? If you don’t know what I mean, it’s code that has this sort of form:
public void PerformSomeBusinessLogic(CustomerOrder order)
{
int x, y, z;
double a, b, c;
int counter;
CustomerOrder tempOrder;
int secondLoopCounter;
string output;
string firstTimeInput;
//Alright, now let's get started because this is going to be looooonnnng method...
...
}
C programmers wrote code like this because in old standards of C it was necessary to declare variables right after the opening brace of a scope before you started doing things like assignment and control flow statements. They’ve carried it forward over the years because, well, old habits die hard. Interestingly, they’re actually doing you a favor. Here’s why.
When looking at a method like this, you know you’re in for doozy. If it has this many local variables, it’s going to be long, convoluted and painful. In the C# world, it probably has regions in it that divide up the different responsibilities of the method. This is also a problem, but a lemons-to-lemonade opportunity for us. The reason is that these C-style programmers are actually telling you how to turn their giant, unwieldy method into a class. All of those variables at the top? Those are your class fields. All of those regions (or comments in languages that don’t support regioning)? Method names.
In one of the resources I’ll recommend, “Uncle” Bob Martin said something along the lines of “large methods are where classes go to hide.” What this means is that when you encounter some gigantic method that spans dozens or hundreds of lines, what you really have is something that should be a class. It’s functionality that has grown too big for a method. So what do you do? Well, you create a new class with its local variables as fields, its region names/comments as method titles, and class fields as dependencies, and you delegate the responsibility.
public class Untestable4
{
public void PerformSomeBusinessLogic(CustomerOrder order)
{
var extractedClass = new MaybeTestable();
extractedClass.Region1Title();
extractedClass.Region2Title();
extractedClass.Region3Title();
}
}
public class MaybeTestable
{
int x, y, z;
double a, b, c;
int counter;
CustomerOrder tempOrder;
int secondLoopCounter;
string output;
string firstTimeInput;
public void Region1Title()
{
...
In this example, there are no fields in the untestable class that the method is using, but if there were, one way to handle this is to pass them into the constructor of the extracted class and have them as fields there as well. So, assuming this extraction goes smoothly (and it might not be that easy if the giant method has a lot of temporal coupling, resulting from, say, recycled variables), what is gained here? Well, first of all, you’ve slain a giant method, which will inevitably be good from a design perspective. But what about testability?
In this case, it’s possible that you still won’t have testable methods, but it’s likely that you will. The original gigantic method wasn’t testable. They never are. There’s really way too much going on in them for meaningful testing to occur — too many control flow statements, loops, global variables, file I/O, etc. Giant methods are giant because they do a lot of things, and if you do enough code things you’re going to start running over the bounds of testability. But the new methods are going to be split up and more focused and there’s a good chance that at least one of them will be testable in a meaningful way. Plus, with the extracted class, you have control over the new constructor that you’re creating whereas you didn’t with the legacy class, so you can ensure that the class can at least be instantiated. At the end of the day, you’re improving the design and introducing a seam that you can get at for testing.
Ask for your dependencies — don’t declare them
Another change you can make that may be relatively straightforward is to move dependencies out of the scope of your class — especially icky dependencies. Take a look at the original version of Untestable3 again.
public class Untestable3
{
int _someField;
public Untestable3()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
_someField = TestabilityKiller.Instance.GetSomeGlobalVariableValue();
}
public int AddToGlobal(int x)
{
return x + _someField;
}
}
When instantiated, this class goes and rattles some global state cages, doing God-knows-what (icky), and then retrieves something from global state (icky). We want to get a test around the AddToGlobal method, but we can’t instantiate this class. For all we know, to get the value of “someField” the singleton gets the British Prime Minster on the phone and asks him for a random number between 1 and 1000 — and we can’t automate that in a test suite. Now, the earlier option of extracting code is, of course, viable, but we also have the option of punting the offending code out of this class. (This may or may not be practical depending on where and how this class is used, but let’s assume it is). Say there’s only one client of this code:
public class Untestable3Client
{
public void SomeMethod()
{
var untestable = new Untestable3();
untestable.AddToGlobal(12);
}
}
All we really want out of the constructor is a value for “_someField”. All of that stuff with the singleton is just noise. Because of the nature of global variables, we can do the stuff Untestable3’s constructor was doing anywhere. So what about this as an alternative?
public class Untestable3Client
{
public void SomeMethod()
{
TestabilityKiller.Instance.DoSomethingHorribleWithGlobalVariables();
var someField = TestabilityKiller.Instance.GetSomeGlobalVariableValue();
var untestable = new Untestable3(someField);
untestable.AddToGlobal(12);
}
}
public class Untestable3
{
int _someField;
public Untestable3(int someField)
{
_someField = someField;
}
public int AddToGlobal(int x)
{
return x + _someField;
}
}
This new code is going to do the same thing as the old code, but with one important difference: Untestable3 is now a liar. It’s a liar because it’s testable. There’s nothing about global state in there at all. It just takes an integer and stores it, which is no problem to test. You’re an old pro by now at unit testing that’s this easy.
When it comes to testability, the new operator and global state are your enemies. If you have code that makes use of these things, you need to punt. Punt those things out of your code by doing what we did here: executing voids before your constructors/methods are called and asking for things returned from global state or new in your constructors/methods. This is another pretty low-impact way of altering a given class to make it testable, particularly when the only problem is that a class is instantiating untestable classes or reaching out into the global state.
Ruthlessly Eliminate Law of Demeter Violations
If you’re not familiar with the idea, the Law of Demeter, or Principle of Least Knowledge, basically demands that methods refer to as few object instances as possible in order to do their work. You can look at the link for more specifics on what exactly this “law” says, and what exactly is and is not a violation, but the most common form you’ll see is strings of dots (or arrows in C++) where you’re walking an object graph: Property.NestedProperty.NestedNestedProperty.You.Get.The.Idea. (It is worth mentioning that the existence of multiple dots is not always a violation of the Law of Demeter — fluent interfaces in general and Linq in the C# world specifically are counterexamples). It’s when you’re given some object instance and you go picking through its innards to find what you’re looking for.
One of the most immediately memorable ways of thinking about why this is problematic is to consider what happens when you’re at the grocery store buying groceries. When the clerk tells you that the total is $86.28, you swipe your Visa. What you don’t do is wordlessly hand him your wallet. What you definitely don’t do is take off your pants and hand those over so that he can find your wallet. Consider the following code, bearing in mind that example:
public class HardToTest
{
public string PrepareSsnMessage(CustomerOrder order)
{
return "Social Security number is " + order.Customer.PersonalInfo.Ssn;
}
}
The method in this class just prepends an explanatory string to a social security number. So why on earth do I need something called a customer order? That’s crazy — as crazy as handing the store clerk your pants. And from a testing perspective, this is a real headache. In order to test this method, I have to create a customer, then create an order and hand that to the customer, then create a personal info object and hand that to the customer’s order, and then create an SSN and hand that to the customer’s order’s personal info. And that’s if everything goes well. What if one of those classes — say, Customer — invokes a singleton in its constructor. Well, now I can’t test the “PrepareSsnMessage” in HardToTest because the Customer class uses a singleton. That’s absolutely insane.
Let’s try this instead:
public class HardToTest
{
public string PrepareSsnMessage(string ssn)
{
return "Social Security number is " + ssn;
}
}
Ah, now that’s easy to test. And we can test it even if the Customer class is doing weird, untestable things because those things aren’t our problem. What about clients, though? They’re used to passing customer orders in, not SSNs. Well, tough — we’re making this class testable. They know about customer order and they its SSN, so let them incur the Law of Demeter violation and figure out how to clean it up. You can only make your code testable one class at a time. That class and its Law of Demeter violation is tomorrow’s project.
When it comes to testing, the more stuff your code knows about, the more setup and potential problems you have. If you don’t test your code, it’s easy to write train wrecks like the “before” method in this section without really considering the ramifications of what you’re doing. The unit tests force you to think about it — “man, this method is a huge hassle to test because problems in classes I don’t even care about are preventing me from testing!” Guess what. That’s a design smell. Problems in weird classes you don’t care about aren’t just impacting your tests — they’re also impacting your class under test, in production, when things go wrong and when you’re trying to debug.
Understand the significance of polymorphism for testing
I’ll leave off with a segue into the next chapter in the series, which is going to be about a concept called “test doubles.” I will explain that concept then and address a significant barrier that you’re probably starting to bump into in your testing travels. But that isn’t my purpose here. For now I’ll just say that you should understand the attraction of using polymorphic code for testing.
Consider the following code:
public class Customer
{
public string FirstName { get { return TestabilityKiller.Instance.GoGetCustomerFirstNameFromTheDatabase(); } }
}
public class CustomerPropertyFormatter
{
public string PrepareFirstNameMessage(Customer customer)
{
return "Customer first name is " + customer.FirstName;
}
}
Here you have a class, CustomerPropertyFormatter, that should be pretty easy to test. I mean, it just takes a customer and accesses some string property on it for formatting purposes. But when you actually write a test for this, everything goes wrong. You create a customer to give to your method and your test blows up because of singletons and databases and whatnot. You can write a test with a null argument and amend this code to handle null gracefully, but that’s about it.
But, never fear — polymorphism to the rescue. If you make a relatively small modification to the Customer class, you set yourself up nicely. All you have to do is make the FirstName property virtual. Once you’ve done that, here’s a unit test that you can write:
public class DummyCustomer : Customer
{
private string _firstName;
public override string FirstName { get { return _firstName; } }
///
/// Initializes a new instance of the DummyCustomer class.
///
public DummyCustomer(string firstName) { _firstName = firstName; } } [TestMethod, Owner(“ebd”), TestCategory(“Proven”), TestCategory(“Unit”)] public void Adds_Text_To_FirstName() { string firstName = “Erik”; var customer = new DummyCustomer(firstName); var formatter = new CustomerPropertyFormatter(); Assert.IsTrue(formatter.PrepareFirstNameMessage(customer).Contains(firstName)); }
Notice that there is a class, DummyCustomer declared inside of the test class that inherits from the Customer class. DummyCustomer is an example of a test double. You’ll notice that I’ve created a scenario here where I define a version of FirstName that I can control — a benign version, if you will. I effectively bypass that database-singleton thing and create a version of the class that exists only in the test project and allows me to substitute a simple, friendly value that I can test against.
As I said, I’ll dive much more into test doubles next time, but for the time being, understand the power of polymorphism for testability. If the legacy code has methods in it that are hard to use, you can create much more testable situations by the use of interface implementation, inheritance, and the virtual keyword. Conversely, you can make testing a nightmare by using keywords like final and sealed (Java and C# respectively). There are valid reasons to use these, but if you want a testable code base, you should favor liberal support of inheritance and interface implementation.
A Note of Caution
In the sections above, I’ve talked about refactorings that you can do on legacy code bases and mentioned that there is some risk associated with doing so. It is up to you to assess the level of risk of touching your legacy code, but know that any changes you make to legacy code without first instrumenting unit tests can be breaking changes, even small ones guided by automated refactoring tools. There are ways to ‘cheat’ and tips and techniques to get a method under test before you refactor it, such as temporarily making private fields public or local variables into public fields. The Michael Feathers book below talks extensively about these techniques to truly minimize the risk.
The techniques that I’m suggesting here would be ones that I’d typically undertake when requirements changes or bugs were forcing me to make a bunch of changes to the legacy code anyway, and the business understood and was willing to undertake the risk of changing it. I tend to refactor opportunistically like that. What you do is really up to your discretion, but I don’t want to be responsible for you doing some rogue refactoring and torpedoing your production code because you thought it was safe. Changing untested legacy code is never safe, and it’s important for you to understand the risks.
More Information
As mentioned earlier, here are some excellent resources for more information on working with and testing legacy code bases:
In the last post in this series, I covered the essentials of unit testing without diving into the topic in any real detail. The idea was to offer sort of a guerrilla guide to what unit testing is for those who don’t know but feel they should. Continuing on that path and generating material for my upcoming presentation, I’m going to continue with the introduction.
In the previous post, I talked about what unit testing is and isn’t, what its actual purpose is, and what some best practices are. This is like explaining the Grand Canyon to someone that isn’t familiar with it. You now know enough to say that it’s a big hole in the earth that provides some spectacular views and that you can hike down into it, seeing incredible shifts in flora and fauna as you go. You can probably convince someone you’ve been there in a casual conversation, even without having seen it. But, you won’t really get it until you’re standing there, speechless. With unit testing, you won’t really get it until you do it and get some benefit out of it.
So, Let’s Write that Test
Let’s say that we have a class called PrimeFinder. (Anyone who has watched my Pluralsight course will probably recognize that I’m recycling the example class from there.) This class’s job is to determine whether or not numbers are prime, and it looks like this:
public class PrimeFinder
{
public bool IsPrime(int possiblePrime)
{
return possiblePrime != 1 && !Enumerable.Range(2, (int)Math.Sqrt(possiblePrime) - 1).Any(i => possiblePrime % i == 0);
}
}
Wow, that’s pretty dense looking code. If we take the method at face value, it should tell us whether a number is prime or not. Do we believe the method? Do we have any way of knowing that it works reliably, apart from running an entire application, finding the part that uses it, and poking at it to see if anything blows up? Probably not, if this is your code and you needed my last post to understand what a unit test was. But this is a pretty simple method in a pretty simple class. Doesn’t it seem like there ought to be a way to make sure it works?
I know what you’re thinking. You have a scratchpad and you copy and paste code into it when you want to experiment and see how things work. Fine and good, but that’s a throw-away effort that means nothing. It might even mislead when your production code starts changing. And checking might not be possible if you have a lot of dependencies that come along for the ride.
But never fear. We can write a unit test. Now, you aren’t going to write a unit test just anywhere. In Visual Studio, what you want to do is create a unit test project and have it refer to your code. So if the PrimeFinder class is in a project called Daedtech.Production, you would create a new unit test project called DaedTech.Production.Test and add a project reference to Daedtech.Production. (In Java, the convention isn’t quite as cut and dry, but I’m going to stick with .NET since that’s my audience for my talk). You want to keep your tests out of your production code so that you can deploy without also deploying a bunch of unit test code.
Once the test class is in place, you write something like this, keeping in mind the “Arrange, Act, Assert” paradigm described in my previous post:
[TestMethod]
public void Returns_False_For_One()
{
var primeFinder = new PrimeFinder(); //Arrange
bool result = primeFinder.IsPrime(1); //Act
Assert.IsFalse(result); //Assert
}
The TestMethod attribute is something that I described in my last post. This tells the test runner that the method should be executed as a unit test. The rest of the method is pretty straightforward. The arranging is just declaring an instance of the class under test (commonly abbreviated CUT). Sometimes this will be multiple statements if your CUTs are more complex and require state manipulation prior to what you’re testing. The acting is where we test to see what the method returns when we pass it a value of 1. The asserting is the Assert.IsFalse() line where we instruct the unit test runner that a value of false for result means the test should pass, but true means that it should fail since 1 is not prime.
Now, we can run this unit test and see what happens. If it passes, that means that it’s working correctly, at least for the case of 1. Maybe once we’re convinced of that, we can write a series of unit tests for a smattering of other cases in order to convince ourselves that this code works. And here’s the best part: when you’re done exploring the code with your unit tests to see what it does and convince yourself that it works (or perhaps you find a bug during your testing and fix the code), you can check the unit tests into source control and run them whenever you want to make sure the class is still working.
Why would you do that? Well, might be that you or someone else later starts playing around with the implementation of IsPrime(). Maybe you want to make it faster. Maybe you realize it doesn’t handle negative numbers properly and aim to correct it. Maybe you realize that method is written in a way that’s clear as mud and you want to refactor toward readability. Whatever the case may be, you now have a safety net. No matter what happens, 1 will never be prime, so the unit test above will be good for as long as your production code is around–and longer. With this test, you’ve not only verified that your production code works now; you’ve also set the stage for making sure it works later.
Resist the Urge to Write Kitchen Sink Tests
When I talked about a smattering of tests, I bet you had an idea. I bet your idea was this:
[TestMethod]
public void Test_A_Bunch_Of_Primes()
{
var primes = new List() { 2, 3, 5, 7, 11, 13, 17 };
var primeFinder = new PrimeFinder();
foreach(var prime in primes)
Assert.IsTrue(primeFinder.IsPrime(prime));
}
After all, it’s wasteful and inefficient to write a method for each case that you want to test when you could write a loop and iterate more succinctly. It’ll run faster, and it’s more concise from a coding perspective. It has every advantage that you’ve learned about in your programming career. This must be good. Right?
Well, not so much, counterintuitive as that may seem. In the first place, when you’re running a bunch of unit tests, you’re generally going to see their result in a unit test runner grid that looks something like a spreadsheet. Or perhaps you’ll see it in a report. If when you’re looking at that, you see a failure next to “IsPrime_Returns_False_For_12” then you immediately know, at a glance, that something went wrong for the case of 12. If, instead, you see a failure for “Test_A_Bunch_Of_Primes”, you have no idea what happened without further investigation. Another problem with the looping approach is that you’re masking potential failures. In the method above, what information do you get if the method is wrong for both 2 and 17? Well, you just know that it failed for something. So you step through in the debugger, see that it failed for 2, fix that, and move on. But then you wind up right back there because there were actually two failures, though only one was being reported.
Unit test code is different from regular code in that you’re valuing clarity and the capture of intent and requirements over brevity and compactness. As you get comfortable with unit tests, you’ll start to give them titles that describe correct functionality of the system and you’ll use them as kind of a checklist for getting code right. It’s like your to-do list. If every box is checked, you feel pretty good. And you can put checkboxes next to statements like “Throws_Exception_When_Passed_A_Null_Value” but not next to “Test_Null”.
There are some very common things that new unit testers tend to do for a while before things click. Naming test methods things like “Test_01” and having dozens of asserts in them is very high on the list. This comes from heavily procedural thinking. You’ll need to break out of that to realize the benefit of unit testing, which is inherently modular because it requires a complete breakdown into components to work. If nothing else, remember that it’s, “Arrange, Act, Assert,” not, “Arrange, Act, Assert, Act, Assert, Assert, Assert, Act, Act, Assert, Assert, Act, Assert, etc.”
Wrap-Up
The gist of this installment is that unit tests can be used to explore a system, understand what it does, and then guard to make sure the system continues to work as expected even when you are others are in it making changes later. This helps prevent unexpected side effects during later modification (i.e. regressions). We’ve also covered that unit tests are generally small, simple and focused, following the “Arrange, Act, Assert” pattern. No one unit test is expected to cover much ground–that’s why you build a large suite of them.
While I’ve been switching a lot of my new projects over to using JustMock, as explained here, I still have plenty of existing projects with Moq, and I still like Moq as a tool. One of the things that I have struggled with in Moq that JustMock provides a nice solution to is the cognitive dissonance that arises when thinking about how to reason about your test doubles.
In some situations, such as when you’re injecting the test doubles into your class under test, you want them to appear as if they were just another run of the mill instance of whatever interface/base you’re passing. But at other times you want to configure the mock, and then you want to think of them as actual mock objects. One solution to that is to use Mock.Get() as I described here. Doing that, you can store a reference to the object as if it were any other object but access it using Mock.Get() when you want to do things to its setup:
[gist id=”4998372″]
That’s all well and good, but, perfectionist that I am, I got really tired of all of that Mock.Get() ceremony. It makes lines pointlessly longer, and I think really detracts from the readability of the test classes. So I borrowed an idea from JustMock with its “Helpers” namespace and created a series of extension methods. Some examples are shown here.
[gist id=”4998277″]
These extension methods allow me to alter my test to look like this:
[gist id=”4998488″]
Now there’s no need to switch context, so to speak, between mock object and simple dependency. You always have a simple dependency that just so happens to have some extension methods that you can use for configuration. No need to keep things around as Mock<T> and call the Object property, and no need for all of the Mock.Get().
Of course, there are caveats. This might already exist somewhere and I’m out of the loop. There might be issues with this that I haven’t yet hit, and there is the debugging indirection. And finally, you could theoretically have namespace collisions, though if you’re making methods called “Setup” and “Verify” on your classes that take expression trees and multiple generic parameters, I’d say you have yourself a corner case there, buddy. But all that aside, hopefully you find this useful–or at least a nudge in a direction toward making your own tests a bit more readable or easy to work with.
I am Erik Dietrich, founder of DaedTech. I’m a former programmer, architect, and IT management consultant, and current founder and CEO of Hit Subscribe.
