Don't mix test code with production code

Don't mix test code with production code

Dariusz Pasciak

December 19, 2014

There are many "do's and don'ts" when it comes to testing code. I would like to focus on one of the "don'ts" that comes up somewhat frequently.

It is not uncommon to see production code that looks something like this:


class WinterAssistant {
				public boolean isTest = false;

				public boolean needWinterJacket() {
								Thermometer thermometer;
								if (isTest) {
												thermometer = new FakeThermometer();
								} else {
												thermometer = new Thermometer();
								}

								return thermometer.getTemperature() < 40;
				}
}

Although this code is successful at removing the dependency on Thermometer in tests, it clutters the WinterAssistant code with test code.

Here are a few alternatives that we can try to this approach:

  • inject Thermometer as a parameter to needWinterJacket
  • inject Thermometer into the WinterAssistant constructor and keep a reference to it in an instance variable
  • extract the construction of Thermometer into a protected method on WinterAssistant

The first two options are pretty self-explanatory, so let's cover those briefly now. In your test code, you construct an instance of FakeThermometer and pass that instance in when constructing WinterAssistant or calling needWinterJacket. In production code, clients of WinterAssistant construct an instance of the real Thermometer and pass that in when constructing WinterAssistant or calling needWinterJacket. This way, you can effectively test your WinterAssistant completely independent of any sort of real Thermometers. At the same time, your WinterAssistant stays free of nasty if( isTest ) blocks that obscure the rest of the code that is relevant to the reader.

So we would have something like the following if we choose to go with option #1.


class WinterAssistant {
				public boolean needWinterJacket(Thermometer thermometer) {
								return thermometer.getTemperature() < 40;
				}
}

Or this if we go with option #2.


class WinterAssistant {
				private Thermometer thermometer;

				public WinterAssistant(Thermometer thermometer){
								this.thermometer = thermometer;
				}

				public boolean needWinterJacket() {
								return thermometer.getTemperature() < 40;
				}
}

Let's investigate the third option. This is one of my favorite methods1 in situations where clean solutions seems impossible to achieve2. This route may be necessary when circumstances prevent you from changing the signature of needWinterJacket or you have no choice but to construct an instance of Thermometer inside your WinterAssistant class.

First, move the construction of Thermometer into a protected method.


class WinterAssistant {
				public boolean needWinterJacket() {
								return createThermometer().getTemperature() < 40;
				}

				protected Thermometer createThermometer(){
								return new Thermometer();
				}
}

Then, in your test code, override createThermometer() so that it returns an instance of your FakeThermometer.


class WinterAssistantStub extends WinterAssistant {
				public FakeThermometer thermometer = new FakeThermometer();

				@Override
				protected Thermometer createThermometer(){
								return thermometer;
				}
}

... and finally, somewhere in your test, instantiate an instance of your WinterAssistantStub instead of the real WinterAssistant and test against that. You will still be testing the real needWinterJacket method, but it will now be using an instance of FakeThermometer instead of the real Thermometer that it would normally construct.

WinterAssistantStub assistant = new WinterAssistantStub();
assistant.thermometer.setCurrentTemperature(39);

assertTrue(assistant.needWinterJacket());

This is a very effective technique for pulling out and faking dependencies. In fact, it is so useful in automated software testing that tools have been created to make it easier and cleaner to do. One such tool is called Mockito.

Mockito takes care of the subclassing and overriding for you. So no more defining a new class, manually overriding the protected methods, and creating and setting instance variables to stub return values.

Let's take a look at what our test would look like if we leverage some tools from Mockito.


Thermometer thermometer = Mockito.stub(Thermometer.class);
doReturn(39).when(thermometer).getTemperature();

WinterAssistant assistant = Mockito.spy(new WinterAssistant());
doReturn(thermometer).when(assistant).createThermometer();

assertTrue(assistant.needWinterJacket());

And that's it! No more subclassing or overriding or temporary variables.

What's happening there? Let's break it down line by line.

First, we create a Mockito stub of Thermometer.

Thermometer thermometer = Mockito.stub(Thermometer.class);

What this does is give us an instance of a class that has the same exact interface as a Thermometer. Because its interface is identical, we can use it and pass it to anything that expects a Thermometer.

We can make this instance behave however we please. We would like it to return the number 39 whenever getTemperature() is called on it. That's exactly what this next line does.

doReturn(39).when(thermometer).getTemperature();

Moving forward, the next line creates a spy3 on a new instance of WinterAssistant.


WinterAssistant assistant = Mockito.spy(new WinterAssistant());

This gives us an instance of a class that basically is a WinterAssistant, but we can also make it behave exactly as we please. If we don't do anything else with it, then assistant would basically behave exactly as an instance of WinterAssistant. However, we do change it slightly. We do that on the next line.

doReturn(thermometer).when(assistant).createThermometer();

We stub out the createThermometer method so that instead of executing the "real" code defined on the original class, that code is bypassed and instead returns the thermometer that we created on the first line.

All these pieces together effectively inject our own instance of Thermometer into the needWinterJacket method when it calls createThermometer. The effects of this code are the same as our earlier version, but a lot shorter and a bit easier to read and follow.


1 Michael Feathers describes this method in great detail in his book Working Effectively With Legacy Code.

2 Let me reiterate that. Although I am a big fan of this method, I do not recommend it as the first thing to try when you need to separate dependencies. There are better and much nicer alternatives; use this route only if those alternatives are not feasible.

3 The main difference between a Mockito stub and a Mockito spy is that calling a method on a stub will never call through to the original method defined on the class, whereas calling a method on a spy will always call through to the original method defined on the class, unless you do something to prevent that (i.e. doReturn)