How Stable are Your Unit Tests? Best Practices to Raise Test Automation Quality

Most of our 10k+ unit tests are written in Java using JUnit and run with the gradle build automation tool. The more tests we added over time, the more often we ran into the problem that unit test executions became unstable. New tests impacted the execution of existing tests. Our “failed test” metric for tests that ran fine for months started to increase. It was tempting to blame bad application code, but through careful analysis we found the real reason for these unstable test results. Many of these problems were caused by new tests that adversely impacted the test environment, and therefore also the execution of other tests.

In this blog post I will show you how we identified the root cause of these specific test failures as well as our derived best practices for unit test design with respect to its interaction with the test environment.

Last week we identified a group of tests that failed for what seemed to be no specific reason. I volunteered to investigate.

A group of unit tests verifies important required behavior of a ThreadPool implementation. In particular, scheduled periodic tasks must continue to execute even after certain exceptions are thrown. To test this, a task is scheduled which throws the target exception. The test then waits for a second execution after the first one was aborted due to the exception.

On some machines and test runs, these tests would time out waiting for re-execution. No log output would be written, although various exception handlers were in place to log any exception. Only the message “Exception: java.lang.NullPointerException thrown from the UncaughtExceptionHandler in thread “pool-unit-test-thread-1” was printed. However, this was never reproducible when executing the tests in Eclipse, only when running the test suite in gradle.

Next steps: I instructed gradle to open a debug port and then connected Eclipse to determine the cause. This revealed that the NullPointerException was generated somewhere in gradle code. I downloaded the source code and discovered that System.getProperty(“line.separator”)returned null and was dereferenced.

With this information, I searched and quickly found another test that verifies the string formatting on different platforms that had the side effect of changing the line.separator property. By calling System.clearProperty(“line.separator”)after the test, it inadvertently set that property to null.

In this context, when the next test was run, log information would be written to the console and gradle code would be invoked because it uses System.setOut to redirect console output. This in turn caused the NullPointerException as gradle accessed the line.separator property value. Note that the execution order is important, and by chance it was sometimes during the ThreadPool test.

Usually a set of handlers would catch that, but as they too wrote to the console, they encountered a NullPointerException and thus propagated the failure.

The quick fix for the failure is to remove the clearProperty call and instead use the previous value of the “line.separator” property. But can we do better?

Part of the above failure is that for test automation to perform within expectations, JVM instances are usually reused, at least within the same project. Therefore ideally unit tests should have no side effects on their test environment, to prevent such failures.

The test environment includes various resources that can influence tests executed afterwards, such as creating files on the file system or as in the case above, altering Java system properties. For files, JUnit offers the TemporaryFolder rule to create temporary files, so we built a similar mechanism for properties.

In general, suppose we want to develop a test which requires a Java System property to be set to a specific value, writing something like this:

Test.java

@Before
public void setup() {
  System.setProperty("prop", "true");
}
@Test
// use that prop..

But, when other tests are run in the same JVM after this test, the property is also set and may influence the behavior of the following tests! So let’s improve that to:

Test.java

private String oldValue;
@Before
public void setup() {
  // setProperty returns the old value of that property.
  oldValue = System.setProperty("prop", "true");
}

@After
public void teardown() {
  System.setProperty("prop", oldValue);
}
@Test
// use that prop..

Looks fine, doesn’t it? But there still is a problem: If the property wasn’t set, setProperty in setup returns null and teardown throws a NullPointerException. So the correct version for a unit test to leave its system property environment unchanged would be:

Test.java

private String oldValue;
@Before
public void setup() {
  // setProperty returns the old value of that property.
  oldValue = System.setProperty("prop", "true");
}

@After
public void teardown() {
  if( oldValue == null ) {
    System.clearProperty("prop");
  } else {
    System.setProperty("prop", oldValue);
  }
}
@Test
// Use that property..

This is a bit verbose.. A helper based on the Java 7 try-with-resources statement accomplishes the same thing:

Test.java

private ScopedProperty property;
@Before
public void setup() {
  property = new ScopedProperty("prop", "true");
}
@After
public void teardown() {
  // Does same things like above.
  property.close();
}
@Test
// use that prop..

or also:

Test.java

@Test
public void test() {
  try(ScopedProperty prop = new ScopedProperty("prop", "true")) {
    // use that prop..
  }
}

In the first case it’s even better to implement a JUnit rule, where you can’t forget to write the @After close. For instance:

Test.java

@ClassRule
public static ScopedPropertyRule prop = new ScopedPropertyRule("prop", "true");
@Test
public void test1() {
 // use that prop..
}
@Test
public void test2() {
// use that prop..}

Where the property is set in the @BeforeClass and the original state restored in @AfterClass.

To avoid test failures based on the order of execution unit tests should be side effect-free. The real world example demonstrated the relevance of this topic. To further this practice, we developed helper classes for the case of system properties. The same principle is also applicable to other environment objects, such as Thread priorities, the Java security manager and classes similar to ScopedProperty and ScopedPropertyRule, and are easily implemented. As we always strive to improve our test automation quality I would be interested in other best practices. Leave us a comment on what you are doing in our development teams. (Thanks to Reinhold Füreder for the input on JUnit rules!)

ScopedProperty.java

/**
  * A helper to switch a system property value and restore the previous one.
  *
  * When used in try-with-resources, restores the values automatically.
  */
public class ScopedProperty implements AutoCloseable {

    private final String key;
    private final String oldValue;

    /**
     *
     * @param key The System.setProperty key
     * @param value The System.setProperty value to switch to.
     */
    public ScopedProperty(final String key, final String value) {
        this.key = key;
        oldValue = System.setProperty(key, value);
    }

    @Override
    public void close() {
        // Can't use setProperty(key, null) -> Throws NullPointerException.
        if( oldValue == null ) {
            // Previously there was no entry.
            System.clearProperty(key);
        } else {
            System.setProperty(key, oldValue);
        }
    }
}

Here, ScopedProperty implements AutoCloseable to function in the try-with-resources statement.

ScopedPropertyRule.java

/**
 * A JUnit test rule, which changes a property value within a test
 * and restores the original one afterwards.
 * See {@link ScopedProperty}.
 */
public class ScopedPropertyRule extends ExternalResource {

    private final String key;
    private final String value;
    private ScopedProperty scopedProperty;

    public ScopedPropertyRule(final String key, final String value) {
        this.key =key;
        this.value = value;
    }

    @Override
    protected void before() throws Throwable {
        scopedProperty = new ScopedProperty(key, value);
    }

    @Override
    protected void after() {
        scopedProperty.close();
    }
}

This class uses ExternalResource to implement a JUnit test rule.