Each System Property Should Have Its Own Class

A lot of Java applications use System properties to hold their configuration. It’s very convenient especially for webapps, because you can set these properties from the server’s admin console.

Naturally, somewhere in the code, the properties are checked and a decision is made based on them. The question is: how can that code be well unit-tested?

Take the following class:

public class Foo {
  public boolean act() {
    if("blue".equals(System.getProperty("color")) {
      return true;
    } else {
      return false;
    }
  }
}

And its corresponding test class:

public class FooTest {
  @Test
  public void actsWithBlue() {
    System.setProperty("color", "blue");
    assertTrue(new Foo().act());
  }

  @Test
  public void actsWithoutBlue() {
    System.setProperty("color", "green");
    assertFalse(new Foo().act());
  }

  @After
  public void clean(){
    System.clearProperty("color");
  }
}

Do these tests run fine? Yes. Do they leave the property wrongly initialized? No, the @After method runs even if a test throws an exception. So where’s the problem?

The problem occurs when you enable multi-threaded test runs. In that scenario the tests are run in parallel, not sequentially, which means that actsWithBlue() could initialize the “color” property before actsWithoutBlue() finishes execution, or vice-versa. Also, if other tests (for other classes) happen to work with real Foo objects instead of mocks, they will be in trouble as well.

I hope it’s clear that no test should set system properties, under any circumstances. Let’s introduce interface Color:

public interface Color {
  public String read();

  static class Fake implements Color {
    private String color;
    public Fake(String color) {
      this.color = color;
    }
    public String read() {
      return this.color;
    }
  }
}

and the real implementation:

public class SystemColor implements Color {
  public String read() {
    return System.getProperty("color");
  }
}

The Foo class becomes:

public class Foo {
  private Color color;
  public Foo(Color clr) {
    this.color = clr;
  }
  public boolean act() {
    if("blue".equals(this.color.read()) {
      return true;
    } else {
      return false;
    }
  }
}

It can now be properly tested using Color.Fake:

public class FooTest {
  @Test
  public void actsWithBlue() {
    assertTrue(
      new Foo(
        new Color.Fake("blue")
      ).act()
    );
  }

  @Test
  public void actsWithoutBlue() {
    assertFalse(
      new Foo(
        new Color.Fake("green")
      ).act()
    );
  }
}

Done. Now the tests don’t have to set any system property and can run in parallel without affecting one another. By the way, I used a “fake” implementation of Color instead of Mockito or other frameworks. I like to do that, if the interfaces are small - then it really makes no sense to bring in all the weight of an entire mocking framework.

Finally, if you’re wondering how to unit test SystemColor.read(), I can only tell you that that’s the kind of code due to which coverage is never 100%. And, after all, I think we can safely assume that System.getProperty(...) (being a part of the jdk) runs ok.

How does your app read system properties?