Bad Tests Good Tests - Modification of Global State

For unit tests a rule of thumb is to keep them independent from each other. It is not so bad if we introduce a dependency on purpose (and explicitly declare it e.g. using TestNG dependsOnMethod feature). A worse scenario is when we are not aware of the dependency ourselves. This can bring serious problems on our heads.

Tip Keep your tests independent from each other. Or at least make the dependency explicit!

Let us have a look at the following example. The test presented below verifies whether log4j configuration code works fine. If some system property (logConfig) is available, it should load file specified by this property. If the property is not set it should load some default values.

LoggingPropertyConfigurator configurator
        = mock(LoggingPropertyConfigurator.class);
BaseServletContextListener baseServletContextListener
        = new BaseServletContextListener(configurator);

@Test
public void shouldLoadDefaultProperties() {
        baseServletContextListener.contextInitialized(null);
        verify(configurator).configure(any(Properties.class));
}

@Test(expected = LoggingInitialisationException.class)
public void shouldThrowLoggingException() {
        System.setProperty("logConfig", "nonExistingFile");
        baseServletContextListener.contextInitialized(null);
}

This test used to be green for months. Then, suddenly, it turned red. Why?! For no apparent reason, the log4j config was not something we changed often. In fact, we haven’t touched it for a long time…

This post is a part of my new free book about tests. You can learn more here. Please share your comments (so the book is better).
The idea for the book is to discuss examples of test classes and to present ways of making them better.

Click here for other posts about tests.

After some digging we have found out that the test failed because the order of execution of tests changed. As long as shouldLoadDefaultProperties was executed before shouldThrowLoggingException everything was fine. But once this order was changed, things started to go wrong. In particular, the logConfig system property was available when shouldLoadDefaultProperties test was executed which altered the behaviour of SUT and made this test fail.

And why did the order of execution changed? Well, it does not really matter. In general test frameworks do not guarantee the order of execution (unless you explicitly ask for it) so you should not rely on this.

Tip Be extra cautious when modifying global state (system properties, file system, database etc.) in your tests. This can influence other tests. Take care to make sure that tests execute in well-defined environment!

Now, how to fix it. Basically there are two solutions.

First, you can impose strict order of execution of these two test methods (TestNG will allow you to do this, JUnit won’t).

@Test
public void shouldLoadDefaultProperties() { ... }

@Test(expected = LoggingInitialisationException.class,
        dependesOnMethod = "shouldLoadDefaultProperties")
public void shouldThrowLoggingException() { ... }

This works as long as there are no more tests which outcome also depend on the value of logConfig system property. If such tests are added you need to remember to also specify their relation (their dependency) to these existing tests.

Another option is to clean logConfig system property variable before shouldLoadDefaultProperties is executed. If there are more tests like this then maybe putting the cleaning code some setUp() method would be a good idea. For example:

@BeforeMethod
public void cleanSystemProperties() {
        System.setProperty("logConfig", null);
}

// the rest of the code remains unchanged
Tip Clean the environment before tests, not after it. This way your tests are guaranteed to run in clean environment.
Please comment using