Item 8: Don’t reference production constants in tests

By Ashley Waldron

All of the source code for this series can be found in this repository. Specifically, the code used throughout this item can be found here and its corresponding test classes found here.

Directly referencing production constants in test assertions is something I see happen fairly frequently and it’s dangerous. To see why let’s update our UserService class to extract the exception message out to a public constant instead of hardcoding the message in the exception creation.

public class Item8UserService {
 
    public static final String INVALID_REGION_MESSAGE = "Region must be less than 30 characters in length";
 
....................
 
    private void validate(CreateUserProfileRequest createUserProfileRequest) {
        if(createUserProfileRequest.getRegion() == null || createUserProfileRequest.getRegion().length() > 30) {
            throw new RuntimeException(INVALID_REGION_MESSAGE);
        }
    }

Now let’s update our testcases to reference this constant when asserting that the message was as expected [See Item8UserServiceTest]:

    @Test
    void createWithNullRegionItem8() {
        createUserProfileRequest.setRegion(null);
 
        RuntimeException thrownException = assertThrows(RuntimeException.class, () -> item7UserService.create(createUserProfileRequest));
 
        assertThat(thrownException.getMessage(), is(equalTo(INVALID_REGION_MESSAGE)));
        then(employeeNumberGenerator).shouldHaveNoInteractions();
        then(userRepository).shouldHaveNoInteractions();
        then(emailService).shouldHaveNoInteractions();
        then(eventBroadcaster).shouldHaveNoInteractions();
    }
 
    @Test
    void createWith31CharacterRegionItem7() {
        createUserProfileRequest.setRegion("ThisRegionIs31CharactersLongThi");
 
        RuntimeException thrownException = assertThrows(RuntimeException.class, () -> item7UserService.create(createUserProfileRequest));
 
        assertThat(thrownException.getMessage(), is(equalTo(INVALID_REGION_MESSAGE)));
        then(employeeNumberGenerator).shouldHaveNoInteractions();
        then(userRepository).shouldHaveNoInteractions();
        then(emailService).shouldHaveNoInteractions();
        then(eventBroadcaster).shouldHaveNoInteractions();
    }

On lines 115 and 128 we replaced the hardcoded String with a reference to the constant and these tests still pass as expected.

This might seem like a good idea because it cuts down on copy and pasted code. However, what happens if a developer changes the value of the INVALID_REGION_MESSAGE constant by accident?

If we change this:

public static final String INVALID_REGION_MESSAGE = "Region must be less than 30 characters in length";

To this [See Item8UserService]:

public static final String INVALID_REGION_MESSAGE = "Region must be less than 3000000000000000000000 characters in length";

What happens when we run our tests again?

They should fail but they don’t. It’s easy to see why, the asserts are basically testing the message against itself! What the asserts are currently saying is: the message in the thrown exception should be set to the value of the INVALID_REASON_MESSAGE constant. But what they should really be saying is: the message in the thrown exception should be “Region must be less than 30 characters in length“.

You could argue that both statements above are valid assertions and I guess they are. But unlike the first statement, the second statement is explicit in what the exact value of the message should be. And so it’s a stricter and more powerful assertion.

Just create another test constant for expected values

If you want to avoid copying the same hardcoded expected message into each test (as was done in the tests before this item) you should create a constant in your test class with the expected message text (just like we did in item 4) rather than reusing the constant from the code under test.