Mark Needham

James Shore has an interesting recent blog post where he describes some alternatives to over reliance on acceptance testing and one of the ideas that he describes is fixing the process whenever a bug is found in exploratory testing.

He describes two ways of preventing bugs from making it through to exploratory testing:

• Make the bug impossible
• Catch the bug automatically

Sometimes we can prevent defects by changing the design of our system so that type of defect is impossible. For example, if find a defect that's caused by mismatch between UI field lengths and database field lengths, we might change our build to automatically generate the UI field lengths from database metadata.

When we can't make defects impossible, we try to catch them automatically, typically by improving our build or test suite. For example, we might create a test that looks at all of our UI field lengths and checks each one against the database.

We had an example of the latter this week around some code which loads rules out of a database and then tries to map those rules to classes in the code through use of reflection.

For example a rule might refer to a specific property on an object so if the name of the property in the database doesn't match the name of the property on the object then we end up with an exception.

This hadn't happened before because we hadn't been making many changes to the names of those properties and when we did people generally remembered that if they changed the object then they should change the database script as well.

Having that sort of manual step always seems a bit risky to me since it's prone to human error, so having worked out what was going on we wrote a couple of integration tests to ensure that every property in the database matched up with those in the code.

We couldn't completely eliminate this type of bug in this case because the business wanted to have the rules configurable on the fly via the database.

It perhaps seems quite obvious that we should look to write these types of tests to shorten the feedback loop and allow us to catch problems earlier than we otherwise would but it's easy to forget to do this so James' post provides a good reminder!

I've read several times about the dangers of the big rewrite when it comes to production code but I've recently been wondering whether or not we should apply the same rules when it comes to test code or not.

I worked with Raphael Speyer for a few weeks last year and on the code base we were working on he often spent some time rewriting tests originally written using rMock to use mockito which was the framework we were driving towards.

One of the benefits that he was able to get from doing this was that he had to understand the test in order to change it which enabled him to increase his understanding of how the code was supposed to work and identify anything that didn't seem quite right.

I quite liked this idea at the time and while spending some time recently working with some tests which required quite a lot of setup and were testing several different things in the same test.

Unfortunately a few of them were failing and it was quite difficult to work out why that was.

My initial approach was to try and work my way through the tests inlining all the test code to start with and then extracting out irrelevant details to make the tests easier to understand.

Despite those attempts I was unable to work out why the test was failing so I worked out what the main things the test was trying to verify and then wrote tests from scratch for each of those cases.

I was able to write tests covering everything the original test did in several smaller tests in less time than I had spent trying to debug the original one and with a fair degree of confidence that I'm testing exactly the same thing.

As I see it the big danger of rewriting is that we're always playing catch up with the current system which is still being worked on in production and we never quite catch up.

I'm not so sure this logic applies in this case because we're only rewriting small bits of code which means that we can replace the original test very quickly.

My main driver when working with tests is to ensure that they're easy to understand and make it easy to reason about the code so if I have to rewrite some tests to make that happen then I think it's a fair trade off.

My initial approach would nearly always be to refactor the tests that are already there. Rewriting is something I'd look to do if I was really struggling to refactor effectively.

I wrote a couple of weeks ago about my thoughts on hand written stubs vs framework generated stubs and I noticed an interesting situation where it helped me out while trying to simplify some test code.

The code in question was making use of several framework generated stubs/mocks and one in particular was trying to return different values depending on the value passed as a parameter.

The test was failing and I spent about half an hour unsuccessfully trying to work out why it wasn't working as expected before I decided to replace it with a hand rolled stub that did exactly what I wanted.

This is a simplified version of the test:

[Test]
public void SomeTest()
{
	var service = MockRepository.GenerateStub<IService>();
 
	service.Stub(x => x.SomeMethod("call1")).Return("aValue");
	service.Stub(x => x.SomeMethod("call2")).Return("anotherValue");
 
	// and so on
 
	new SomeObject(service).AnotherMethod();
 
       // some assertions
}

For the sake of the test I only wanted 'service' to return a value of 'aValue' the first time it was called and then 'anotherValue' for any other calls after that.

I therefore wrote the following hand rolled stub to try and simplify the test for myself and plugged it into the original test:

public class AValueOnFirstCallThenAnotherValueService : IService
{
	private int numberOfCalls = 0;
 
	public string SomeMethod(string parameter)
	{
		if(numberOfCalls == 0)
		{
			numberOfCalls++;
			return "aValue";
		}
		else
		{
			numberOfCalls++;
			return "anotherValue";
		}
	}
}

[Test]
public void SomeTest()
{
	var service = new AValueOnFirstCallThenAnotherValueService();
 
	new SomeObject(service).AnotherMethod();
 
       // some assertions
}

I've never tried this particular approach before but it made it way easier for me to identify what was going wrong and I was then able to get the test to work as expected and move onto the next one.

In retrospect it should have been possible for me to work out why the original framework generated stub wasn't working but it seemed like the right time to cut my losses and the time to write the hand generated one and get it working was an order of magnitude less.

I got the chance to work with Phil for a couple of weeks last year and one of the most interesting things that he started teaching me was the importance of reducing the clutter in our tests and ensuring that we take some time to refactor them as well as the code as part of the 'red-green-refactor' cycle.

I'm still trying to work out the best way to do this but I came across a really interesting post by J.B. Rainsberger where he describes how he removes irrelevant details from his tests.

Since I worked with Phil I've started noticing some of the ways that we can simplify tests so that they are more useful as documentation of how our system works.

Wrapping methods around irrelevant test builders

One thing I've noticed in tests recently is that generally most of the setup code for a test is irrelevant and is just there to get the test to actually run. There's very little that's actually interesting and more often than not it ends up getting hidden amongst the other irrelevant stuff.

The test builder pattern is a really useful one for allowing us to easily setup test data but I feel that if we're not careful it contributes to the clutter.

To describe a contrived example:

[Test]
public void SomeTest() 
{
	var bar = BarBuilder.Build.WithBaz("baz").BuildBar();
	var foo = FooBuilder.Build.Bar(bar).BuildFoo();
 
	var someObject = new SomeObject();
	var result = someObject.SomeMethod(foo);
 
	Assert.That(result.Baz, Is.EqualTo("baz");
}

In this example 'Foo' is actually not important at all. What we're really interested in is Baz which happens to be accessed via Foo.

I've started refactoring tests like that into the following style:

[Test]
public void SomeTest() 
{
	var bar = BarBuilder.Build.WithBaz("baz").BuildBar();
 
	var someObject = new SomeObject();
	var result = someObject.SomeMethod(FooWith(bar));
 
	Assert.That(result.Baz, Is.EqualTo("baz");
}
 
private Foo FooWith(Bar bar)
{
	return FooBuilder.Build.Bar(bar).BuildFoo();
}

In this example it doesn't make much different in terms of readability but when there's more dependencies it works quite well for driving the test into a state where all we see are the important details that form part of the 'Arrange – Act – Assert' pattern.

New up object under test inline

Object initialisation is often not worthy of a variable in our test since it doesn't really add anything to our understanding of the test.

I only really break this rule when we need to call one method on the object under test and then need to call another method to verify whether or not the expected behaviour happened.

More often than not this is only the case when dealing with framework code. It's much easier to avoid this in our own code.

In the example that I started with we can inline the creation of 'SomeObject' without losing any of the intent of the test:

[Test]
public void SomeTest() 
{
	var bar = BarBuilder.Build.WithBaz("baz").BuildBar();
 
	var result = new SomeObject().SomeMethod(FooWith(bar));
 
	Assert.That(result.Baz, Is.EqualTo("baz");
}

The only time I don't do this is when the constructor takes in a lot of dependencies and keeping it all inlined would take the code off the right side of the screen.

In any case it's a sign that something's gone wrong and the object probably has too many dependencies so we need to try and fix that.

Pull up static dependencies into fields

Another technique I've been trying is pulling static dependencies i.e. ones whose values are not mutated in the test up into fields and initialising them there.

A typical example would be in tests that have a clock.

[Test]
public void ShouldShowFoosOlderThanToday()
{
	var clock = new ControlledClock(new DateTime(2010,1,16));
	var fooService = MockRepository.GenerateStub<IFooService>();
 
	var fooFromYesterday = new Foo { Date = 1.DayBefore(clock) });
	var aCollectionOfFoos = new List<Foo> { fooFromYesterday };
	fooService.Stub(f => f.GetFoos()).Return(aCollectionOfFoos);
 
	var oldFoos = new FooFinder(clock, fooService).GetFoosFromEarlierThanToday();
 
	Assert.That(oldFoos.Count, Is.EqualTo(1));
	// and so on
}

I would pull the clock variable up to be a field since the value we want to return for it is going to be the same for the whole test fixture.

[TestFixture]
public class TheTestFixture
{
	private readonly IClock Clock = new ControlledClock(new DateTime(2010,1,16));
 
	[Test]
	public void ShouldShowFoosOlderThanToday()
	{
		var fooService = MockRepository.GenerateStub<IFooService>();
 
		var fooFromYesterday = new Foo { Date = 1.DayBefore(clock) });
		var aCollectionOfFoos = new List<Foo> { fooFromYesterday };
		fooService.Stub(f => f.GetFoos()).Return(aCollectionOfFoos);
 
		var oldFoos = new FooFinder(Clock, fooService).GetFoosFromEarlierThanToday();
 
		Assert.That(oldFoos.Count, Is.EqualTo(1));
		// and so on
	}
}

I'm less certain what I would do with 'fooService'. I've run into problems previously by pulling these types of dependencies up into a setup method if we've also moved the corresponding 'Stub' or 'Mock' call as well. With that setup the intent of the test is now in two places which makes it more difficult to understand.

In Summary

It's really interesting to read about the way that others are trying to write better tests and Brian Marick also has a post where he describes how he is able to create even more intention revealing tests in a dynamic language.

It'd be cool to here some more ideas around this.

A few months ago Uncle Bob wrote a post about TDD where he suggested that he preferred to use hand created stubs in his tests wherever possible and only resorted to using a Mockito created stub as a last resort.

I wrote previously about my thoughts of where to use each of the two approaches and one example of where hand written stubs seems to make sense is the clock.

I wonder if this ties in with J.B. Rainsberger's theory of irrelevant details in the tests which make use of it.

We would typically define an interface and stub version of the clock like so:

public interface IClock
{
	DateTime Now();
}

public class ControlledClock : IClock
{
	private readonly DateTime dateTime;
 
	public ControlledClock(DateTime dateTime)
	{
		this.dateTime = dateTime;
	}
 
	public DateTime Now() 
	{ 
		return dateTime; 
	}
}

I forgot about it to start with and was stubbing it out using Rhino Mocks but I realised that every single test needed something similar to the following code:

1
2
3

var theCurrentTime = new DateTime(2010, 1, 16);
var clock = MockRepository.GenerateStub<IClock>();
clock.Stub(c => c.Now()).Return(theCurrentTime);

We can extract out the creation of the first two lines into fields but the third line remains and typically that might end up being pushed into a setup method which is run before each test.

With a clock it's maybe not such a big deal but with other dependencies from my experience it can become very difficult to follow where exactly the various return values are being setup.

When we use a hand written stub we only have to write the following code and then the date is controlled everywhere that calls 'Now()':

private readonly IClock clock = new ControlledClock(new DateTime(2010,1,16));

Following on from that my colleague Mike Wagg suggested the idea of creating extension methods on integers to allow us to fluently define values relative to the clock.

[Test]
public void ShouldShowFoosOlderThanToday()
{
	var clock = new ControlledClock(new DateTime(2010,1,16));
	var fooService = MockRepository.GenerateStub<IFooService>();
 
	var fooFromYesterday = new Foo { Date = 1.DayBefore(clock) });
	var aCollectionOfFoos = new List<Foo> { fooFromYesterday };
	fooService.Stub(f => f.GetFoos()).Return(aCollectionOfFoos);
 
	var oldFoos = new FooFinder(clock, fooService).GetFoosFromEarlierThanToday();
 
	Assert.That(oldFoos.Count, Is.EqualTo(1));
	// and so on
}

The extension method on integer would be like this:

public static class IntegerExtensions
{
	public static DateTime DayBefore(this int value, IClock clock)
	{
		return clock.Now.Subtract(TimeSpan.FromDays(value));
	}
}

It reads pretty well and seems more intention revealing than any other approaches I've tried out so I think I'll be continuing to use this approach.

A few months ago Uncle Bob wrote a post about TDD where he suggested that he preferred to use hand created stubs in his tests wherever possible and only resorted to using a Mockito created stub as a last resort.

I've tended to use framework created ones but my colleague Matt Dunn and I noticed that it didn't seem to work out too well for us writing some tests around a controller where the majority of our tests were making exactly the same call to that repository and expected to receive the same return value but a few select edge cases expected something different.

The edge cases came along later on by which time we'd already gone past the stage of putting the stub expectation setup in every test and had moved it up into a setup method which ran before all test.

We tried to keep on using the same stub instance of the dependency which meant that we were trying to setup more than one stub expectation on the same method with different return values.

[TestFixture]
public SomeControllerTests
{
	private ITheDependency theDependency;
 
	[SetUp]
	public void Before()
	{
		theDependency = MockRepository.GenerateStub<ITheDependency>();
		theDependency.Stub(t => t.SomeMethod()).Return(someResult);
 
		controller = new Controller(theDependency);
	}
 
	....
 
	[Test]
	public void SomeAnnoyingEdgeCaseTest()
	{
		theDependency.Stub(t => t.SomeMethod().Return(someOtherResult);
 
		controller.DoSomething();
 
		// and so on...
	}
 
}

We were struggling to remember where we had setup the various return values and could see a few ways to try and reduce this pain.

The first was to extract those edge case tests out into another test fixture where we would setup the stub expectation on a per test basis instead of using a blanket approach for all of them.

This generally works pretty well although it means that we now have our tests in more than one place which can be a bit annoying unless you know that's what's being done.

Another way which Matt suggested is to make use of a hand written stub for the tests which aren't bothered about the return value and then use a framework created one for specific cases.

The added benefit of that is that it's more obvious that the latter tests care about the result returned because it's explicitly stated in the body of the test.

We found that our tests were easier to read once we'd done this and we spent much less time trying to work out what was going on.

I think framework generated stubs do have thier place though and as Colin Goudie points out in the comments on Uncle Bob's post it probably makes more sense to make use of a framework generated stub when each of our tests returns different values for the same method call.

If we don't do that then we end up writing lots of different hand written stubs which I don't think adds much value.

I still start out with a framework generated stub but if it's becoming overly complicated or repetitive then I'm happy to switch to a hand written one.

It'd be interesting to hear others' approaches on this.

A fairly common discussion that I've had with several of my colleagues is around the way that we name the variables used for mocks and stubs in our tests.

There seems to be about a 50/50 split between including 'Stub' or 'Mock' on the end of those variable names and not doing so.

In a simple example test using Rhino Mocks as the testing framework this would be the contrast between the two approaches:

[Test]
public void ShouldDoSomething()
{
	var someDependency = MockRepository.CreateMock<ISomeDependency>();
 
	someDependency.Expect(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependency);
        myController.DoSomething()
 
	someDependency.VerifyAllExpectations();
}

[Test]
public void ShouldDoSomething()
{
	var someDependencyMock = MockRepository.CreateMock<ISomeDependency>();
 
	someDependencyMock.Expect(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependencyMock);
        myController.DoSomething()
 
	someDependencyMock.VerifyAllExpectations();
}

I favour the former where we don't specify this information because I think it adds unnecessary noise to the name and is a detail about the implementation of the object behind that variable which I don't care about when I'm reading the test.

I do care about it if I want to change something but if that's the case then I can easily see that it's a mock or stub by looking at the place where it's instantiated.

From my experience we often tend to end up with the situation where the variable name suggests that something is a mock or stub and then it's used in a different way:

[Test]
public void ShouldDoSomething()
{
	var someDependencyMock = MockRepository.CreateMock<ISomeDependency>();
 
	someDependencyMock.Stub(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependencyMock);
        myController.DoSomething()
}

That then becomes a pretty misleading test because the reader is unsure whether the name is correct and the stub call is incorrect or whether it should in fact be a stub and the name is wrong.

The one time that I've seen that extra information being useful is when we have really long tests – perhaps when writing tests around legacy code which is tricky to get under test.

In this situation it is very nice to be able to easily see exactly what we're doing with each of our dependencies.

Hopefully this is only a temporary situation before we can work out how to write simpler tests which don't require this extra information.

Liz recently posted about mock objects and the original 'mock roles, not objects' paper and one thing that stood out for me is the idea that we should only mock types that we own.

I think this is quite an important guideline to follow otherwise we can end up in a world of pain.

One area which seems particularly vulnerable to this type of thing is when it comes to testing code which interacts with Hibernate.

A common pattern that I've noticed is to create a mock for the 'EntityManager' and then verify that certain methods on it were called when we persist or load an object for example.

There are a couple of reasons why doing this isn't a great idea:

1. We have no idea what the correct method calls are in the first place so we're just guessing based on looking through the Hibernate code and selecting the methods that we think make it work correctly.
2. If the library code gets changed then our tests break even though functionally the code might still work

The suggestion in the paper when confronted with this situation is to put a wrapper around the library and then presumably test that the correct methods were called on the wrapper.

Programmers should not write mocks for fixed types, such as those defined by the runtime or external libraries. Instead they should write thin wrappers to implement the application abstractions in terms of the underlying infrastructure. Those wrappers will have been defined as part of a need-driven test.

I've never actually used that approach but I've found that with Hibernate in particular it makes much more sense to write functional tests which verify the expected behaviour of using the library.

With other libraries which perhaps don't have side effects like Hibernate does those tests would be closer to unit tests but the goal is still to test the result that we get from using the library rather than being concerned with the way that the library achieves that result.

About a month ago or so Gary Bernhardt wrote a post showing how to get started with TDD and while the post is quite interesting, several comments on the post pointed out that he had jumped from iteratively solving the problem straight to the solution with his final step.

Something which I've noticed while solving algorithmic problems in couple of different functional programming languages is that the test driven approach doesn't work so well for these types of problems.

Dan North points out something similar in an OreDev presentation where he talks about writing a BDD framework in Clojure.

To paraphrase:

If you can't explain to me where this approach breaks down then you don't know it well enough. You're trying to sell a silver bullet.

The classic failure mode for iterative development is the big algorithm case. That's about dancing with the code and massaging it until all the test cases pass.

Uncle Bob also points this out while referring to the way we develop code around the UI:

There is a lot of coding that goes into a Velocity template. But to use TDD for those templates would be absurd. The problem is that I’m not at all sure what I want a page to look like. I need the freedom to fiddle around with the formatting and the structure until everything is just the way I want it. Trying to do that fiddling with TDD is futile. Once I have the page the way I like it, then I’ll write some tests that make sure the templates work as written.

I think the common theme is that TDD works pretty well when we have a rough idea of where we intend to go with the code but we just don't know the exact path yet. We can take small steps and incrementally work out exactly how we're going to get there.

When we don't really know how to solve the problem – which more often than not seems to be the case with algorithmic type problems – then at some stage we will take a big leap from being nowhere near a working solution to the working solution.

In those cases I think it still makes sense to have some automated tests both to act as regression to ensure we don't break the code and to tell us when we've written the algorithm correctly.

An example of a problem where TDD doesn't work that well is solving the traveling salesman problem.

In this case the solution to the problem is the implementation of an algorithm and it's pretty difficult to get there unless you actually know the algorithm.

During that dojo Julio actually spent some time working on the problem a different way – by implementing the algorithm directly – and he managed to get much further than we did.

It seems to me that perhaps this explains why although TDD is a useful design technique it's not the only one that we should look to use.

When we have worked out where we are driving a design then TDD can be quite a useful tool for working incrementally towards that but it's no substitute for taking the time to think about what exactly we're trying to solve.

I recently came across an interesting blog post by Rod Hilton on unit testing and it reminded me of a couple of conversations Phil, Raph and I were having about the best way to test classes which delegate some responsibility to another class.

An example that we ran into recently was where we wrote some code which required one controller to delegate to another.

public class ControllerOne extends Controller {
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
    }
}

public class ControllerTwo extends Controller {
	private final ControllerOne controllerOne;
 
	public ControllerTwo(ControllerOne controllerOne) {
		this.controllerOne = controllerOne;
	}
 
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
		....
		return controllerOne.handleRequest(...);
    }
}

My initial thought when working out how to test this code was that we should check that the request is actually getting routed via ControllerOne:

@Test
public void theTest() {
	ControllerOne controller One = mock(ControllerOne.class);
 
	ControllerTwo controllerTwo = new ControllerTwo(controllerOne);
 
	controllerTwo.handleRequest(...)
 
	verify(controllerOne).handleRequest(...);
}

When we discussed this Raph and Phil both pointed out that we didn't care that specifically about the implementation of how the request was handled. What we care about is that the result we get after the request is handled is as expected.

We therefore changed our test to be more like this:

@Test
public void theTest() {
	ControllerOne controller One = mock(ControllerOne.class);
	ModelAndView myModelAndView = new ModelAndView();
	when(controllerOne.handleRequest(...).thenReturn(myModelAndView);
 
	ControllerTwo controllerTwo = new ControllerTwo(controllerOne);
 
	ModelAndView actualModelAndView = controllerTwo.handleRequest(...)
 
	assertThat(actualModelAndView, equalTo(myModelAndView));
}

I've been finding more and more recently that when it comes to writing tests which do some sort of delegation the 'stub + assert' approach seems to work out better than just verifying.

You lose the fine grained test that verifying mocks provides but we can still pretty much tell indirectly whether the dependency was called because if it wasn't then it's unlikely (but of course still possible) that we would have received the correct 'ModelAndView' in our assertion.

My current approach is that I'd probably only mock and verify an interaction if the dependency is a service which makes a network call or similarly expensive call where the interaction is as important as the result obtained.

For example we probably wouldn't want to make that call multiple times and with verification we're able to ensure that doesn't happen.

I find as I've used mocking frameworks more I feel like I'm drifting from a mockist style of testing to one getting closer to the classicist approach. I wonder if that's quite a normal progression.