It is pretty hard to figure out a good definition for clean code because everyone of us has our own definition for the word clean. However, there is one definition which seems to be universal:
Clean code is easy to read.
This might come as a surprise to some of you, but I think that this definition applies to test code as well. It is in our best interests to make our tests as readable as possible because:
- If our tests are easy to read, it is easy to understand how our code works.
- If our tests are easy to read, it is easy to find the problem if a test fails (without using a debugger).
It isn’t hard to write clean tests, but it takes a lot of practice, and that is why so many developers are struggling with it.
I have struggled with this too, and that is why I decided to share my findings with you.
This is the third part of my tutorial which describes how we can write clean tests. This time we will learn two techniques which can be used to remove magic numbers from our tests.
Constants to the Rescue
We use constants in our code because without constants our code would be littered with magic numbers. Using magic numbers has two consequences:
- Our code is hard to read because magic numbers are just values without meaning.
- Our code is hard to maintain because if we have to change the value of a magic number, we have to find all occurrences of that magic number and update everyone of them.
In other words,
- Constants help us to replace magic numbers with something that describes the reason of its existence.
- Constants make our code easier to maintain because if the value of a constant changes, we have to make that change only to one place.
If we think about the magic numbers found from our test cases, we notice that they can be divided into two groups:
- Magic numbers which are relevant to a single test class. A typical example of this kind of magic number is the property value of an object which created in a test method. We should declare these constants in the test class.
- Magic numbers which are relevant to multiple test classes. A good example of this kind of magic number is the content type of a request processed by a Spring MVC controller. We should add these constants to a non-instantiable class.
Let’s take a closer look at both situations.
Declaring Constants in the Test Class
So, why should we declare some constants in our test class?
After all, if we think about the benefits of using constants, the first thing that comes to mind is that we should eliminate magic numbers from our tests by creating classes which contains the constants used in our tests. For example, we could create a TodoConstants class which contains the constants used in the TodoControllerTest, TodoCrudServiceTest, and TodoTest classes.
This is a bad idea.
Although it is sometimes wise to share data in this way, we shouldn’t make this decision lightly because most of the time our only motivation to introduce constants in our tests is to avoid typos and magic numbers.
Also, if the magic numbers are relevant only to the a single test class, it makes no sense to introduce this kind of dependency to our tests just because we want to minimize the number of created constants.
In my opinion, the simplest way to deal with this kind of situation is to declare constants in the test class.
Let’s find out how we can improve the unit test described in the previous part of this tutorial. That unit test is written to test the registerNewUserAccount() method of the RepositoryUserService class, and it verifies that this method is working correctly when a new user account is created by using a social sign provider and a unique email address.
The source code of the that test case looks as follows:
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import org.springframework.security.crypto.password.PasswordEncoder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class)
public class RepositoryUserServiceTest {
private RepositoryUserService registrationService;
@Mock
private PasswordEncoder passwordEncoder;
@Mock
private UserRepository repository;
@Before
public void setUp() {
registrationService = new RepositoryUserService(passwordEncoder, repository);
}
@Test
public void registerNewUserAccount_SocialSignInAndUniqueEmail_ShouldCreateNewUserAccountAndSetSignInProvider() throws DuplicateEmailException {
RegistrationForm registration = new RegistrationForm();
registration.setEmail("john.smith@gmail.com");
registration.setFirstName("John");
registration.setLastName("Smith");
registration.setSignInProvider(SocialMediaService.TWITTER);
when(repository.findByEmail("john.smith@gmail.com")).thenReturn(null);
when(repository.save(isA(User.class))).thenAnswer(new Answer<User>() {
@Override
public User answer(InvocationOnMock invocation) throws Throwable {
Object[] arguments = invocation.getArguments();
return (User) arguments[0];
}
});
User createdUserAccount = registrationService.registerNewUserAccount(registration);
assertEquals("john.smith@gmail.com", createdUserAccount.getEmail());
assertEquals("John", createdUserAccount.getFirstName());
assertEquals("Smith", createdUserAccount.getLastName());
assertEquals(SocialMediaService.TWITTER, createdUserAccount.getSignInProvider());
assertEquals(Role.ROLE_USER, createdUserAccount.getRole());
assertNull(createdUserAccount.getPassword());
verify(repository, times(1)).findByEmail("john.smith@gmail.com");
verify(repository, times(1)).save(createdUserAccount);
verifyNoMoreInteractions(repository);
verifyZeroInteractions(passwordEncoder);
}
}The problem is that this test case uses magic numbers when it creates a new RegistrationForm object, configures the behavior of the UserRepository mock, verifies that information of the returned User object is correct, and verifies that the correct method methods of the UserRepository mock are called in the tested service method.
After we have removed these magic numbers by declaring constants in our test class, the source code of our test looks as follows:
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;
import org.springframework.security.crypto.password.PasswordEncoder;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class)
public class RepositoryUserServiceTest {
private static final String REGISTRATION_EMAIL_ADDRESS = "john.smith@gmail.com";
private static final String REGISTRATION_FIRST_NAME = "John";
private static final String REGISTRATION_LAST_NAME = "Smith";
private static final Role ROLE_REGISTERED_USER = Role.ROLE_USER;
private static final SocialMediaService SOCIAL_SIGN_IN_PROVIDER = SocialMediaService.TWITTER;
private RepositoryUserService registrationService;
@Mock
private PasswordEncoder passwordEncoder;
@Mock
private UserRepository repository;
@Before
public void setUp() {
registrationService = new RepositoryUserService(passwordEncoder, repository);
}
@Test
public void registerNewUserAccount_SocialSignInAndUniqueEmail_ShouldCreateNewUserAccountAndSetSignInProvider() throws DuplicateEmailException {
RegistrationForm registration = new RegistrationForm();
registration.setEmail(REGISTRATION_EMAIL_ADDRESS);
registration.setFirstName(REGISTRATION_FIRST_NAME);
registration.setLastName(REGISTRATION_LAST_NAME);
registration.setSignInProvider(SOCIAL_SIGN_IN_PROVIDER);
when(repository.findByEmail(REGISTRATION_EMAIL_ADDRESS)).thenReturn(null);
when(repository.save(isA(User.class))).thenAnswer(new Answer<User>() {
@Override
public User answer(InvocationOnMock invocation) throws Throwable {
Object[] arguments = invocation.getArguments();
return (User) arguments[0];
}
});
User createdUserAccount = registrationService.registerNewUserAccount(registration);
assertEquals(REGISTRATION_EMAIL_ADDRESS, createdUserAccount.getEmail());
assertEquals(REGISTRATION_FIRST_NAME, createdUserAccount.getFirstName());
assertEquals(REGISTRATION_LAST_NAME, createdUserAccount.getLastName());
assertEquals(SOCIAL_SIGN_IN_PROVIDER, createdUserAccount.getSignInProvider());
assertEquals(ROLE_REGISTERED_USER, createdUserAccount.getRole());
assertNull(createdUserAccount.getPassword());
verify(repository, times(1)).findByEmail(REGISTRATION_EMAIL_ADDRESS);
verify(repository, times(1)).save(createdUserAccount);
verifyNoMoreInteractions(repository);
verifyZeroInteractions(passwordEncoder);
}
}This example demonstrates that declaring constants in the test class has three benefits:
- Our test case is easier to read because the magic numbers are replaced with constants which are named properly.
- Our test case is easier to maintain because we can change the values of constants without making any changes to the actual test case.
- It is easier to write new tests for the registerNewUserAccount() method of the RepositoryUserService class because we can use constants instead of magic numbers. This means that we don’t have to worry about typos.
However, sometimes our tests use magic numbers which are truly relevant to multiple test classes. Let’s find out how we can deal with this situation.
Adding Constants to a Non-Instantiable Class
If the constant is relevant for multiple test classes, it makes no sense to declare the constant in every test class which uses it. Let’s take a look at one situation where it makes sense to add constant to a non-instantiable class.
Let’s assume that we have to write two unit tests for a REST API:
- The first unit test ensures that we cannot add an empty todo entry to the database.
- The second unit test ensures that we cannot add an empty note to the database.
These unit tests use the Spring MVC Test framework. If you are not familiar with it, you might want to take a look at my
Spring MVC Test tutorial.
The source code of the first unit test looks as follows:
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.MediaType;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
import java.nio.charset.Charset;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = {WebUnitTestContext.class})
@WebAppConfiguration
public class TodoControllerTest {
private static final MediaType APPLICATION_JSON_UTF8 = new MediaType(
MediaType.APPLICATION_JSON.getType(), MediaType.APPLICATION_JSON.getSubtype(),
Charset.forName("utf8")
);
private MockMvc mockMvc;
@Autowired
private ObjectMapper objectMapper;
@Autowired
private WebApplicationContext webAppContext;
@Before
public void setUp() {
mockMvc = MockMvcBuilders.webAppContextSetup(webAppContext).build();
}
@Test
public void add_EmptyTodoEntry_ShouldReturnHttpRequestStatusBadRequest() throws Exception {
TodoDTO addedTodoEntry = new TodoDTO();
mockMvc.perform(post("/api/todo")
.contentType(APPLICATION_JSON_UTF8)
.content(objectMapper.writeValueAsBytes(addedTodoEntry))
)
.andExpect(status().isBadRequest());
}
}The source code of the second unit test looks as follows:
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.MediaType;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
import java.nio.charset.Charset;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = {WebUnitTestContext.class})
@WebAppConfiguration
public class NoteControllerTest {
private static final MediaType APPLICATION_JSON_UTF8 = new MediaType(
MediaType.APPLICATION_JSON.getType(), MediaType.APPLICATION_JSON.getSubtype(),
Charset.forName("utf8")
);
private MockMvc mockMvc;
@Autowired
private ObjectMapper objectMapper;
@Autowired
private WebApplicationContext webAppContext;
@Before
public void setUp() {
mockMvc = MockMvcBuilders.webAppContextSetup(webAppContext).build();
}
@Test
public void add_EmptyNote_ShouldReturnHttpRequestStatusBadRequest() throws Exception {
NoteDTO addedNote = new NoteDTO();
mockMvc.perform(post("/api/note")
.contentType(APPLICATION_JSON_UTF8)
.content(objectMapper.writeValueAsBytes(addedNote))
)
.andExpect(status().isBadRequest());
}
}Both of these test classes declare a constant called APPLICATION_JSON_UTF8. This constant specifies the content type and the character set of the request. Also, it is clear that we need this constant in every test class which contains tests for our controller methods.
Does this mean that we should declare this constant in every such test class?
No!
We should move this constant to a non-instantiable class because of two reasons:
- It is relevant to multiple test classes.
- Moving it to a separate class makes it easier for us to write new tests for our controller methods and maintain our existing tests.
Let’s create a final WebTestConstants class, move the APPLICATION_JSON_UTF8 constant to that class, and add a private constructor to the created class.
The source code of the WebTestConstant class looks as follows:
import org.springframework.http.MediaType;
public final class WebTestConstants {
public static final MediaType APPLICATION_JSON_UTF8 = new MediaType(
MediaType.APPLICATION_JSON.getType(), MediaType.APPLICATION_JSON.getSubtype(),
Charset.forName("utf8")
);
private WebTestConstants() {
}
}After we have done this, we can remove the APPLICATION_JSON_UTF8 constants from our test classes. The source code of our new test looks as follows:
import com.fasterxml.jackson.databind.ObjectMapper;
import net.petrikainulainen.spring.jooq.config.WebUnitTestContext;
import net.petrikainulainen.spring.jooq.todo.dto.TodoDTO;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.web.WebAppConfiguration;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
import java.nio.charset.Charset;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(classes = {WebUnitTestContext.class})
@WebAppConfiguration
public class TodoControllerTest {
private MockMvc mockMvc;
@Autowired
private ObjectMapper objectMapper;
@Autowired
private WebApplicationContext webAppContext;
@Before
public void setUp() {
mockMvc = MockMvcBuilders.webAppContextSetup(webAppContext).build();
}
@Test
public void add_EmptyTodoEntry_ShouldReturnHttpRequestStatusBadRequest() throws Exception {
TodoDTO addedTodoEntry = new TodoDTO();
mockMvc.perform(post("/api/todo")
.contentType(WebTestConstants.APPLICATION_JSON_UTF8)
.content(objectMapper.writeValueAsBytes(addedTodoEntry))
)
.andExpect(status().isBadRequest());
}
}We have just removed duplicate code from our test classes and reduced the effort required to write new tests for our controllers. Pretty cool, huh?
If we change the value of a constant which is added to a constants class, this change effects to every test case which uses this constant. That is why we should minimize the number of constants which are added to a constants class.
Summary
We have now learned that constants can help us to write clean tests, and reduce the effort required to write new tests and maintain our existing tests. There are a couple of things which we should remember when we put the advice given in this blog post to practice:
- We must give good names to constants and constants classes. If we don’t do that, we aren’t leveraging the full potential of these techniques.
- We shouldn’t introduce new constants without figuring out what we want to achieve with that constant. The reality is often a lot more complex than the examples of this blog post. If we write code on autopilot, the odds are that we will miss the best solution to the problem at hand.
| Reference: | Writing Clean Tests – Beware of Magic from our JCG partner Petri Kainulainen at the Petri Kainulainen blog. |
