2

I'm new to unit testing and trying to learn the proper style. I like to setup the object that I am testing so that I can test it as if it were in use instead of only testing a newly constructed object. I can't test removing things from an object that is empty, as many objects are when constructed.

Take the following for an example where ObservedList is being tested and ListListener is a necessary class that is being mocked.

public final class ObservedListTest {
  private ListListener<Integer> listener;
  private ObservedList<Integer> list;
  @BeforeMethod public void setup() {
    listener = mock(ListListener.class);
    list = new ObservedList<Integer>(listener);
    list.addAll(Arrays.asList(1,2,3));
    reset(listener);
  }
  @Test public void addFirst() {
    list.add(0, -1);
    verify(listener).listEdited(list, 0, 1, Collections.<Integer>emptyList());
    verifyNoMoreInteractions(listener);
  }
  @Test void addAtEnd() {
    list.add(9);
    verify(listener).listEdited(list, 3, 4, Collections.<Integer>emptyList());
    verifyNoMoreInteractions(listener);
  }
  @Test void removeMiddle() {
    list.remove(Integer.valueOf(2));
    verify(listener).listEdited(list, 1, 1, Collections.singletonList(2));
    verifyNoMoreInteractions(listener);
  }
}

As a novice this seems to work well to me, but I know that it's bad practice because it uses the reset method. I call reset because I don't want the actual tests to get confused due to interactions that happened in the setup.

The javadoc for reset doesn't even get around to telling you what the method does because it is so busy telling you that you shouldn't use it. Ordinarily I'd simply take that advice and avoid reset by removing my setup method and adjusting my tests to look more like this:

  @Test void removeMiddle() {
    listener = mock(ListListener.class);
    list = new ObservedList<Integer>(listener);
    list.addAll(Arrays.asList(1,2,3));
    list.remove(Integer.valueOf(2));
    InOrder inOrder = inOrder(listener);
    inOrder.verify(listener).listEdited(list, 0, 3,
      Collections.<Integer>emptyList());
    inOrder.verify(listener).listEdited(list, 1, 1,
      Collections.singletonList(2));
    verifyNoMoreInteractions(listener);
  }

This also seems to work well to me. The problem is that the documentation for the reset method says:

Instead of reset() please consider writing simple, small and focused test methods over lengthy, over-specified tests.

I respect that Mockito is designed to encourage good style in unit tests and I want to learn from it, but I'm having a hard time sorting out what message it is trying to send me. When I eliminate reset from my tests, my tests get complex, longer and less focused, so obviously I'm doing it wrong.

What does doing it right look like?

2 Answers 2

2

Remove the reset. Remove the verifyNoMoreInteractions. Problem solved. Your tests are simpler, shorter, and more focused.

4
  • That would make my tests far more permissive. Isn't that declaring that ObservedList is allowed to send incorrect messages to ListListener? I don't want to allow that. If permissiveness were a good thing I could eliminate the test completely, permit everything, and that would be simplest of all, but I really would like to actually test this thing.
    – Geo
    Oct 18, 2013 at 4:51
  • 1
    @Geo, Ryan is describing the testing philosophy that Mockito was designed for, which favors flexible, loose tests over brittle, tightly-specified tests. Typically, unless you're testing listeners or callback behavior, extra method calls shouldn't cause a test to fail: By the time you expect and verify every call, you've duplicated the implementation inside-out within the test. Oct 18, 2013 at 6:54
  • @JeffBowman, does this mean that Mockito shouldn't be used for mocking listeners or callbacks? It seems quite intent upon only being used for its own testing philosophy, but I hesitate to use a mix of multiple mocking tools.
    – Geo
    Oct 18, 2013 at 7:33
  • 1
    @Geo, Mockito is fine for mocking callbacks and listeners--that's where you'd more likely see verifyZeroInteractions or verifyNoMoreInteractions, because method interactions (or lack thereof) are specifically a documented part of the API of the system under test. My point is that those methods should be rarely used outside of that. Oct 18, 2013 at 8:29
2

Mockito 2.1 added a clearInvocations method, which can be used like this:

public final class ObservedListTest {
  private ListListener<Integer> listener;
  private ObservedList<Integer> list;

  @BeforeMethod public void setup() {
    listener = mock(ListListener.class);
    list = new ObservedList<Integer>(listener);
    list.addAll(Arrays.asList(1,2,3));
    clearInvocations(listener);
  }

  @Test public void addFirst() {
    list.add(0, -1);
    verify(listener, only()).listEdited(list, 0, 1, Collections.<Integer>emptyList());
  }

  @Test void addAtEnd() {
    list.add(9);
    verify(listener, only()).listEdited(list, 3, 4, Collections.<Integer>emptyList());
  }

  @Test void removeMiddle() {
    list.remove(Integer.valueOf(2));
    verify(listener, only()).listEdited(list, 1, 1, Collections.singletonList(2));
  }
}

Note that clearInvocations javadoc also states:

Try to avoid this method at all costs. Only clear invocations if you are unable to efficiently test your program.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.