Pragmatic Unit Testing in Java 8 with JUnit (2015)

Part 3. The Bigger Design Picture

Chapter 11. Refactoring Tests

Your tests represent a significant investment. They’ll pay off in minimizing defects and by allowing you to keep your production system clean through refactoring. But they also represent a continual cost. You need to continually revisit your tests as your system changes. At times you’ll want to make sweeping changes and might end up having to fix numerous broken tests as a result.

In this chapter you’ll address problems in your tests that can lead to increased costs. You’ll learn to refactor your tests, much as you would refactor your production system, to maximize understanding and minimize maintenance costs.

Searching for an Understanding

We’re tasked with making some enhancements to the search capabilities of our application. We know we’ll be changing the util.Search class, but none of us is familiar with exactly what the Search class does. We turn to the tests. Well, test. We have only one test, and at first glance we roll our eyes in frustration. What in the world is this test trying to prove?

(Text in pageContent by Herman Melville from Moby Dick.)

The test name, testSearch, doesn’t tell us anything useful. We see a couple of comments that don’t add much value either. If we want to fully understand what’s going on, we’ll have to read the test line-by-line and try to piece its steps together.

(We’re not even going to show you the Search class itself in this chapter—our focus will solely be on cleaning up the tests so that we can use them to understand how Search behaves. The source distribution will sate your curiosity.)

We decide to refactor testSearch() as we work our way through understanding it, with the goal of shaping it into one or more clear, expressive tests. To do so, we look for various test smells—nasty bits of code that emanate a bad odor.

Test Smell: Unnecessary Test Code

The test code that comprises testSearch() doesn’t expect any exceptions to be thrown. It contains a number of assertions against positive facts. If the test code throws an exception, a try/catch block catches it, spews a stack trace onto System.out, and explicitly fails the test. In other words, exceptions are unexpected by this test method.

Unless your tests expect an exception to be thrown—because you’ve explicitly designed the test to set the stage for throwing an exception—you can simply let the exceptions fly. Don’t worry, JUnit traps any exceptions that explode out of your test. JUnit marks a test that throws an exception as an error and displays the stack trace in its output. The explicit try/catch block adds no additional value.

Remove the try/catch block and modify the signature of testSearch() to indicate that it can throw an IOException:

The test now contains a little less distracting clutter. Yay!

We next notice a not-null assert—an assertion that verifies that a value is not null. The result of search.getMatches() is assigned to the matches local variable. The next statement asserts that matches is not a null value. The final assert verifies that the size of matches is at least 1:

Checking that a variable isn’t null before dereferencing it is safe and a good thing, right?

In production code, perhaps. In this test, the not-null assert is again clutter. It adds no value: if the matches reference ends up null, the call to matches.size() happily throws an exception. JUnit traps this exception and errors the test.

Like the try/catch block, the not-null assert imparts no additional useful information. It is unnecessary test code, so remove it:

That’s one fewer line of test to wade through!

Test Smell: Missing Abstractions

A well-structured test distills the interaction with the system to three portions: arranging the data, acting on the system, and asserting on the results (see Keeping Tests Consistent with AAA). Although the test requires detailed code to accomplish each of these steps, we can improve understanding by organizing those details into abstractions—code elements that maximize the essential concepts and hide the unnecessary details.

Our muddled test contains five lines that assert against the list of matches returned by search.getMatches(). We must read these five lines individually to understand what’s going on:

The five lines of assertion detail cover a single concept: does the list of matches contain a single entry with a specific search string and surrounding context? Let’s introduce a custom assertion that buries the five lines of detail required to make that assertion:

Paraphrased, the custom-matcher assertion says, “Assert that matches contains a list whose sole entry is equal to a Match object with specific values for the search string and surrounding context.” Just what we wanted. The five lines of implementation detail are embodied in the new custom-matcher class:

Implementing the matcher requires a few more lines of code, but simplifying the effort to understand the test is worth it. Further, we’ll be able to reuse the matcher in numerous additional tests. You can find another example of creating a custom assertion, with more-detailed explanation of the pieces required, at Creating a Custom Matcher to Verify an Invariant.

Anywhere you find two or three lines of code that implement a single concept, find a way to distill them to a single, clear statement in the test.

We spot another small opportunity for introducing an abstraction in the second chunk of the test. The final assertion compares the size of the results to 0:

The missing abstraction here is the concept of emptiness. Altering the assertion reduces the extra mental overhead needed to understand the size comparison:

Every small amount of mental clutter adds up. A system that contains never-ending clutter wears you down, much as road noise adds up to further fatigue you on a long car trip.

Test Smell: Irrelevant Information

A well-abstracted test emphasizes everything that’s important to understanding it and deemphasizes anything that’s not. The data used in a test should help tell a story.

Sometimes you’re forced to supply data to get code to compile, even though that data is irrelevant to the test at hand. For example, a method might take additional arguments that have no impact on the test.

Our test contains some “magic literals” that aren’t at all clear:

and:

We’re not sure what the "1" string represents, so we navigate into the constructors for Search and Match. We discover that the "1" represents a search title, a field whose value we don’t care about.

Including the "1" magic literal raises unnecessary questions. What does it represent? How, if at all, is it relevant to the results of the test? Your readers waste time when they must dig around to find answers. A better solution: give them the answer by introducing a meaningfully named constant.

The second call to the Search constructor contains a URL as the title argument:

At first glance, it appears that the URL has a correlation with the URL passed to the URL constructor two statements earlier. Digging reveals that no real correlation exists. Replace the confusing URL and the "1" magic literal with the A_TITLE constant, which represents a title with any value:

You might have named the constant ANY_TITLE or ARBITRARY_TITLE. Or you might have used the convention of an empty string to represent data that you don’t care about (though sometimes the distinction between an empty string and a nonempty string is relevant).

Test Smell: Bloated Construction

We must pass an InputStream to a Search object through its constructor. Our test builds an InputStream in two places. The first construction requires three statements:

As in our earlier example, where you introduced a custom assertion to compare matches (see Test Smell: Missing Abstractions), the extra implementation detail in the test represents a missing abstraction. The solution: introduce a helper method that creates an InputStream given appropriate text:

Hiding distracting detail has started to pay off. Our test is shaping into something that we can follow more quickly.

Test Smell: Multiple Assertions

We’ve mentioned a few times in this book (one example: F[I]RST: [I]solate Your Tests) that it’s a good idea to move in the direction of a single assert per test. You’ll sometimes find reasons to assert multiple postconditions in a single test, but more often the multiple assertions indicate that you have two test cases.

Our longer test screams, “Split me!” The first case represents finding a search result in the input, and the second case represents finding no match. Split the test into two, providing each with a name that concisely states the expected behavior given the context for the test:

If you split the test into two naively, you’ll note that the second call to stream.close() no longer compiles. A further look uncovers a small defect: the second test’s input stream is named inputStream, not stream, which means that the original test called close() twice on the same streamreference. Retain the close() statement in the second test after renaming the reference to inputStream.

Also, take the liberty of removing unhelpful comments. Single-purpose tests promote better test names that can help eliminate the need for comments.

Test Smell: Irrelevant Details in Test

Although we want to turn off logging when tests run, the code to do so is a distraction to understanding the essence of any test. And though as good coding citizens we should always close streams, doing so is also a distraction.

Move these bits of clutter to @Before and @After methods. To allow both tests to take advantage of the stream.close() in the @After method, change the second test to reference the stream field instead of the local variable named inputStream.

We also ponder the line that reads:

That assertion isn’t an irrelevant detail; it’s a valid assertion. We might consider that it’s a second postcondition of running a search, but it hints at something else: where’s the test case that generates a true value for search.errored()? Delete the assertion and make a note to add a third (and maybe also a fourth) test before committing.

Here are the decluttering changes:

Take care when moving details to @Before, @After, or helper methods. Make sure you don’t remove information useful to understanding a test.

Test Smell: Misleading Organization

Knowing which part of the test is the act part, which is the arrange part, and which is the assert can speed up cognition. Use AAA (Keeping Tests Consistent with AAA) to make the intent explicit. The arrows in the following listing show the blank lines to insert:

We’re getting close. Time for a final pass against the two tests!

Test Smell: Implicit Meaning

The biggest question each of your tests must clearly answer is, why does it expect the result it does? Readers must be able to correlate between the arrange and assert portions of the test. If the reason for getting the result that the assert expects isn’t clear, your readers waste time digging through the code to find an answer.

The returnsMatchesShowingContextWhenSearchStringInContent test expects a single match on a search for practical joke against a very long string. A reader can eventually spot the place in the string where the phrase practical joke appears and can then do the math to figure out that ten characters before it and ten characters after it represent the string:

But that’s making your test readers dig to find understanding. They’ll be annoyed no matter how amusing the test data is. Make things explicit by choosing better test data. Change the input stream to contain but a relative smattering of text. Also change the content so that the surrounding context information doesn’t need to be explicitly counted:

Take a closer look at the second test, noMatchesReturnedWhenSearchStringNotInContent. It works against a live URL’s input stream, making it a slow test. Although we want at least one such live test, we decide to turn this test into a unit test.

Initialize the stream field to contain a small bit of arbitrary text. To help make the test’s circumstance clear, search for "text that doesn’t match":

Using streamOn() lets you remove the throws clause from the test’s signature.

You have no end of ways to improve the correlation across a test. Meaningful constants, better variable names, better data, and sometimes even doing small calculations in the test can help. Use your creativity here!

Adding a New Test

With our initial ugly test whittled into two sleek, clear tests, it’s now relatively easy to add a couple of new tests. First let’s write a test that demonstrates how a search returns true for the errored() query:

And add a test for the opposite:

Time spent to add the new tests: less than a few minutes each.

After

The refactored tests are a thing of simplicity. Readers understand what case is being demonstrated by reading a test’s name. They can focus initially on the act part of the test to know what code is getting executed. They read the arrange part to determine the context in which the test is running, and they read the single assert so they know what the expected result is. Each of these digesting actions happens quickly, far more so than before. The time to comprehend tests reduces from perhaps minutes down to handfuls of seconds.

You now have a complete picture of what you must do in the name of design: refactor your production code for clarity and conciseness, refactor your production code to support more flexibility in design, design your system to support mocking of dependency challenges, and refactor your tests to minimize maintenance and maximize understanding.

You’re ready to move on to the final part of this book, a smorgasbord of larger topics on unit testing.