TDD: Keeping test intent when using test builders
While the test data builder pattern is quite a useful one for simplifying the creation of test data in our tests I think we need to be quite careful when using it that we don't lose the intent of the test that we're writing.
The main advantage that I see with this pattern is that by using it we can provide default values for properties of our objects which aren't important for the bit of functionality that we're currently testing but which need to be provided otherwise the test can't actually be run.
In order for our tests to express their intent clearly it is still important to explicitly set the values that we care about for the current test otherwise it will become quite difficult to work out why a test is failing when it does.
For example I recently came across a failing test similar to this:
[Test] public void ShouldTestSomeCoolStuff() { var foo = new FooBuilder().Build(); var yetAnotherObject = someOtherObject.ThatTakesIn(foo); Assert.AreEqual("DEFAULTBAR", yetAnotherObject.Bar); Assert.AreEqual("DEFAULTBAZ", yetAnotherObject.Baz) }
It was failing because the actual values being returned were 'defaultBar' and 'defaultBaz' which initially made us wonder if there was some capitalisation going wrong in one of our objects.
As it turned out the values being returned were just the default ones from the 'FooBuilder' and we were asserting the wrong thing:
public class FooBuilder { private string bar = "defaultBar"; private string baz = "defaultBaz"; public FooBuilder Bar(string value) { bar = value; return this; } public FooBuilder Baz(string value) { baz = value; return this; } public Foo Build() { return new Foo(bar, baz); } }
While it didn't take us too long to get to the cause of the problem I think it would have made our lives easier if we'd been able to tell why it was failing just from looking at the test instead of having to look in a couple of different classes to figure it out.
A test written more like this would help us to achieve that:
[Test] public void ShouldTestSomeCoolStuff() { var foo = new FooBuilder().Bar("myBar").Baz("myBaz").Build(); var yetAnotherObject = someOtherObject.ThatTakesIn(foo); Assert.AreEqual("myBar", yetAnotherObject.Bar); Assert.AreEqual("myBaz", yetAnotherObject.Baz) }
Then if we want to remove duplication we can just put the expected values into more descriptive variables:
[Test] public void ShouldTestSomeCoolStuff() { var expectedBar = "myBar"; var expectedBaz = "myBaz"; var foo = new FooBuilder().Bar(expectedBar).Baz(expectedBaz).Build(); var yetAnotherObject = someOtherObject.ThatTakesIn(foo); Assert.AreEqual(expectedBar, yetAnotherObject.Bar); Assert.AreEqual(expectedBaz, yetAnotherObject.Baz) }
There's a bit more test code than in the first example but if the test fails again we should be able to work out why more quickly than before because we are now more explicit about which values are actually important for this test.
Hi Mark
I don't know why you take the last step. You're not alone — for some reason everyone feels the need to pull out local variables for duplicated string constants. But I think it harms rather than improves the readability of the test, and that should be the test, rather than blindly trying to minimize duplication.
I think that deep down inside you feel the same way, which is why you say "if we want to remove duplication" rather than just going ahead and doing it. Have the courage of your convictions — shake off the shackles of reflexive deduplication.
Ben Butler-Cole
22 Sep 09 at 5:34 am
Hi Ben,
That's a really interesting observation actually and it always plays out in my mind the same way it has in this example – I don't remove the duplication until I get a nagging feeling that I should be doing it right at the end.
And then there's 2 extra lines at the top of the test which don't really tell us anything other than what we can already tell from glancing at the test – thanks for pointing that out though!
I wonder when it's important to actually extract local variables though – my current thinking (for non strings) is that if I care about that bit of data for the test I want to try and extract the value into a more descriptive name but if not then I'm happy to leave it inline.
I'd be interested in knowing your thoughts.
Mark
Mark Needham
22 Sep 09 at 7:45 am
[...] method under test. Besides being difficult to read (though we experienced some improvement by using test builders), those objects evidently had a lot of duplication with the data in the XML [...]
Test Automation for the Persistence Layer with FIT, DBUnit and HSQLDB – MundoJava 38 « Alexandre Gazola
14 Nov 09 at 12:32 pm