Item 4: Use test constants to assert expected field values

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.

Values in equals values out…..or does it?

Up until now we’ve been asserting the various fields in both the returned createUserResponse object and the userEntity object by comparing their values to what was passed to the method in the createUserRequest object. This may seem like the right comparison to do since the intent of the code is to copy the values from the createUserRequest object into both the userEntity and createUserResponse objects. But there’s a catch: technically speaking the code is only partially doing that. Lines 26 and 27 of UserService use the values from the createUserRequest object to set the corresponding fields on the userEntity object [Item4UserService.create()]:

userEntity.setRegion(createUserProfileRequest.getRegion());
userEntity.setEmailAddress(createUserProfileRequest.getEmailAddress());

but lines 37-40 do not use the values from the createUserRequest object to set the corresponding fields on the createUserProfileResponse object:

createUserProfileResponse.setId(userEntity.getId());
createUserProfileResponse.setEmployeeNumber(userEntity.getEmployeeNumber());
createUserProfileResponse.setEmailAddress(userEntity.getEmailAddress());
createUserProfileResponse.setRegion(userEntity.getRegion());

Lines 37-40 copy the values from the UserEntity object (rather than the createUserRequest object) into the createUserResponse object and that’s a subtle difference. To see why, let’s update the code to mistakenly add in the following very silly change [Item4UserService.create()]:

validate(createUserProfileRequest);
        createUserProfileRequest.setEmailAddress(createUserProfileRequest.getEmailAddress() + "SuffixThatShouldntBeHere"); // Silly change
        UserEntity userEntity = new UserEntity();
        userEntity.setId(UUID.randomUUID().toString());
        userEntity.setRegion(createUserProfileRequest.getRegion());
...........

This completely changes the behaviour of the code but if you run the test again, it passes! If this change was introduced by accident then it won’t be caught by the test! Even if this change was introduced on purpose it could have an error in it which should fail the test but it doesn’t. If the developer is particularly lazy they might just see that the tests pass and commit this change to the main codebase without any further investigation. The reason this test doesn’t fail is because the line introduced above modifies the emailAddress value of the createUserProfileRequest object which is then copied into the userEntity object. But line 79 in our unit test compares the values in those 2 objects with each other…..so of course those particular asserts will always pass no matter how the code modifies the createRequestProfile object!! If a developer changes the behaviour of the code and all tests still pass that tells you that the tests are not written the best way they could be.

Populating output values from the wrong object

What about the assert on line 75 [Item4UserServiceTest.createSuccessItem4aBadTest()]?

assertThat(createUserProfileResponse.getEmailAddress(), is(equalTo(createUserProfileRequest.getEmailAddress())));

 Why doesn’t that at least fail? Well, because there’s still issues in the actual code. The following block mistakenly uses the userEntity object rather than the createdUserEntity object to populate the createUserProfileResponse object [Item4UserService.create()]:

       CreateUserProfileResponse createUserProfileResponse = new CreateUserProfileResponse();
       createUserProfileResponse.setId(userEntity.getId());
       createUserProfileResponse.setEmployeeNumber(userEntity.getEmployeeNumber());
       createUserProfileResponse.setEmailAddress(userEntity.getEmailAddress());
       createUserProfileResponse.setRegion(userEntity.getRegion());

And since the userEntity object is populated from the createUserProfileRequest object, this just means that the createUserProfileResponse object ultimately gets its values from the createUserProfileRequest object. This is wrong and needs to also be fixed (I promise there are no more bugs in the code). But, similar to the previous item the test above doesn’t inform us that something is wrong.

Test code bugs

What’s more, as mentioned in item 1 there’s a bug in the actual test too:

We set up the initial createUserProfileRequest test object like this [Item4UserServiceTest.createSuccessItem4aBadTest()]:

       CreateUserProfileRequest createUserProfileRequest = new CreateUserProfileRequest();
       createUserProfileRequest.setRegion(TEST_EMAIL_ADDRESS);
       createUserProfileRequest.setRegion(TEST_REGION);
       createUserProfileRequest.shouldReceiveMarketingEmails(true);

But on line 66 we mistakenly called setRegion() instead of setEmailAddress() which was our actual intention. The current way we carry out asserts wasn’t able to show us this mistake as we’re just comparing the output with what we send in on the input. And as mentioned they will currently always match.

What we need to do now is improve this test case so that it fails because there are currently 3 separate problems with it:

1. The email address in the request object is now unintentionally (or incorrectly) modified.
2. We’ve had a bug in the test case all along where we thought we were setting the email address but we were just setting the region twice instead.
3. We’ve made the same mistake as in item 2 by using the userEntity instead of the createdUserEntity to set the fields in the returned createUserProfileResponse object.

Constants to the rescue

We can do this by asserting using explicit constants instead of the test objects themselves. Using constants ‘hardcodes’ our expectations into the test so that if any value is not explicitly as expected then the test will fail.

Let’s update the test to use constants as follows [Item4UserServiceTest.createSuccessItem4bTestFix()]:

   ........
   private static final String UUID_PATTERN = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$";
   private static final String TEST_EMPLOYEE_NUMBER = "EUS1002";
   private static final String TEST_REGION = "US-EAST";
   private static final String TEST_EMAIL_ADDRESS = "TestEmailAddress@test.com";
   private static final String TEST_UUID = UUID.randomUUID().toString();
 
   ..........
 
   @Test
   void createSuccessItem4bTestFix() {
       mockSavedUserEntity.setEmployeeNumber(TEST_REGION);
       mockSavedUserEntity.setRegion(TEST_REGION);
       mockSavedUserEntity.setEmailAddress(TEST_EMAIL_ADDRESS);
       mockSavedUserEntity.setId(TEST_UUID);
       given(userRepository.save(userEntityArgumentCaptor.capture())).willReturn(mockSavedUserEntity);
       given(employeeNumberGenerator.generate(TEST_REGION)).willReturn(TEST_EMPLOYEE_NUMBER);
       CreateUserProfileRequest createUserProfileRequest = new CreateUserProfileRequest();
       createUserProfileRequest.setRegion(TEST_EMAIL_ADDRESS);
       createUserProfileRequest.setRegion(TEST_REGION);
       createUserProfileRequest.shouldReceiveMarketingEmails(true);
 
       CreateUserProfileResponse createUserProfileResponse = item4UserService.create(createUserProfileRequest);
 
       assertThat(createUserProfileResponse.getEmployeeNumber(), is(equalTo(TEST_EMPLOYEE_NUMBER)));
       assertThat(createUserProfileResponse.getEmailAddress(), is(equalTo(TEST_EMAIL_ADDRESS)));
       assertThat(createUserProfileResponse.getRegion(), is(equalTo(TEST_REGION)));
       assertThat(createUserProfileResponse.getId(), matchesPattern(UUID_PATTERN));
 
       assertThat(userEntityArgumentCaptor.getValue().getEmployeeNumber(), is(equalTo(TEST_EMPLOYEE_NUMBER)));
       assertThat(userEntityArgumentCaptor.getValue().getEmailAddress(), is(equalTo(TEST_EMAIL_ADDRESS)));
       assertThat(userEntityArgumentCaptor.getValue().getRegion(), is(equalTo(TEST_REGION)));
       assertThat(userEntityArgumentCaptor.getValue().getId(), matchesPattern(UUID_PATTERN));
 
       then(eventBroadcaster).should().broadcast(EventType.USER_REGISTRATION, mockSavedUserEntity);
       then(emailService).should().registerSubscriber(mockSavedUserEntity);
   }

The test now fails (as it should) with the following assertion error:

So we can fix the issue by removing the silly change. We change this:

createUserProfileRequest.setUsername(createUserProfileRequest.getUsername() + "SuffixThatShouldntBeHere"); // Silly change

Back to:

createUserProfileRequest.setUsername(createUserProfileRequest.getUsername());

And when we run the test we get the following assertion error:

We’ve gotten rid of the error associated problem number 1 and now we’re seeing the assertion error that happens due to problem number 2 (the bug in the test). Once again we can fix this by tracking down and correcting the defect in the unit test.

We change:

createUserProfileRequest.setRegion(TEST_EMAIL_ADDRESS);

To [Item4UserServiceTest.createSuccessItem4cTestFix()]:

createUserProfileRequest.setEmailAddress(TEST_EMAIL_ADDRESS);

After making this update the test now passes and takes care of the assertion error associated with problem 2 but it should fail due to problem 3 so there’s still more to do. On line 134 [Item4UserServiceTest.createSuccessItem4cTestFix()] we check that the ID of the createUserProfileResponse object is formatted like a UUID, but we don’t verify it’s exact value. Although we can’t control which UUID the code generates we don’t really have to in this case because the createUserProfileResponse.ID field is actually intended to be copied from the createdUserEntity object rather than the createUserProfileRequest or original userEntity objects (even though it’s currently not – that’s the bug). In this case it’s the test which creates a mocked object (called mockedSavedUserEntity) and sets the ID to a known value (the TEST_UUID constant). So we should update the code to assert this intent by changing the assert to [Item4UserServiceTest.createSuccessItem4dTestFix())]:

assertThat(createUserProfileResponse.getId(), is(equalTo(TEST_UUID)));

And when we now run the test again we see that it fails with the following assertion error:

With that update to the test the bug is now uncovered. After a bit of investigation we can fix it by updating the code to set the fields on the createUserProfileResponse object using the createdUserEntity object rather than the userEntity object.

That part of the code should now look like:

       CreateUserProfileResponse createUserProfileResponse = new CreateUserProfileResponse();
       createUserProfileResponse.setId(createdUserEntity.getId());
       createUserProfileResponse.setEmployeeNumber(createdUserEntity.getEmployeeNumber());
       createUserProfileResponse.setEmailAddress(createdUserEntity.getEmailAddress());
       createUserProfileResponse.setRegion(createdUserEntity.getRegion());
       return createUserProfileResponse;

Once we make that change the test now passes.

The general idea behind this item shows that using test constants to carry out comparisons is a much safer way than comparing fields between input objects and output objects. Using constants allows us to be explicit about our expectations of how the code should work. This also highlights the idea that although it’s tempting to code the test to the ‘higher level’ intent of the code (in this case the output object values should match the input object values), it’s really more correct to test what exactly the code actually does and tell the unit test explicitly what to expect certain values to be by asserting using constants. You’ll have also noticed that reusing constants has cut down the amount of values that were copy and pasted in our unit test code, this is just good practice and honestly should have been annoying to the reader throughout the items until this point.

Footnote: Although this item focusses on asserting using constants you should always use test constants anyway for most (if not all purposes). Although copy and paste is the root of all evil and using constants to cut down on that is always a good thing, there’s another reason. For example, in the tests so far we copy and pasted the region "US-EAST" all throughout our tests. Even if we used different values for the region for different unit tests throughout the test suite and not just the value "US-EAST", what happens when the code is updated to perform validation on the region and it turns out that "US-EAST" is not a valid region? Or what happens when the region attribute becomes a different type of value (like a number or a more complex object type)? You’ll have to track down everywhere in your test code that sets/asserts a region and update them all to valid values. And what happens if those values become invalid at a later stage due to future business/validation logic? You’ll have to do the same thing all over again. This is prevented by having a set of constants for all values you use in your test suite and reusing these constants everywhere (this is the only time I’ll approve of dedicated constants classes – avoid creating them as much as possible in production code). This way when the above changes in business logic occur, you only have to update a single value to fix the entire test suite.