Ivan

Retrofitting unit tests

Sometimes you need to retrofit unit tests to existing code that doesn't have unit tests, in order to fix it, modify it or refactor it.
This can be difficult, because code that has not been developed with unit tests will sometimes have dependencies that mean that tests written against the code as it stands would also test the code that it depends upon.

This often results in having to set up lots of dependencies which can take programming time and often make the tests run slower too.
This article describes one particular scenario and several different ways of retrofitting tests to code that has dependencies. These different approaches provide different trade-offs to get the immediate job done (e.g. a bug fix) verses long term maintainability.
This article assumes an understanding of Mock Objects12 but isn't really about Mock Objects. This article has bits of Dependency Injection3 in it too, but isn't about that either. It's about how to add tests to untested code that you need to fix.

I will present a few different options and won't make a blanket statement about which one is "best". It's a judgement call about balancing the effort of making the test code look good, making the non-test code look good, not changing too much of the non-test code (e.g. because the test coverage is very poor so making any changes is risky), and getting the job done, e.g. getting the test written that helps you fix some bug.

Consider the following Java class:

public class Legacy {
	public void doSomething(){
		Dependency dependency = new Dependency(someConstructionValue());
		dependency.foo(someParameter());
	}
	...

where someConstructionValue() and someParameter() are private methods or some code that could be extracted into methods.

The code and names are rather abstract, as the approaches described here are applicable in many different circumstances.
The creation of the dependency object could equally be by a static method call, e.g:
"Singleton.createDependency(someConstructionValue())" instead of:
"new Dependency(someConstructionValue())" - for the purposes of this article they are the same, however, I'll stick with the constructor.

Examples could be where the Dependency is an EJB or a Database.

Let's say you've found a bug where the wrong value of something is being put in a database. You track down the bug and find that the value of "someParameter()" is wrong in some circumstances; that gets passed into the Dependency (lets say an EJB) which then causes it to put the wrong value in the database. To write a test without isolating the dependencies, you would need to be able to set up a Legacy object in the same state that caused the bug and then check what the value was in the database. There are lots of problems with this. If the test fails, you can't be sure where the problem lies - whether it's in the Legacy class or in the Dependency. Using the EJB/Database layers will make the test run slowly too.

All you really want to test is that the Dependency is passed the correct value - you can have other tests for the Dependency as to what it does when it is passed the correct value. For the rest of this article, I won't be considering what you need to do to set up the Legacy object; rather, just it's dependencies.

For the bug described earlier, you have a few options. You could make the method someParameter() public and write a test calling that directly. This is simple and quick to do, but exposes a method that would otherwise be private. Furthermore, you only know that the
value of this method will be used by reading the code - the test doesn't confirm that the value from someParameter() is actually passed into the dependency object. Nevertheless, it's quick and might get the job done. You might not be proud doing that but if it fixes some critical bug really quickly you might be best doing the fix by writing a test this way and implementing a better solution later.

Maybe better would be to test that the correct object is passed in to the call on the dependency object using a Mock Object. However, in this method, we have a problem: the Dependency object is created within the method so we can't subsitute a Mock Object for the constructed object.

There are several ways around this.

Local Injection

One approach would be to change the code to:

	public void doSomething(Dependency dependency){
		dependency.foo(someParameter());
	}
	public void doSomething(){
		Dependency dependency = new Dependency(someConstructionValue());
		doSomething(dependency);
	}

This means you can pass in a Mock Object version of the dependency to doSomething(Dependency) and write the test against that method. Again, this has the problem of exposing a method just for testing, and there is no guarantee that doSomething() calls doSomething(Dependency). To test using a Mock Object, doSomething(Dependency) would be called by the test, passing in a Mock Object that implements the Dependency interface, and the test would be that the dependency expects to have the foo method called with the correct parameter (see 1 and 2).

Constructor-based Dependency Injection

Many options depend on whether someConstructionValue() is a fixed value or not and whether the Dependency object is stateful.
If the value of someConstructionValue() is a fixed value and the Dependency object is not stateful, you can pass in the Dependency object on construction, which keeps the interface of Legacy the same but allows for substituting the Dependency with a Mock Object.

public class Legacy {
	Dependency dependency;
	public Legacy(Dependency dependency){
		this.dependency = dependency;
	}
	public void doSomething(){
		dependency.foo(someParameter());
	}

Other options for Dependency Injection are covered in3. If Legacy used to have a no-arg constructor, you either have to change the code that does that construction, or in this case you might consider the slightly ugly option of having two constructors, the one taking a Dependency for testing and a no-arg one:

	public Legacy(){
		this(new Dependency(someConstructionValue()));
	}

Factory

How about if someConstructionValue() isn't fixed so you can't just create the dependency on construction? The "Local Injection" approach described earlier still works.

If you want to use a Dependency Injection style approach, you could pass in a mock DependencyFactory. In the real construction of a Legacy object, it would use a real DependencyFactory with a very simple implementation:

public class RealDependencyFactory implements DependencyFactory{
	public Dependency newDependency(Object someConstructionValue){
		return new Dependency(someConstructionValue);
	}
}

and Legacy becomes:

public class Legacy {
	DependencyFactory factory;
	public Legacy(DependencyFactory factory){
		this.factory = factory;
	}
	public void doSomething(){
		Dependency dependency = factory.newDependency(someConstructionValue());
		dependency.foo(someParameter());
	}

This does the job but adds quite a lot of code. Even worse, in order to write the test, you need to set up the Mock DependencyFactory to return a Mock Dependency, which is what you really want to set expectations on.

Localize constructor parameter

If the Dependency is part of your code base, and if it's essentially stateless, it might be that it's interface can be changed so the value that was passed in on construction could instead be passed in as parameter on the foo method. The code would then look like:

public class Legacy {
	Dependency dependency;
	public Legacy(Dependency dependency){
		this.dependency = dependency;
	}
	public void doSomething(){
		dependency.foo(someConstructionValue(), someParameter());
	}

and the test could pass in a Mock Dependency with expectations on the someParameter value or even on the someConstructionValue too, which would be useful if this needs testing too.

Adaptor

If the "localize constructor parameter" approach isn't possible (e.g. the Dependency isn't part of your code base) you could create an adaptor to do that.

If the Dependency class is:

public class Dependency {
	public Dependency(Object object) {
		...
	}

	public void foo(Object object) {
		...
	}
}

Then you can create an adaptor class:

public class DependencyAdaptor {
	public void foo(Object constructionValue, Object parameter) {
		new Dependency(constructionValue).foo(parameter);
	}
}

And Legacy becomes:

public class Legacy {
	DependencyAdaptor dependency;
	public Legacy(DependencyAdaptor dependency){
		this.dependency = dependency;
	}
	public void doSomething(){
		dependency.foo(someConstructionValue(), someParameter());
	}
	...

Factory method

Instead of the "adaptor" approach, another possibility would be the following:

public class Legacy {
	public void doSomething(){
		Dependency d = newDependency();
		d.foo(someParameter());
	}
	protected Dependency newDependency(){
		return new Dependency(someConstructionValue());
	}

Then, in your test code, you could create a subclass:

public class LegacyExtensionForTesting extends Legacy{
	public Dependency mockDependency;
	
	protected Dependency newDependency(){
		return mockDependency;
	}

and write tests against the subclass, setting the mockDependency and it's expectations. This might look terribly hacky, but will get the job done and doesn't impact the non-test code too much.

Many thanks to my ThoughtWorks colleagues: Owen Rogers, Jim Arnold, Chris Stevenson and Nat Pryce, for their comments on this article.

Posted by ivan at August 13, 2004 2:10 PM