Writing Clean Tests – New Considered Harmful

Petri KainulainenMay 28th, 2014Last Updated: May 28th, 2014

5 63 10 minutes read

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 fourth part of my tutorial which describes how we can write clean tests. This time we will learn why we will should not create objects in our test methods by using the new keyword. We will also learn how we can replace the new keyword with factory methods and test data builders.

New Is Not the New Black

During this tutorial we have been refactoring a unit test which ensures that the registerNewUserAccount(RegistrationForm userAccountData) method of the RepositoryUserService class works as expected when a new user account is created by using a unique email address and a social sign in provider.

The RegistrationForm class is a data transfer object (DTO), and our unit tests sets its property values by using setter methods. The source code of our unit test looks as follows (the relevant code is highlighted):

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);
    }
}

So, what is the problem? The highlighted part of our unit test is short and it is relatively easy to read. In my opinion, the biggest problem of this code is that it is data centric. It creates a new RegistrationForm object and sets the property values of the created object, but it doesn’t describe the meaning of these property values.

If we create new objects in the test method by using the new keyword, our tests become harder to read because:

The reader has to know the different states of the created object. For example, if we think about our example, the reader has to know that if we create a new RegistrationForm object and set the property values of the email, firstName, lastName, and signInProvider properties, it means that the object is a registration which is made by using a social sign in provider.
If the created object has many properties, the code which creates it, litters the source code of our tests. We should remember that even though we need these objects in our tests, we should focus on describing the behavior of the tested method / feature.

Although it isn’t realistic to assume that we can completely eliminate these drawbacks, we should do our best to minimize their effect and make our tests as easy to read as possible.

Let’s find out how we can do this by using factory methods.

Using Factory Methods

When we create new objects by using factory methods, we should name the factory methods and their method parameters in a such way that it makes our code easier to read and write. Let’s take a look at two different factory methods and see what kind of an effect they have to the readability of our unit test.

These factory methods are typically added to an object mother class because often they are useful to more than one test class. However, because I want to keep things simple, I will add them directly to the test class.

The name of the first factory method is newRegistrationViaSocialSignIn(), and it has no method parameters. After we have added this factory method to our test class, the source of our unit test looks as follows (the relevant parts are highlighted):

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 = newRegistrationViaSocialSignIn();

        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);
    }
   
    private RegistrationForm newRegistrationViaSocialSignIn() {
        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);

        return registration;
    }
}

The first factory method has the following consequences:

The part of our test method, which creates the new RegistrationForm object, is a lot cleaner than before and the name of the factory method describes the state of the created RegistrationForm object.
The configuration of our mock object is harder to read because the value of the the email property is “hidden” inside our factory method.
Our assertions are harder to read because the property values of the created RegistrationForm object are “hidden” inside our factory method.

If we would use the object mother pattern, the problem would be even bigger because we would have to move the related constants to the object mother class.

I think that it is fair to say that even though the first factory method has its benefits, it has serious drawbacks as well.

Let’s see if the second factory method can eliminate those drawbacks.

The name of the second factory method is newRegistrationViaSocialSignIn(), and it takes the email address, first name, last name, and social sign in provider as method parameters. After we have added this factory method to our test class, the source of our unit test looks as follows (the relevant parts are highlighted):

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 = newRegistrationViaSocialSignIn(REGISTRATION_EMAIL_ADDRESS,
                                                                REGISTRATION_FIRST_NAME,
                                                                REGISTRATION_LAST_NAME,
                                                                SOCIAL_MEDIA_SERVICE
        );

        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);
    }
   
    private RegistrationForm newRegistrationViaSocialSignIn(String emailAddress, String firstName, String lastName, SocialMediaService signInProvider) {
        RegistrationForm registration = new RegistrationForm();
   
        registration.setEmail(emailAddress);
        registration.setFirstName(firstName);
        registration.setLastName(lastName);
        registration.setSignInProvider(signInProvider);

        return registration;
    }
}

The second factory method has the following consequences:

The part of our test method, which creates the new RegistrationForm object, is a bit messier than the same code which uses the first factory method. However, it is still cleaner than the original code because the name of the factory method describes the state of the created object.
It seems to eliminate the drawbacks of the first factory method because the property values of the created object are not “hidden” inside the factory method.

Seems cool, right?

It would be really easy to think that all is well in the paradise, but that is not the case. Although we have seen that factory methods can make our tests more readable, the thing is that they are a good choice only when the following conditions are met:

The factory method doesn’t have too many method parameters. When the number of method parameter grows, our tests become harder to write and read. The obvious question is: how many method parameters a factory method can have? Unfortunately it is hard to give an exact answer to that question but I think that using a factory method is a good choice if the factory method has only a handful of method parameters.
The test data doesn’t have too much variation. The problem of using factory methods is that a single factory method is typically suitable for one use case. If we need to support N use cases, we need to have N factory methods. This is a problem because over time our factory methods become bloated, messy, and hard to maintain (especially if we use the object mother pattern).

Let’s find out if test data builders can solve some of these problems.

Using Test Data Builders

A test data builder is a class which creates new objects by using the builder pattern. The builder pattern described in Effective Java has many benefits, but our primary motivation is to provide a fluent API for creating the objects used in our tests.

We can create a test data builder class which creates new RegistrationForm objects by following these steps:

Create a RegistrationFormBuilder class.
Add a RegistrationForm field to the created class. This field contains a reference to the created object.
Add a default constructor to the created class and implement it by creating a new RegistrationForm object.
Add methods which are used to set the property values of the created RegistrationForm object. Each method sets the property value by calling the correct setter method and returns a reference to the RegistrationFormBuilder object. Remember that the method names of these methods can either make or break our DSL.
Add a build() method to the created class and implement it by returning the created RegistrationForm object.

The source code of our test data builder class looks as follows:

public class RegistrationFormBuilder {

    private RegistrationForm registration;

    public RegistrationFormBuilder() {
        registration = new RegistrationForm();
    }

    public RegistrationFormBuilder email(String email) {
        registration.setEmail(email);
        return this;
    }

    public RegistrationFormBuilder firstName(String firstName) {
        registration.setFirstName(firstName);
        return this;
    }

    public RegistrationFormBuilder lastName(String lastName) {
        registration.setLastName(lastName);
        return this;
    }

    public RegistrationFormBuilder isSocialSignInViaSignInProvider(SocialMediaService signInProvider) {
        registration.setSignInProvider(signInProvider);
        return this;
    }

    public RegistrationForm build() {
        return registration;
    }
}

After we have modified our unit test to use the new test data builder class, its source code looks as follows (The relevant part is highlighted):

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 RegistrationFormBuilder()
            .email(REGISTRATION_EMAIL_ADDRESS)
            .firstName(REGISTRATION_FIRST_NAME)
            .lastName(REGISTRATION_LAST_NAME)
            .isSocialSignInViaSignInProvider(SOCIAL_SIGN_IN_PROVIDER)
            .build();

        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);
    }
}

As we can see, test data builders have the following benefits:

The code which creates new RegistrationForm objects is both easy to read and write. I am a big fan of fluent APIs, and I think that this code is both beautiful and elegant.
The builder pattern ensures that the variation found from our test data is no longer a problem because we can simply add new methods to the test data builder class.
The configuration of our mock object and our assertions are easy to read because the constants are visible in our test method and our DSL emphasizes the meaning of each property value.

So, should we use the builder pattern for everything?

NO!

We should use test data builders only when it makes sense. In other words, we should use them when:

We have set more than a handful of property values.
Our test data has a lot of variation.

The builder pattern is a perfect choice if one of these conditions is true. The reason for this is that we can create a domain-specific language by naming the setter-like methods of the builder class. This makes our tests easy to read and write even if we would have create a lot of different objects and set a lot of property values.

That is the power of the builder patten.

If you want to learn more about fluent APIs, you should read the following articles:

That is all for today. Let’s move on and summarize what we learned from this blog post.

Summary

We learned why it is a bad idea to create objects in the test method by using the new keyword, and we learned two different ways to create the objects which are used in our tests.

To be more specific, this blog post has taught us three things:

It is a bad idea to create the required objects in the test method by using the new keyword because it makes our tests messy and hard to read.
If we have to set only a handful of property values and our test data doesn’t have a lot of variation, we should create the required object by using a factory method.
If we have to set a lot of property values and / or our test data has a lot of variation, we should create the required object by using a test data builder.

Reference:

Writing Clean Tests – New Considered Harmful from our JCG partner Petri Kainulainen at the Petri Kainulainen blog.