Monday, August 12, 2013

Unit tests: When to Mock, When to Stub

Most of my recent development has been in Java, where we use JUnit4 and Mockito for unit tests. Having transitioned from EasyMock for mocking, I see a lot of the same mistakes when switching to Mockito.

Verifying implementation details instead of outcomes

If you've done a decent amount of mocking, you've probably seen something like the following:


public class ColorPreferenceChanger {

  private final UserService userService;

  @Inject
  ColorPreferenceChanger(UserService userService) {
    this.userService = userService;
  }
  
  /**
   * Updates the logged in user's color preference to the
   * given color.
   */
  public setColorPref(Color color) {
    User user = userService.getUser();
    userService.setColorPref(user, color);
  }
}

@RunWith(JUnit4.class)
public class ColorPreferenceChangerTest {

  @Mock private UserService userService;

  @Before
  public void setUp() {
    MockitoAnnotations.initMocks();
  } 

  @Test
  public void updatePreferenceShouldSetColorPreference() {

    when(userService.getUser()).thenReturn(fakeUser);

    colorPreferenceChanger.setFavoriteColor(Color.BLUE);

    verify(userService).getUser();
    verify(userService).setColorPref(fakeUser, Color.BLUE);
    verifyNoMoreInteractions(userService);

  }

}

The verifications in bold are verifying unnecessary implementation details and increase the brittleness of the test. You can think of your unit tests as a readable demonstration that your code behaves as expected under certain situations. Verifying that userService.getUser() was invoked proves exactly nothing about whether the method behaved correctly. What if the user was fetched and then ignored? What if the setColorPref method was never invoked after the user was fetched? This is a really common pattern among developers who are accustomed to writing EasyMock or other old mocking frameworks where stubbing and verification are done in (nearly) the same way.

Generally speaking, stubbing and verifying the same call is a code smell. Stubbing (when/then calls) are used to set up the environment for your test. In Behavior Driven Development (BDD), tests are split into 3 sections:
  1. Given
  2. When
  3. Then
From a mocking perspective, this corresponds with
  1. Given (this stubbed service)
  2. When (I invoke my method)
  3. Then (these state-changing methods should have been invoked)
Taking our example above, here are the given/when/then sections:

@Test
public void setFavoriteColorShouldUpdateColorPreference() {

  // Given
  when(userService.getUser()).thenReturn(fakeUser);

  // When
  colorPreferenceChanger.setFavoriteColor(Color.BLUE);

  // Then
  verify(userService).setColorPref(fakeUser, Color.BLUE);

}

Because we verify that that the correct color preference was set with fakeUser, verifying the get interaction is completely redundant! The test would fail if the setColorPref method was invoked with any other user.


But I won't know if the stubbed method was invoked with the correct arguments!


If you stub for the values you expect, you will know, because your test won't pass without the missing data (otherwise, why are you stubbing it?). In other words, stub for specific values. Instead of:

when(userService.getDocument(anyLong()))
    .thenReturn(fakeDocument);

Do:

when(userService.getDocument(1234))
    .thenReturn(fakeDocument);

Frameworks like Mockito will return null or empty for unexpected invocations and your test will fail with a NullPointerException or similar error.


That will be really hard to troubleshoot when the test fails!


When the test fails, you can add in the verify() call as a troubleshooting measure, similar to how you might add a println statement in the middle of your code to quickly troubleshoot something. This will give you a more helpful diagnostic message about incorrect arguments. But it doesn't belong in the checked-in test because it doesn't verify any meaningful output of your method. It confuses the reader of your test by mixing verification of state-changing methods that matter with interaction/getter methods that don't.


What's wrong with verifyNoMoreInteractions()?


As mentioned in the Mockito docs, Methods like verifyNoMoreInteractions() or verifyZeroInteractions() can be overused, leading to brittle and hard-to-maintain tests. If you were to modify the class you're testing to call another method, you would need to update all existing tests to stub and verify the new method - even tests that have nothing to do with the new method! For instance, if we were to modify the ColorPreferenceChanger to send an email notification to a user when their preference has changed, we would need to update all existing tests to take this into account. Ideally, we should just need to add a single new test or modify an existing test to verify that the email is sent. Mockito intentionally uses nice mocks by default to make it easy to test specific slices of behavior without over-specifying your test. Think twice before modifying this philosophy if you want to avoid brittle tests.


Something bad could happen if the mock class is touched!


There are some methods that you want to make sure are never invoked. For this, Mockito provides verify/never.

verify(userService, never()).deleteUserAccount(fakeUser);

For the developer reading your unit test, this calls out the problematic method far more clearly, and better documents your intent.

However, consider that you may not need the negative verification at all! Negative verifications come from a place of fear. I find them useful when the outcome is an actual possibility for the given class. For instance, I would only verify that deleteUserAccount was never invoked if there was an actual code path in the class that would result in that outcome. Just because deleteUserAccount is an available method on the userService does not mean that I should verify that it was never invoked, if my class has no reason to ever invoke it. Unit tests are not meant to handle adversarial situations where you assume someone is writing harmful code for your class. Throwing in verifyNoMoreInteractions for your mock as a trip-wire increases the brittleness of the test.

That isn't to say that it's not a useful tool - just don't reach for it for every test.

It would be remiss of me not to mention Martin Fowler's excellent paper: Mocks Aren't Stubs.

No comments:

Post a Comment