Writing Clean Tests - Divide and Conquer

A good unit test should fail for only one reason. This means that a proper unit test tests only one logical concept.

If we want to write clean tests, we have to identify those logical concepts, and write only one test case per logical concept.

This blog post describes how we can identify the logical concepts found from our tests, and split an existing unit test into multiple unit tests.

Pretty Clean Isn't Good Enough

Let’s start by taking a look at the source code of our 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 source code of this unit 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 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);

		assertThatUser(createdUserAccount)
			.hasEmail(REGISTRATION_EMAIL_ADDRESS)
			.hasFirstName(REGISTRATION_FIRST_NAME)
			.hasLastName(REGISTRATION_LAST_NAME)
			.isRegisteredUser()
			.isRegisteredByUsingSignInProvider(SOCIAL_SIGN_IN_PROVIDER);

		verify(repository, times(1)).findByEmail(REGISTRATION_EMAIL_ADDRESS);
		verify(repository, times(1)).save(createdUserAccount);
		verifyNoMoreInteractions(repository);
		verifyZeroInteractions(passwordEncoder);
	}
}

This unit test is pretty clean. After all, our test class, test method, and the local variables created inside the test method have descriptive names. We have also replaced magic numbers with constants and created domain-specific languages for creating new objects and writing assertions.

And yet, we can make this test even better.

The problem of this unit test is that it can fail for more than one reason. It can fail if

  1. Our service method doesn’t check that the email address entered to the registration form is not found from our database.
  2. The information of the persisted User object doesn’t match with the information entered to the registration form.
  3. The information of the returned User object isn’t correct.
  4. Our service method creates a password for the user by using the PasswordEncoder object.

In other words, this unit test tests four different logical concepts, and this causes the following problems:

  • If this test fails, we don't necessarily know why it failed. This means that we have to read the source code of the unit test.
  • The unit test is a bit long which makes it somewhat hard to read.
  • It is hard to describe the expected behavior. This means that it is very hard to figure great names for our test methods.
We can identify the logical concepts covered by a single unit test by identifying the situations when that unit test will fail.

That is why we need to split this test into four unit tests.

One Test, One Point of Failure

Our next step is to split our unit test into four new unit tests and ensure that each of them tests a single logical concept. We can do this by writing the following unit tests:

  1. We need to ensure that our service method checks that the email address given by the user is unique.
  2. We need to verify that information of the persisted User object is correct.
  3. We need to ensure that the information of the returned User object is correct.
  4. We need to verify that our service method doesn’t create an encoded password for a user who uses social sign in provider.

After we have written these unit tests, the source code of our test class looks as follows:

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
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 net.petrikainulainen.spring.social.signinmvc.user.model.UserAssert.assertThat;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
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 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_ShouldCheckThatEmailIsUnique() 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);

        registrationService.registerNewUserAccount(registration);

        verify(repository, times(1)).findByEmail(REGISTRATION_EMAIL_ADDRESS);
    }

    @Test
    public void registerNewUserAccount_SocialSignInAndUniqueEmail_ShouldSaveNewUserAccountAndSetSignInProvider() 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);

        registrationService.registerNewUserAccount(registration);

        ArgumentCaptor<User> userAccountArgument = ArgumentCaptor.forClass(User.class);
        verify(repository, times(1)).save(userAccountArgument.capture());

        User createdUserAccount = userAccountArgument.getValue();

        assertThatUser(createdUserAccount)
                .hasEmail(REGISTRATION_EMAIL_ADDRESS)
                .hasFirstName(REGISTRATION_FIRST_NAME)
                .hasLastName(REGISTRATION_LAST_NAME)
                .isRegisteredUser()
                .isRegisteredByUsingSignInProvider(SOCIAL_SIGN_IN_PROVIDER);
    }


    @Test
    public void registerNewUserAccount_SocialSignInAndUniqueEmail_ShouldReturnCreatedUserAccount() 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);

        assertThatUser(createdUserAccount)
                .hasEmail(REGISTRATION_EMAIL_ADDRESS)
                .hasFirstName(REGISTRATION_FIRST_NAME)
                .hasLastName(REGISTRATION_LAST_NAME)
                .isRegisteredUser()
                .isRegisteredByUsingSignInProvider(SOCIAL_SIGN_IN_PROVIDER);
    }

    @Test
    public void registerNewUserAccount_SocialSignInAnUniqueEmail_ShouldNotCreateEncodedPasswordForUser() 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);

        registrationService.registerNewUserAccount(registration);

        verifyZeroInteractions(passwordEncoder);
    }
}

The obvious benefit of writing unit tests which test only one logical concept is that it easy to know why the test failed. However, this approach has two other benefits as well:

  • It is easy to specify the expected behavior. This means that it is easier to figure out good names for our test methods.
  • Because these unit tests are considerably shorter than the original unit test, it is easier to figure out the requirements of the tested method / component. This helps us to transform our tests into executable specifications.

Let's move on and summarize what we learned from this blog post.

Summary

We have now successfully splitted our unit test into four smaller unit tests which test a single logical concept. This blog post has taught us two things:

  • We learned that we can identify the logical concepts covered by a single unit test by identifying the situations when that test will fail.
  • We learned that writing unit tests which test only one logical concept helps us to write transform our test cases into executable specifications, which identity the requirements of the tested method / component.
6 comments… add one
  • denisazevedo Sep 16, 2014 @ 21:15

    Hey Petri,
    Thank you for these brilliants post in this series.

    I was wondering why you verify zero interactions on password encoder in your last method:
    `registerNewUserAccount_SocialSignInAnUniqueEmail_ShouldNotCreateEncodedPasswordForUser()`
    Shouldn't have an interaction to encode the password before saving the new user?

    • Petri Sep 16, 2014 @ 21:45

      Hi,

      That is a good question.

      These particular tests are originally from the example application of my Spring Social tutorial. That example application encodes the password only if the user creates his/her user account by using the "normal" registration. If the user creates a user account by using a social sign in provider, the password is not encoded (or saved to the database) because the user does not need it (he/she can sign in by using the same social sign in provider).

  • Charles Roth Mar 13, 2015 @ 17:46

    I've been really enjoying this series so far, and I've linked it to the wiki pages that I use to help teach effective unit-testing in our company.

    But this is the first one that I want to push back on. I think we have another trade-off here: between the length of tests, and the length of test *classes*. In the example given (which illustrates the principle very well), I find the total result MORE complicated than the original.

    The idea that a test tests a SINGLE thing is an assumption, and one that I do not necessarily accept. I prefer to think of it in terms of coupling. If the coupling between two things that are being tested is very close, in many cases I'd RATHER have them in a single test. If they are in two different tests, I lose the information that these "things" are tightly related to each other.

    Plus it just makes for more code. And all other things being equal (which they often are not, admittedly), more code is harder to read, understand, and add to.

    That being said, I would agree that, 'way too often, people put TOO MANY things in a single test. But that does not necessarily prove that ONE THING in a test is always the right way.

    • Petri Mar 14, 2015 @ 12:31

      Hi Charles,

      Thank you for your kinds words (and of course links). I really appreciate them.

      But this is the first one that I want to push back on.

      Great :)

      I think we have another trade-off here: between the length of tests, and the length of test *classes*. In the example given (which illustrates the principle very well), I find the total result MORE complicated than the original.

      I agree (at least in some degree). I write this blog post about nine months ago and the principle I described in this blog post seemed very good at the beginning. However, soon (well, to be honest, it took me eight months) I realized that this principle has few serious problems which you described when you said:

      Plus it just makes for more code. And all other things being equal (which they often are not, admittedly), more code is harder to read, understand, and add to.

      I agree! It took me eight months to accept that my idea is not perfect, but now that I have done it, it is clear that using this principle will force us to write a lot of duplicate code (mock configuration, creation of objects that are used in our tests, etc) => changing existing features is very slow because we have to change (too) many unit tests.

      he idea that a test tests a SINGLE thing is an assumption, and one that I do not necessarily accept. I prefer to think of it in terms of coupling. If the coupling between two things that are being tested is very close, in many cases I’d RATHER have them in a single test. If they are in two different tests, I lose the information that these “things” are tightly related to each other.

      A good point. The only that links different tests together is the method name, but because the method names tend to become quite long, they might not describe what is actually tested.

      I am going to write a new blog post that addresses the problems you mentioned and describes the method that I use to solve them (I would love to hear your thoughts about it after I have published it).

Leave a Reply