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!

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.

I've written before about my favoured approach of writing tests in such a way that they have clear 'Given/When/Then' sections and something which I come across quite frequently is tests where the latter steps have been combined into one method call which takes care of both of these.

An example of this which I came across recently was roughly like this:

@Test
public void shouldCalculatePercentageDifferences() {
	verifyPercentage(50, 100, 100);
	verifyPercentage(100, 100, 0);
	verifyPercentage(100, 50, -50);
}

private void verifyPercentage(int originalValue, int newValue, int expectedValue) {
	assertEquals(expectedValue, new PercentageCalculator().calculatePercentage(originalValue, newValue));
}

This code is certainly adhering to the DRY principle although it took us quite a while to work out what the different numbers being passed into 'verifyPercentage' were supposed to represent.

With this type of test I think it makes more sense to have a bit of duplication to make it easier for us to understand the test.

We changed this test to have its assertions inline and make use of the Hamcrest library to do those assertions:

@Test
public void shouldCalculatePercentageDifferences() {
	assertThat(new PercentageCalculator().calculatePercentage(50, 100), is(100));
	assertThat(new PercentageCalculator().calculatePercentage(100, 100), is(0));
	assertThat(new PercentageCalculator().calculatePercentage(100, 50), is(-50));
}

I think we may have also created a field to instantiate 'PercentageCalculator' so that we didn't have to instantiate that three times.

Although we end up writing more code than in the first example I don't think it's a problem because it's now easier to understand and we'll be able to resolve any failures more quickly than we were able to previously.

As Michael Feathers points out during Jay Fields' 'Beta Test' presentation we need to remember why we try and adhere to the DRY principle in the first place.

To paraphrase his comments:

In production code if we don't adhere to the DRY principle then we might make a change to a piece of code and we won't know if there's another place where we need to make a change as well.

In test code the tests always tell us where we need to make changes because the tests will break.

We recently changed the way that we test against our main integration point on the project I've been working on so that in our tests we retrieve the service object from our dependency injection container instead of 'newing' one up.

Our tests therefore went from looking like this:

[Test]
public void ShouldTestSomeService()
{
	var someService = new SomeService();
	// and so on
}

To something more like this:

[Test]
public void ShouldTestSomeService()
{
	var someService = UnityFactory.Container.Resolve<ISomeService>();
	// and so on
}

This actually happened as a side effect of another change we made to inject users into our system via our dependency injection container.

We have some 'authenticated services' which require the request to contain a SAML token for a valid user so it seemed to make sense to use the container in the tests instead of having to duplicate this piece of behaviour for every test.

We needed to add our fake authorised user into the container for our tests but apart from this the container being used is the same as the one being used in the production code.

Our tests are therefore now calling the services in a way which is much closer to the way that they are called in the code than was previously the case.

I think this is good as it was previously possible to have the tests working but then have a problem calling the services in production because something in the container wasn't configured properly.

The downside is that these tests now have more room for failure than they did previously and they are not just testing the end point which was their original purpose.

In a way what we have done is convert these tests from being contract tests to integration tests.

I like the new way but I'm not completely convinced that it's a better approach.

My colleague Sarah Taraporewalla recently wrote about her thoughts on test doubles (to use Gerard Meszaros' language) and it got me thinking about the approach I generally take in this area.

Stub objects

I use stubs mostly to control the output of depended on components of the system under test where we don't want to verify those outputs.

Most of the time I make use of the mocking library's ability to stub out method calls on these dependencies.

I find that it generally seems to require less effort to do this than to create hand written stubs although chatting to Dave about this he pointed out that one situation where it would make more sense to use a hand written stub is when stubbing out a clock/time provider. This is because there are likely to be multiple calls to it all over the place and most of the time you probably want it to return the same value anyway.

I actually quite like the fact that you need to specify all the stub calls that you want to make in each test – it helps you to see when you have too many dependencies and then hopefully you can do something about that.

On previous projects I worked on we decided the way to get around that problem was to define all the stub method calls in a setup method but that seems to lead to a world of pain later on when you forget that you've stubbed a method in the setup and now want to assert an expectation on it or (to a lesser extent) write a test which doesn't actually use the stub.

Fake objects

I often confuse fakes with stubs as seem to be quite similar to each other in their intent – the difference as I understand it is that with a stub we are controlling the output of a dependency whereas a fake just sits there and lets interactions happen with it. The values passed in earlier calls to the fake may be returned in later calls to it.

The most common use of this pattern is to replace a real database with a fake one for testing although on a recent project we were making use of a hand written fake session store to avoid having to refer to the real session in our test code.

We might have one call to the 'SessionFake' to store a value and then if a retrieve call is made later on we would return the value that we previously stored.

The approach Sarah describes for stubbing repositories seems quite similar to this as well.

Mock objects

I use mocks to replace depended on components of the system under test when I do care about the way that is is used i.e. we want to verify the behaviour of the dependencies.

If we see a mock object being created in a test then we should see a call to a 'verify' method later on to ensure that the expected methods are called on it.

I used to use these all over the place for just about every test where I wanted to control the way that a dependency acted until I realised how fragile and confusing that made the tests.

Now, after recently watching a presentation by Jay Fields, I try to ensure that I'm only setting up one expectation per test and use of the other test double approaches for any other dependencies that needs to be taken care of in that test.

Dummy objects

Most of the time when I pass dummy values into tests they tend to be strings and I prefer to pass in a value of 'irrelevantValue' rather than just passing in a null which may lead to difficult to locate Null Pointer Exceptions further down the line if the value which we thought was just a dummy starts being used.

We are generally only passing in these dummy values to satisfy the requirements of the system under test which may require values to be entered even if the particular piece of functionality that we are testing doesn't make use of them.

Overall

I think my current approach to testing leans more towards mockist rather than classicist although I think I am probably moving more towards the middle as I see the problems we can run into with over mocking.

With test doubles my current approach has minimising the effort required to create them as the most important aspect but I'm sure that will change given a different context. With all the test doubles I generally try and use test data builders where it's not overkill.

Our technical book club this week focused on 'The Readability of Tests' chapter from Steve Freeman & Nat Pryce's upcoming book 'Growing Object Oriented Software, guide by tests'.

I've been reading through some of the other chapters online and I thought this would be an interesting chapter to talk about as people seem to have different opinions on how DRY tests should be, how we build test data, how we name tests and so on.

These were some of my thoughts and our discussion on the chapter:

• I found it interesting that there wasn't any mention of the BDD style of test naming whereby the name of the test begins with 'should…'. I've been using these style of naming for about 2 years now as I find it useful for allowing us to question whether or not the test is valid. There are equally arguments against using the word 'should' as it's not particularly assertive and perhaps we ought to be more certain about what our tests are asserting.

  Recently I have started to move more towards Jay Fields idea that test names are just comments and if we write tests to be really clear and readable then the test name becomes redundant.

• The chapter talks about the order in which the authors write their tests, the approach being to try and start with the assertion first and then write the execution and setup steps. My current approach is to write the execution step first and then build up the setup and expectations almost simultaneously. I've never been able to quite get the hang of writing the test bottom up but it's something I might experiment with again.
• Refactoring tests is something I've written about previously and my current thinking is that our aim shouldn't be to remove absolutely all duplication in tests but instead remove it to a stage where we can still easily understand the test when it fails. This seems to fit in with the authors' idea of 'refactoring but not too hard'.

  I am currently following the idea of having three distinct areas in my tests (Given, When, Then) with each section separated by an empty line. I find writing them in this style makes it easier for me to quickly work out why a test is failing.

  I was recently watching Jay Fields' presentation from SpeakerConf and Michael Feathers makes an interesting comment that we need to keep in mind that the reason for removing duplication in code is so that when we need to make changes we know where to do that. In test code the test failing will tell us where we need to make changes so the need to remove duplication to do this is less.

  I'm still heavily in favour of trading duplication for better readability when it comes to writing tests.

• The idea of keeping consistency in tests is an important one although I think it's difficult to keep this consistency across the whole suite of tests. Certainly within a single test fixture it should be possible though.

  One example of something which doesn't follow this approach is the 'ExpectedException' annotation in JUnit/NUnit which goes against the style of pretty much all other tests.

• When it comes to setting up tests data I think it's pretty much given that test data builders are a really good way to help remove noise and duplication from our tests. Other patterns such as object mother can be useful but it doesn't seem to work as well when you have multiple different was that you want to setup your data for tests.
• There's no specific mention of 'Setup' and 'Teardown' methods in the chapter but this is another area which I think has an important impact on readability.

  I'm not yet completely against tear down methods for integration style tests but I've seen a lot of pain causes by putting mocks in setup methods and even just having the setup method means that you have to go up and down the test fixture just to work out what's going on. I prefer to try and keep all the context needed for a test in one place .

• I found the section about the way that we name literals/variables in tests to be particularly interesting as this is a discussion I've been having with a couple of colleagues recently.

  I find it useful to state why that variable is important or not important for this particular test (i.e. give it context) so that someone can easily understand what's going on when they look at the test. For example if we have a variable in a test that doesn't affect the outcome then it might be useful to name it 'stubFoo' or 'irrelevantFoo' or something similar.

  I've previously been against the idea of naming dependencies we're mocking/stubbing as 'mockRepository' or 'stubRepository' but I've been trying this out a bit this week and it exposed some mistakes I'd made which I don't think I would have seen otherwise.

• Another idea which I quite liked is the idea of only testing one feature set per test.

  I've certainly written a lot of tests which break this rule and you really suffer when you need to make a change later on.

  Jay Fields also applies this rule to mocks whereby you can only have one expectation per test but as many stubs as you want.

  I've been trying that out both these approaches this week and although there's probably more code overall as a result of writing more tests, each of the tests feels much more succinct and understandable.

I recently came across Pavel Brodzinski's blog and while browsing through some of his most recent posts I came across one discussing when unit testing doesn't work.

The majority of what Pavel says I've seen happen before on projects I've worked on but I disagree with his suggestion that writing unit tests is boring:

6. Writing unit tests is boring. That’s not amusing or challenging algorithmic problem. That’s not cool hacking trick which you can show off with in front of your geeky friends. That’s not a new technology which gets a lot of buzz. It’s boring. People don’t like boring things. People tend to skip them.

I think it depends on the way that the unit tests are being written.

When I first started working at ThoughtWorks I used to think that writing tests was boring and that it was much more fun writing production code. A couple of years have gone by since then and I think I actually get more enjoyment out of writing tests these days.

There are some things we've done on teams I've worked on which contribute to my enjoyment when writing unit tests:

Small steps

While working on a little application to parse some log files last week I had to implement an algorithm to find the the closing tag of an xml element in a stream of text.

I had a bit of an idea of how to do that but coming up with little examples to drive out the algorithm helped me a lot as I find it very difficult to keep large problems in my head.

The key with following the small steps approach is to only writing one test at a time as that helps keep you focused on just that one use of this class which I find much easier than considering all the cases at the same time.

The feeling of progress all the time, however small, contributes to my enjoyment of using this approach.

Test first

I think a lot of the enjoyment comes from writing unit tests before writing code, TDD style.

The process of moving up and down the code as we discover different objects that should be created and different places where functionality should be written means that writing our tests/examples first is a much more enjoyable process than writing them afterwards.

The additional enjoyment in this process comes from the fact that we often discover scenarios of code use and problems that we probably wouldn't have come across if we hadn't driven our code that way.

Ping pong pairing

I think this is the most fun variation of pair programming that I've experienced, the basic idea being that one person writes a test, the other writes the code and then the next test before the first person writes the code for that test.

I like it to become a bit of a game whereby when it's your turn to write the code you write just the minimal amount of code possible to make the test pass before driving out a proper implementation with the next test you write.

I think this makes the whole process much more light hearted than it can be otherwise.

In Summary

The underlying premise of what makes writing unit tests pretty much seems to be about driving our code through those unit tests and preferably while working with someone else.

Even if we choose not to unit test because we find it boring we're still going to test the code whether or not we do it in an automated way!

I recently read a blog post by Joshua Lockwood where he spoke of some people who claim they don't have time to test.

Learning the TDD approach to writing code has been one of best things that I've learnt over the last few years – before I worked at ThoughtWorks I didn't know how to do it and the only way I could verify whether something worked was to load up the application and manually check it.

It was severely painful and on one particular occasion I managed to put some code with a bug into production because I didn't know all the places that making that code change would impact.

It's not a good way of working and I'm glad I've been given the opportunity to work with people who have showed me a better way.

My experience pretty much matches a comment made by Chris Missal on the post where he pointed out that you are going to test your code anyway so you might as well automate that test!

"You're already testing with the debugger, TestPage1.aspx, or whatever… Just save that code and automate it!"

I've just spent the last 2 hours doing some refactoring on an F# twitter application I'm working on and because I didn't write any tests it's been a very painful experience indeed.

Every time I make a change I have to copy all the code into F# interactive, run the code and then manually make sure that I haven't broken anything.

I've been doing this in fairly small steps – make one change then run it – but the cycle time is still much greater than it would be if I had just put some tests around the code in the first place.

I think we should be looking to test more than just the 'complex code' as well – there have been numerous occasions when I've put the logic for a conditional statement the wrong way around and a test has come to the rescue.

It pretty much applies to all the languages that I've worked in and if we can't see how to easily create an automated test for a bit of code then it's a sign that we're doing something wrong and we might want to take a look at that!

I wrote previously about creating DRY tests and after some conversations with my colleagues recently about the balance between reducing duplication but maintaining readability I think I've found the compromise between the two that works best for me.

The underlying idea is that in any unit test I want to be aiming for a distinct 3 sections in the test – Given/When/Then, Arrange/Act/Assert or whatever your favourite description for those is.

Why?

I find that tests written like this are the easiest for me to understand – there would typically be a blank line between each distinct section so that scanning through the test it is easy to understand what is going on and I can zoom in more easily on the bit which concerns me at the time.

When there's expectations on mocks involved in the test then we might end up with the meat of the 'Then' step being defined before the 'When' section but for other tests it should be possible to keep to the structure.

A lot of the testing I've been working on recently has been around mapping data between objects – there's not that much logic going on but it's still important to have some sort of verification that we have mapped everything that we need to.

We often end up with a couple of tests which might look something like this:

public void ShouldEnsureThatFemaleCustomerIsMappedCorrectly()
{
	var customer = new Customer() 
					{
						Gender = Gender.Female
						Address = new Address(...)
					}
 
	var customerMessage = new CustomerMapper().MapFrom(customer)
 
	Assert.AreEqual(CustomerMessage.Gender.Female, customerMessage.Gender);
	Assert.AreEqual(new Address(..), customerMessage.Address);
	// and so on...
}
 
public void ShouldEnsureThatMaleCustomerIsMappedCorrectly()
{
	var customer = new Customer() 
				{
					Gender = Gender.Male
					Address = new Address(...)
				}
 
	var customerMessage = new CustomerMapper().MapFrom(customer)
 
	Assert.AreEqual(CustomerMessage.Gender.Male, customerMessage.Gender);
	Assert.AreEqual(new Address(..), customerMessage.Address);
	// and so on...
}

(For the sake of this example 'CustomerMessage' is being auto generated from an xsd)

We've got a bit of duplication here – it's not that bad but if there are changes to the CustomerMessage class, for example, we have more than one place to change.

It is actually possible to refactor this so that we encapsulate nearly everything in the test, but I've never found a clean way to do this so that you can still understand the intent of the test.

public void ShouldEnsureThatFemaleCustomerIsMappedCorrectly()
{
	AssertCustomerDetailsAreMappedCorrectly(customer, Gender.Female, CustomerMessage.Gender.Female);
}
 
public void ShouldEnsureThatMaleCustomerIsMappedCorrectly()
{				
	AssertCustomerDetailsAreMappedCorrectly(customer, Gender.Male, CustomerMessage.Gender.Male);
}
 
private void AssertCustomerDetailsAreMappedCorrectly(Customer customer, Gender gender, CustomerMessage.Gender gender)
{			
	var customer = new Customer() 
					{				
						Gender = gender,
						Address = new Address(...)
					}
 
	var customerMessage = new CustomerMapper().MapFrom(customer)
 
	Assert.AreEqual(CustomerMessage.Gender.Male, customerMessage.Gender);
	// and so on...	
}

(Of course we would be mapping more than just gender normally but gender helps illustrate the pattern that I've noticed)

We've achieved our goal of reducing duplication but it's not immediately obvious what we're testing because that's encapsulated too. I find with this approach that it's more difficult to work out what went wrong when the test stops working, so I prefer to refactor to somewhere in between the two extremes.

public void ShouldEnsureThatFemaleCustomerIsMappedCorrectly()
{
	var customer = CreateCustomer(Gender.Female, new Address(...));
 
	var customerMessage = MapCustomerToCustomerMessage(customer);
 
	AssertFemaleCustomerDetailsAreMappedCorrectly(customer, customerMessage);
}
 
public void ShouldEnsureThatMaleCustomerIsMappedCorrectly()
{
	var customer = CreateCustomer(Gender.Male, new Address(...));
 
	var customerMessage = MapCustomerToCustomerMessage(customer);
 
	AssertMaleCustomerDetailsAreMappedCorrectly(customer, customerMessage);
}
 
private CustomerMessage MapCustomerToCustomerMessage(Customer customer)
{
	return new CustomerMapper().MapFrom(customer);
}
 
private Customer CreateCustomer(Gender gender, Address address)
{
	return new Customer() 
				{
					Gender = gender,
					Address = address
				};
}
 
private void AssertMaleCustomerDetailsAreMappedCorrectly(Customer customer, CustomerMessage customerMessage)
{			
	Assert.AreEqual(CustomerMessage.Gender.Male, customerMessage.Gender);
	// and so on...	
}
 
private void AssertFemaleCustomerDetailsAreMappedCorrectly(Customer customer, CustomerMessage customerMessage)
{			
	Assert.AreEqual(CustomerMessage.Gender.Female, customerMessage.Gender);
	// and so on...	
}

Although this results in more code than the 1st approach I like it because there's a clear three part description of what is going on which will make it easier for me to work out which bit is going wrong. I've also split the assertions for Male and Female because I think it makes the test easier to read.

I'm not actually sure whether we need to put the 2nd step into its own method or not – it's an idea I've been experimenting with lately.

I'm open to different ideas on this – until recently I was quite against the idea of encapsulating all the assertion statements in one method but a few conversations with Fabio have led me to trying it out and I think it does help reduce some duplication without hurting our ability to debug a test when it fails.

I had a discussion recently with Fabio about DRYness in our tests and how we don't tend to adhere to this principal as often in test code as in production code.

I think certainly some of the reason for this is that we don't take as much care of our test code as we do production code but for me at least some of it is down to the fact that if we make our tests too DRY then they become very difficult to read and perhaps more importantly, very difficult to debug when there is a failure.

There seem to be different types of DRYness that weave themselves into our test code which result in our code becoming more difficult to read.

Suboptimal DRYness

Setup method

Putting code into a setup method is the most common way to reduce duplication in our tests but I don't think this is necessarily the best way to do it.

The problem is that we end up increasing the context required to understand what a test does such that the reader needs to read/scroll around the test class a lot more to work out what is going on. This problem becomes especially obvious when we put mock expectations into our setup method.

One of those expectations becomes unnecessary in one of our tests and not only is it not obvious why the test has failed but we also have a bit of a refactoring job to move the expectations out and only into the tests that rely on them.

Helper methods with more than one responsibility

Extracting repeated code into helper methods is good practice but going too far and putting too much code into these methods defeats the purpose.

One of the most common ways that this is violated is when we have methods which create the object under test but also define some expectations on that object's dependencies in the same method.

This violates the idea of having intention revealing method names as well as making it difficult to identify the reason for test failures when they happen.

Assertions

I tend to follow the Arrange, Act, Assert approach to designing tests whereby the last section of the test asserts whether or not the code under test acted as expected.

I'm not yet convinced that following the DRY approach is beneficial here because it means that you need to do more work to understand why a test is failing.

On the other hand if assertions are pulled out into an intention revealing method then the gain in readability might level out the extra time it takes to click through to a failing assertion.

My favourite approach to test assertions is to use behavioral style assertions

e.g.

stringValue.ShouldEqual("someString")

…and I don't think applying the DRY principle here, if we have a lot of similar assertions, adds a lot of value.

DRY and expressive

I'm not against DRYness in tests, I think it's a good thing as long as we go about it in a way that still keeps the code expressive.

Test data setup

The setup and use of test data is certainly an area where we don't gain an awful lot by having duplication in our tests. If anything having duplication merely leads to clutter and doesn't make the tests any easier to read.

I have found the builder pattern to be very useful for creating clutter free test data where you only specifically define the data that you care about for your test and default the rest.

Single responsibility helper methods

If we decide that extracting code into a helper method increases the readability of a test then the key for is to ensure that these helper methods only do one thing otherwise it becomes much more difficult to understand what's going on.

My current thinking is that we should aim for having only one statement per method where possible so that we can skim through these helper methods quickly without having to spend too much time working out what's going on.

An idea Dan North talks about (and which is nicely illustrated in this blog post) is putting these helper methods just before the test which makes use of them. I haven't tried this out yet but it seems like a neat way of making the code more DRY and more readable.

In Summary

I've noticed recently that I don't tend to read test names as often as I used to so I'm looking to the test code to be expressive enough that I can quickly understand what is going on just from scanning the test.

Keeping the code as simple as possible, extracting method when it makes sense and removing clutter are some useful steps on the way to achieving this.