Blog

Do you have 15 minutes to spare?

Sure, what can I do for you?

Well, you know I’ve been working on that new feature we were discussing at the whiteboard last week. I’ve finished the implementation of that user story. All the automated tests are green, the CI build has completed successfully, and I’ve deployed the latest version on the test environment where Luke and I already did some exploratory testing.

Sounds good.

So I was wondering whether we could do a code review together? I’ll walk you through the code, if you have some time to spare off course.

Sure thing. Show me the code!

During our design session we talked about integrating a third-party REST service for retrieving quote prices.

Yes, I remember that discussion.

So, as we agreed back then, I’ve put all the code for calling this REST service in an adapter class.

Perfect! I also see that you’ve defined and exposed your own custom data objects.

Yes indeed. I didn’t want to expose the data objects of the REST service itself. When the API of the REST service gets changed by the host, we only have to modify the implementation of the adapter class. By using separate data objects for the interface of the adapter, we prevent that those kinds of third-party API changes ripple through the rest of the system.

Well done.

Let me quickly show you the unit tests for the adapter.

Interesting …

These are the test scenarios that cover every path through the code of the adapter. I did spend quite some time figuring out how the REST client library that we’re using is actually implemented.

How so? I heard from Sarah that she used this same library a couple of months ago. She mentioned how easy it was to use this REST client library.

She’s right! It’s definitely quite easy to use. This part of the code in the adapter class demonstrates this. You can see that it only requires just a few lines of code. However, I did have a lot of difficulties to get the proper stubs in place in order to get the unit tests running green.

Can you show me the code for this?

Sure. I’ve refactored all this nasty setup code into a separate method inside the test fixture class. This method is then called by all the test methods for setting up the correct stub values.

The implementation of that method sure looks very complicated. I also notice that you’ve used a lot of test doubles.

Yes, *sigh*. I did have to create a lot of test doubles. You see, the API of the REST client library defines a domain-specific language. That meant that I had to set up a test double that returns other test doubles for every method being used. This was needed in order to avoid null reference exceptions, and also being able to return a list of quote prices.

That looks quite problematic, don’t you think?

I agree that it did require quite some effort. However, why do you consider this to be problematic? Everything went smooth once I figured out how to set up all these test doubles to work with the DSL. Also, the tests execute without too much hassle.

I’m quite sure these unit tests are running just fine. However, can I ask you this? After you’ve cobbled together the implementation of the adapter, did you have to make any changes when you first tested this implementation against the real REST service?

Well yes, quite a few actually. I did have to specify some additional headers and a couple of other things related to authentication.

You also needed to make the necessary changes to the unit tests then?

Yes, I did. At first there weren’t that many tests doubles compared to what I’ve ended up with.

I’m willing to believe that’s true. Here’s the thing. Suppose I’d tell you that this REST client library is no longer being maintained anymore and that it needs to be replaced with another library. What would be the implications for the code of the adapter class?

Well, then we obviously need to replace these couple lines of DSL code with something else. That shouldn’t be that difficult.

What would happen to the unit tests of the adapter?

Well I guess that these tests need to be completely rewritten from scratch.

Bingo!

Now I’m a bit confused. Why should that be a problem? In your hypothetical case we’ve switched to another REST client library after all, right? Doesn’t that automatically imply that these unit tests should be changed as well?

These unit tests in their current form need to be changed indeed. However, the actual reason for this change is that they’re too tightly coupled to the code structure of the adapter class. You see, using a REST client library is an implementation detail of the adapter class. It shouldn’t matter to the tests which library is being used and how it’s API is shaped. When we decide to replace this REST client library with another one, then the tests shouldn’t be modified at all. You know why?

Why?

Because they should tell us whether the behaviour of the adapter is still exactly the same as before we swapped out the REST client library. We need these tests as a fall back in order to verify whether the adapter still works as it’s supposed to.

But then they’re no longer unit tests. Do you want to replace them with integration tests?

Yes indeed, and that’s OK. Unit tests don’t make much sense for production code that lives at the edge of a software system. The adapter class you’ve created is located at the boundary of our application. Integration tests are very much warranted here. In fact, how would we otherwise know for certain whether things work correctly without actually carrying out HTTP calls?

I see your point. How can we move forward then?

Well, for starters, let’s remove all of these test doubles. Then we’ll refactor the tests by decoupling them from the implementation of the adapter.

— After a short while —

I must say that the tests now look a lot better than before.

Indeed. The most important thing is that they now fail for the right reason. At least one of these tests will turn red as soon as there’s a change in the behaviour of the adapter, not when its implementation gets changed.

I wish I had found out about this sooner.

Your original unit tests actually warned you about this.

How so?

You mentioned earlier that it was quite difficult to write unit tests for this adapter, right? That was in fact a first warning sign. You also talked about test doubles returning other test doubles because the API of the REST client library is a DSL. That was a second warning sign. Also, the REST client library itself is not being maintained by our team. Using test doubles for types that we don’t own is your third warning sign. And last but not least, just don’t use test doubles for I/O operations.

That were a lot of warning signs indeed. Maybe I just needed more coffee at the time I wrote the tests?

Good idea! Let’s grab ourselves a drink.

Jan Van Ryswyck

Thank you for visiting my blog. I’m a professional software developer since Y2K. A blogger since Y2K+5. Provider of training and coaching in XP practices. Curator of the Awesome Talks list. Past organizer of the European Virtual ALT.NET meetings. Thinking and learning about all kinds of technologies since forever.