The Temporary Test Property

Just because you can make a variable a long-term property of a test fixture doesn’t mean you should.

This is the Everything is a Property test smell.

It may be seen in languages such as JavaScript where there’s a master let setting up some useful variables for various tests to use to assign values to. I’ve more seen it in Java though, where there can be a sort of phobia of creating temporary variables inside tests, so instead all the variables are declared in the parent class.

Let’s look at a quick example:

class ParagraphAnalyzerTest {
    private String analyzed;
    private ParagraphAnalyzer analyzer = new ParagraphAnalyzer();

    @Test
    void nouns() {
        analyzed = analyzer.justNouns("This is a word");
        assertThat(analyzed).isEqualTo("word");
    }

    @Test
    void verbs() {
        analyzed = analyzer.justVerbs("This is a word");
        assertThat(analyzed).isEqualTo("is");
    }

    @Test
    void ends() {
        analyzed = analyzer.first("This is a word");
        assertThat(analyzed).isEqualTo("This");

        analyzed = analyzer.last("This is a word");
        assertThat(analyzed).isEqualTo("words");
    }
}

In the above simple, fictional example, the test class has two members. There’s ParagraphAnalyzer which is the code under test and which it seems to make sense to have as a member of the fixture. Then there’s analyzed which seems to have a different value in each test.

The fact that each test gives its own value to the analyzed is a smell that analyzed is not being managed by the test fixture and so does not belong as a property of it.

We’re saving a repeated declaration of a String every time… which is so very not worth the baggage of this risky shared property.

Now, the risk is probably nothing, because the above test will create a new instance of the class for each test case, so there’s no bleed between states… but that’s only true if I don’t change the test instance lifecycle. JUnit 5 will let me do this differently. Plus, if I’m in the habit of using members for temp variables, what’s to stop me doing that with static members as well?

There’s no justification for this temp variable being promoted somewhere higher up the food chain.

You may also notice another test anti-pattern in the last unit test, above. The value of analyzed is being reassigned. Another awkward move. This is the Two For the Price of One test smell, where a single test case is executing two independent use cases.

When To Use What?

Consider each test case as having its own state. The test case inherits any commonly set up or torn down state from the test fixture. This approach avoids code repetition, and other difficulties with setting up expensive resources for each test.

But… as soon as the common set up is done, each test case should do its own thing, clearly and without littering the test fixture with its problems.

Storing what is essentially request scope in the instance state of an object is an anti pattern outside of testing and should be considered on inside it too!