Insulating Tests from Interface Changes

I've been reading Robert Martin's Clean Code book recently and just finished the testing section yesterday. I've been TDDing for a few years now and am a big fan of using libraries that help make tests more readable, for example hamcrest matchers but I wasn't sure if the arguments put forward in the Clean Code book for building up a DSL for testing sounded like a step too far. I thought I'd give it a go today. What I found was that not only does it make the tests easier to read, it also makes them much faster to write and far simpler to handle changes to the code they are exercising. Building up a test DSL insulates your tests from changes in the production code.

Consider the following interface:

List fetchRecentEventsForPlace( EventsForPlaceSearch searchData );

Here's the old style test that I started the day with:

@Test
public void recentEventsCanBeLimitedByArrival_old() {
  Plan missedArrival = planFactory.missedArrival(FIRST_JAN_09);
  Plan missedDeparture = planFactory.missedDeparture(FIRST_JAN_09);
  persist(missedArrival, missedDeparture);

  EventsForPlaceSearch search = new EventsForPlaceSearch(place)
      .withEventTypes(EventType.ARRIVAL)
      .withPlanned()
      .withUnplanned();
  List results = planRepository.fetchRecentEventsForPlace(search);

  assertThat(results.size(), is(1));
  resultsHaveEvents(results, missedArrival.getEvents());
  resultsDoNotHaveEvents(results, missedDeparture.getEvents());
}

As you can see, I had already started building utility classes to help construct test data, making the tests easier to read and write. But when it comes to executing the code I want to test, I am calling the production code directly in the test:

EventsForPlaceSearch search = new EventsForPlaceSearch(place)
  .withEventTypes(EventType.ARRIVAL)
  .withPlanned()
  .withUnplanned();
List results = planRepository.fetchRecentEventsForPlace(search);

The next example shows the refactored test, using more literate function calls to construct the EventsForPlaceSearch object and perform the search:

@Test
public void recentEventsCanBeLimitedByArrival() {
  Plan missedArrival = planFactory.missedArrival(FIRST_JAN_09);
  Plan missedDeparture = planFactory.missedDeparture(FIRST_JAN_09);
  persist(missedArrival, missedDeparture);

  List results = recent(arrivals());

  assertThat(results.size(), is(1));
  resultsHaveEvents(results, missedArrival.getEvents());
  resultsDoNotHaveEvents(results, missedDeparture.getEvents());
}

private List recent( EventsForPlaceSearch search ) {
  return planRepository.fetchRecentEventsForPlace(search);
}

The immediate benefit of this change is the literacy of the test, you can quite easily read that the test is fetching all recent arrivals. The second benefit that you notice when adding subsequent tests is that suddenly everything is faster to write:

recent(arrivals());
recent(departures());
recent(plannedDepartures());

As if this wasn't enough, the other benefit is that the tests are insulated from changes to the production code. This did not occur to me until I actually had to change the interface of the fetchRecentEventsForPlace function while developing it. When I came to integrate the first cut of the function into the rest of the application I realised that most places it was used would require a slightly different data structure. Rather than forcing the application to perform the conversion it made more sense to change the interface of the repository to be aligned with the requirements of the client. I ended up changing the return type from List to SearchResults:

SearchResults fetchRecentEventsForPlace( EventsForPlaceSearch searchData );

Now normally a change like this on some well tested code is a pain, because you have to go to all the tests and change the expected return type and then change the expectations of the test to work with the new return type. This time however there was only one place to change, the method that provides my test DSL. To accommodate the change I simply modified the one function like so:

private List recent( EventsForPlaceSearch search ) {
  return planRepository.fetchRecentEventsForPlace(search).getRows();
}

I did not have to change one line of my test methods and all my tests are still valid and pass. This is the way it should be. Thank you Uncle Bob!