Mark Needham

Following on from my post around the definition of a unit test, a recent discussion on the Test Driven Development mailing list led me to question what my own approach is for writing unit tests.

To self quote from my previous post:

A well written unit test in my book should be simple to understand and run quickly.

Quite simple in theory but as I have learnt (and am still learning) the hard way, much harder to do in practice. Breaking that down further what does it actually mean?

Intention revealing name

There was some discussion a few months ago with regards to whether test names were actual valuable, but as the majority of my work has been in Java or C# I think it is very important.

I favour BDD style test names which describe the behaviour of what we are testing rather than the implementation details. For me naming the tests in this way allows people who look at the test in future to question whether it is a valid test as well as whether it is actually doing what it claims to be doing.

No clutter

If we can keep tests short and to the point they are much easier for the next person to read.

To achieve this we need to ensure that we keep the code in the test method to the minimum, including putting object setup code into another method so that it doesn’t clutter the test and only setting the expectations that we care about if we are using a mocking framework.

This is made much easier by the Arrange-Act-Assert approach being followed by mocking frameworks nowadays. I think this approach maps quite nicely to the Given-When-Then BDD syntax as a nice way of defining our tests or examples in BDD land.

Don’t remove all duplication

While removing duplication from code is generally a good thing I don’t think we should apply the DRY principle too judiciously on test code.

As Phil points out this can make tests very difficult to read and understand. I tend to favour test expressiveness over removing all duplication.

One behaviour per test

I used to try and follow the idea of having only one assertion per test but Sczcepan’s idea of testing one behaviour per class is much better.

This is one part of writing tests where we should stick to the Single Responsibility Principle in as far as not overloading the test with assertions which then make it more difficult to work out where the code failed if a test fails.

Expressive failure messages

When using JUnit or NUnit for assertions in the IDE the assertion failure messages don’t really make much difference because we have the code fresh in our mind and it’s only one click to get to the failure.

If an assertion with either of these frameworks fails on the build on the other hand it’s much harder at a glance to tell exactly why it failed. This is why I favour Hamcrest which tells you precisely why your test failed.

In Summary

For me the key with unit tests is to make sure that other people in the team can read and understand them easily.

No doubt there are other ways of ensuring our unit tests are well written but these are the ways that I consider the most important at the moment.

Chad Myers gets it spot on in his recent post about not testing private methods – private methods are private because they should be inaccessible from outside the class and their functionality should be tested via one of the public methods that calls them.

I’ve found that when a piece of code seems really difficult to test without exposing a private method then we’re probably trying to test that functionality from the wrong place.

I came across just this problem earlier this week when trying to test the functionality of one of our controllers – we are using the ASP.NET MVC framework to build our application.

My first approach was to try and test everything from the controller test but it quickly became apparent that this was going to be very difficult as the only data we had access to was the ActionResult which was insufficient for our testing purposes.

It became clear that if we put the logic into one of the objects being created inside the method (by a WCF web services call) that it would be significantly easier to test the logic – so that’s what we did! Suddenly it became much easier to test and we were able to come up with other test scenarios that we hadn’t previously considered.

The lesson to learn here is to constantly reflect on whether we are testing from the right place and if it feels too difficult then it probably is.

Another testing lesson I learnt recently was not to override protected methods in order to test other logic in the class. This is arguably even worse than exposing private methods because now we are not testing the production code but a variant of it.

The reason I did it was because all the logic for preventing caching of data was encapsulated into a single method and since I wasn’t interesting in testing that part of the code I thought it would be fine to just ignore it for those tests.

Nick pointed out that the actual problem was that we were missing an abstraction and as a result had ended up creating a dependency that made testing difficult. In this case the object was dependent on HttpResponse.

The lesson here is to ensure that objects depend on abstractions and not on concrete implementations wherever possible.

The underlying lesson in both cases being that we need to be constantly reflecting and questioning our approach to ensure that we write clean code and test that code in a clean way.

I’ve always been a big proponent of writing tests before writing code, and I roll off the standard reasons to people who question this approach:

• They help to drive the design
• They provide a safety net when making future changes
• They provide a way of communicating the intent of the code to the rest of the team

And so on. Despite knowing all this I recently took a non test driven approach to writing some bits of code – we were keen to get the system working end to end so it seemed a trade off worth making to prove that it was doable.

I knew that I would suffer when it came to testing this code after I had already written it and indeed it did!

I’ve tested around legacy code before and although Michael Feather’s book makes it much easier, it’s still not a great experience. The saving grace has always been that I didn’t write the code whereas in this case it was quite definitely my fault that I ended up with this problem.

The original pain came from not knowing where I should start. Since we didn’t have any tests I wasn’t sure exactly what the code was doing despite the fact that I had written most of it fairly recently.

In particular I ran into the problem whereby I wrote a test which passed so all seemed good. Suspicious as to whether this was the case I decided to comment out some of the code which was apparently making the code pass. I expected the test to fail and give me the red bar but in actual fact the test still passed!

It turned out that I had written the expectations for my mocks slightly wrong in the test – a mistake that I would have definitely caught if I had written the test first but could have easily missed by testing last.

After this I decided that because there wasn’t too much complex logic in this particular piece it would be much better to comment out the code and then test first before fitting it back together again. Eventually I got all the code covered but it took much longer than if I had just tested it properly the first time around.

For me software development is all about creating quick feedback loops and I find it very frustrating when I’m unable to do this. Testing first is one very good way of achieving this as it provides a clear way forward and gives feedback on our progress.

I will certainly be very reluctant to test last again.

One of the interesting discussions that has come up on several projects I’ve worked on is whether or not we should test code that was written purely to help us test production code.

One of the main arguments used against testing test utility code is that it is not production code and therefore perhaps doesn’t need to be held to the same standards because it lacks the complexity of production code.

Even ignoring this assumption, which I don’t agree with, I think we are still missing the point as to why we actually test in the first place.

Although a lot of the value of the test driven approach is that it helps us to drive design, the other side of the story, which we perhaps only fully appreciate after working with legacy code, is the safety net that having a suite of tests around our code provides.

From my experience the complexity of the test utility code is what determines whether it is valuable to test it or not. If it’s just builders to help create test data then I would agree there is little value in testing it, but if there’s actual logic and/or complexity then having a safety net is a good thing. Even if the tests are very simple they help protect us to ensure our code is still working when we make future changes.

The thing to watch for is that test utility code often starts off being very simple but slowly complexity creeps in making it more difficult to verify whether it actually does what we want it to.

As a rough marker, when you end up debugging through your test utility code to work out why it isn’t working as you expected it’s probably time to consider putting some tests around it.

Continuing with my mini Hibernate mappings series, this post talks about the different ways of setting up the test data for our Hibernate tests.

1. Where to test the mappings from?
2. How to test for equality?
3. How to setup the test data?

There are a couple of ways that we can setup data for Hibernate tests.

Insert Hibernate Object

This approach involves creating a new object and saving it to the database using the save method on the Hibernate session.

The benefit of this approach is that our test is pretty clean. It looks like any old unit test where we create an object and then check that what we get back is the same object.

The problem is that we are effectively testing two things – the ability to save and then load our Hibernate object. We will be able to tell whether or not our Hibernate mappings are correct using this approach, but the failures we get when they do fail may not be that obvious – it could just be a database exception which makes the test fail.

SQL Insertion

The other approach I have seen is to write manual JDBC calls to insert data into the various tables in our database and then check that we can load our object from the database using Hibernate.

The advantage of this is that our test is now only testing the mappings when we load the data from the database which helps reduce our test’s invariants or potential points of failure.

The disadvantage is that the tests can seem very brittle – if we make a small change to the column names in our tables then the test setup code may now fail to work anymore.

I’m not really completely happy with either of these approaches – neither seems optimal to me but both can help us achieve our objective. It’s just a matter of choosing which trade off we’ve prepared to accept.

I started a mini Hibernate series with my last post where I spoke of there being three main areas to think about when it comes to testing:

1. Where to test the mappings from?
2. How to test for equality?
3. How to setup the test data?

Once we have worked out where to test the mappings from, if we have decided to test them through either our repository tests or directly from the Hibernate session then we have some choices to make around how to test for equality.

I’ve seen this done in several ways:

Override equals

This was the first approach I saw and in a way it does make some sort of sense to test like this.

We don’t have to expose any of the internals of the class and we can get feedback on whether our objects have the same fields values or not. In addition we can normally get the IDE to generate the code for the equals method so it doesn’t require much extra effort on our behalf.

Typically an equality test along these lines would look something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

@Entity
public class Foo {
	@Column(name="BAR")
	private String bar;
 
	public Foo(String bar) {
		this.bar = bar;
	}
 
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
 
        Foo foo = (Foo) o;
 
        return !(bar != null ? !bar.equals(foo.bar) : foo.bar != null);
 
    }
}

1
2
3
4
5
6

import static org.hamcrest.MatcherAssert.assertThat;
...
Foo expectedFoo = new Foo("barValue");
Foo foo = getFooFromHibernate();
 
assertThat(foo, equalTo(expectedFoo));

The problem with this approach is that objects which are in Hibernate are likely to be entities and therefore their equality really depends on whether or not they have the same identity, not whether they have the same values. Therefore our equals method on the object should only compare the id value of the object to determine equality.

Implementing the equals method just for testing purposes may also be considered a code smell.

Getters

This approach involves adding getters to our objects to check that the values of each field have been set correctly.

While this approach is marginally better than not testing the mappings at all, the temptation to then use these getters in other pieces of the code can lead to us having objects with no behaviour at all with our logic spread all over the application.

1
2
3
4
5
6
7
8
9
10
11
12
13

@Entity
public class Foo {
	@Column(name="BAR")
	private String bar;
 
	public Foo(String bar) {
		this.bar = bar;
	}
 
	public getBar() {
		return this.bar;
	}
}

1
2
3
4
5
6

import static org.hamcrest.MatcherAssert.assertThat;
...
String bar = "barValue";
Foo foo = getFooFromHibernate();
 
assertThat(foo.getBar(), equalTo(bar));

Reflection

An approach I was introduced to recently involves using reflection to check that Hibernate has hydrated our objects correctly.

We initially rolled our own ‘Encapsulation Breaker’ to achieve this before realising that the OGNL library did exactly what we wanted to do.

By adding a custom Hamcrest matcher into the mix we end up with quite a nice test for verifying whether our mappings are working correctly.

1
2
3
4
5

@Entity
public class Foo {
	@Column(name="BAR")
	private String bar;
}

1
2
3
4

import static org.hamcrest.MatcherAssert.assertThat;
...
Foo foo = getFooFromHibernate();
assertThat(foo, hasMapping("bar", equalTo("someValue")));

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

public class HasMapping<T> extends BaseMatcher<T> {
    private String mapping;
    private Matcher<T> mappingValueMatcher;
 
    public HasMapping(String mapping, Matcher<T> mappingValueMatcher) {
        this.mapping = mapping;
        this.mappingValueMatcher = mappingValueMatcher;
    }
 
    public void describeTo(Description description) {
        description.appendText("A mapping from ");
        description.appendText(mapping);
        description.appendText(" that matches ");
        valueMatcher.describeTo( description );
    }
 
    @Factory
    public static <T> HasMapping hasMapping(String mapping, Matcher<T> mappingValueMatcher) {
        return new HasMapping(mapping, mappingValueMatcher);
    }
 
    public boolean matches(Object o) {
        try {
            Object value = OgnlWrapper.getValue(mapping, o);
            return mappingValueMatcher.matches(value);
        } catch (OgnlException e) {
            return false;
        }
    }
 
}

The drawback of this approach is that if we change the names of the fields on our objects we need to make a change to our test to reflect the new names.

I ran into the example dilemma a bit while writing this but hopefully the ideas have been expressed in the code presented. I didn’t want to put too much code in this post but if you’re interested in what the OgnlWrapper does I posted more about this on my post about Java checked exceptions.

I’ve had the opportunity to work with Hibernate and it’s .NET twin NHibernate on several of my projects and one of the more interesting decisions around its use is working out the best way to test the hibernate mappings that hook together our domain model and the database.

There are three decisions to make around how best to do this:

1. Where to test the mappings from?
2. How to test for equality?
3. How to setup the test data?

This post will focus on the ways I have seen with regards to choosing where to test the mappings from.

Functional Tests

This approach advocates only testing whether we have setup the mappings correctly when we run our acceptance or functional tests – we do not write tests specifically for testing hibernate mappings.

The benefit of this approach is that we are more likely to have acceptance tests in place, so this is just another thing that they can be used to catch.

While this approach is better than not testing at all, from my experiences the test feedback cycle is too slow – it takes too long to change one of the hibernate mappings and then run the test to check if it worked or not.

Repository Tests

With this approach we test whether our hibernate mappings are working as part of our repository tests.

The tricky thing with testing our hibernate mappings this way is that typically we only want to set up one object in the database and then test that Hibernate hydrates it correctly, but our repository doesn’t necessarily need a method for finding a single object.

We either end up adding on a method just for testing or we have to try and find our object from a list of other objects and then test it.

On the other hand, this approach does seem to work quite well when we have quite chatty repositories which provide a degree of flexibility around how we can retrieve our objects.

Direct Tests

This approach is my current favourite and involves loading the object directly from the Hibernate session and then testing it.

I was introduced to this idea by a colleague of mine and it seems to fit the idea of testing just one thing more closely than the other two approaches.

The strange thing about this approach is that we are testing directly with an API that is hidden from our system beyond the Repository.

In terms of simplicity with regards to testing hibernate mappings, however, this is the best approach I have seen.

—

I did a quick survey of some people last week and the most popular way of testing the mappings expressed was using Repository tests.

This post covers the other ways I have seen – are there any others people have come across or are using?

I’ve not used Selenium much in my time – all of my previous projects have been client side applications or service layers – but I’ve spent a bit of time getting acquainted with it this week.

While activating some acceptance tests this week I noticed quite a strange error happening if the tests ran in a certain order:

com.thoughtworks.selenium.SeleniumException: ERROR: Current window or frame is closed! 
at com.thoughtworks.selenium.HttpCommandProcessor.doCommand(HttpCommandProcessor.java:73)
at com.thoughtworks.selenium.HttpCommandProcessor.getString(HttpCommandProcessor.java:154)
at com.thoughtworks.selenium.HttpCommandProcessor.getBoolean(HttpCommandProcessor.java:227)
at com.thoughtworks.selenium.DefaultSelenium.isElementPresent(DefaultSelenium.java:394)

I tried commenting out some of the tests and then running the other ones and everything worked fine but when I ran all of them the problem returned.

Eventually after some eagle eyed debugging by a colleague of mine we realised that the only difference was the lack of the following line in the failing test:

selenium.selectWindow(null);

A quick look at the documentation explains precisely why this works:

if windowID is null, (or the string “null”) then it is assumed the user is referring to the original window instantiated by the browser).

What had in fact happened was that the previous test had launched a pop up window and when it closed that window the focus wouldn’t return to the original window launched when Selenium first started unless we executed the above method call.

I think that out of all the Extreme Programming practices Test Driven Development is the one which I like the best. I feel it provides a structure for development work and helps me to remain focused on what I am trying to achieve rather than writing code which may not necessarily be needed.

However, there are times when it’s difficult to use a TDD approach, and Pat Kua suggested earlier this year that if you’re using a TDD approach all the time you’re doing something wrong.

As Pat points out spiking is one time when it can pay off to not to test first, although as was pointed out on the TDD mailing list this doesn’t necessarily mean that you can’t take a test driven approach to learning new APIs or trying out new things.

Kent Beck speaks of Learning Tests – code written using tests to improve our understanding of an API and also guard against changes in future updates of the API – in Test Driven Development by Example, an idea which is referenced in Chapter 8 of Uncle Bob’s Clean Code. This is not a new approach.

Tools like the JUnit TestRunner provide a really easy way to try things out and get immediate feedback as to whether or not the API works as you expect. As Ben Hall writes on twitter it also provides a level of documentation which you can refer back to later.

Even if we don’t want to write an actual test the principles of getting quick feedback and working in small steps can still be used in our exploration activities.

To give a couple of examples, Damana and I didn’t write unit tests when we were exploring Ruby LDAP options but we were only writing a couple of lines at a time then running them using TextMate to see if our understanding was correct. We were then able to keep this code in a ‘spikes’ directory for future reference.

A couple of years ago a colleague and I were exploring (what was at the time) Tangosol Coherence’s API. We were using a method on the API to filter some data but for some reason it wasn’t returning the data that we expected.

Convinced that we were using the API correctly we decided to code up two JUnit tests – one with a call to the method which we felt had a bug in it, and another achieving the same ‘filter’ using two other methods on the API.

This helped us prove that there was a bug in the API and we ended up using the workaround we had discovered to solve our problem.

I’m sure there are other approaches that can achieve the same outcome but if you know how to test drive code then it makes sense to use an approach that is familiar to you.

A few of my colleagues recently posted their opinions about acceptance tests which tied in nicely with a discussion about acceptance testing that was had at the Alt.NET conference in London.

For the sake of argument I will assume that when we refer to acceptance tests we are talking about tests at the GUI level which are being automatically driven by a tool, usually Selenium but maybe something like White if it is a client side application. I’m not sure if this definition is 100% accurate but it feels to me that this is what is being referred to when we describe something as an acceptance test.

The discussion at the Alt.NET conference centered around the value of having these acceptance tests when they often take a long time to run therefore dragging out the time that it takes to build our application. These are some of the same problems that Sarah points out as well as issues around the maintenance of these tests.

Some of the responses to this pointed out that we should not rely too heavily on acceptance tests to confirm the correctness of our system especially when we can often do this using functional or integration tests which do not have to hit the UI and therefore run much more quickly.

We could still have acceptance tests but maybe they would only be necessary for critical scenarios where the system would become unusable if they did not pass – e.g. for the login screen.

I’m not sure what the solution is – I haven’t actually worked on a project which had the problems that Phillip and Sarah have experienced but I’ve heard the horror stories from speaking to some of my colleagues.

Sarah made one particular comment which I can relate to:

In fact, when I finish developing my story, the integration/persistence/unit tests already cover all acceptance criteria.

One of the projects I worked on had the concept of acceptance criteria for every story but we didn’t have acceptance tests so to speak. There were certainly unit tests and functional tests covering each story but that was the extent of our automated testing.

There were a couple of regression issues which we may not have had if we had acceptance tests but overall I felt the approach worked really well for us and the team consistently delivered. At one stage we did try to introduce some White driven GUI level tests but they were never introduced into the continuous integration process while I was on the project.

I appreciate that this is just one scenario and that each project is different but the lessons I took from this was that we should just do what works best in a given situation and not be too dogmatic in our approach.

Phillip actually has a different view and (to probably quote him out of context) believes that having automated acceptance tests is vital:

My main concern is that I think people value Acceptance Testing too much. Don’t get me wrong: automated acceptance tests are essential to a healthy agile process, the problem is what we do with them after we deliver a story.

I do agree that we often place too much emphasis on acceptance testing but I’m not convinced that we need automated acceptance tests to have a ‘healthy agile process’.

As I’ve been writing this the terminology seems a bit strange to me. Is it actually correct to say that an ‘acceptance test’ always implies an automated test that is run from the GUI?

Or can an ‘acceptance test’ be a collection of functional and integration tests which confirm that a piece of business functionality works as expected?