Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a private method in my test class that constructs a commonly used Bar object. The Bar constructor calls someMethod() method in my mocked object:

private @Mock Foo mockedObject; // My mocked object
...

private Bar getBar() {
  Bar result = new Bar(mockedObject); // this calls mockedObject.someMethod()
}

In some of my test methods I want to check someMethod was also invoked by that particular test. Something like the following:

@Test
public void someTest() {
  Bar bar = getBar();

  // do some things

  verify(mockedObject).someMethod(); // <--- will fail
}

This fails, because the mocked object had someMethod invoked twice. I don't want my test methods to care about the side effects of my getBar() method, so would it be reasonable to reset my mock object at the end of getBar()?

private Bar getBar() {
  Bar result = new Bar(mockedObject); // this calls mockedObject.someMethod()
  reset(mockedObject); // <-- is this OK?
}

I ask, because the documentation suggests resetting mock objects is generally indicative of bad tests. However, this feels OK to me.

Alternative

The alternative choice seems to be calling:

verify(mockedObject, times(2)).someMethod();

which in my opinion forces each test to know about the expectations of getBar(), for no gain.

share|improve this question

2 Answers 2

up vote 8 down vote accepted

I believe this is one of the cases where using reset() is ok. The test you are writing is testing that "some things" triggers a single call to someMethod(). Writing the verify() statement with any different number of invocations can lead to confusion.

  • atLeastOnce() allows for false positives, which is a bad thing as you want your tests to always be correct.
  • times(2) prevents the false positive, but makes it seem like you are expecting two invocations rather than saying "i know the constructor adds one". Further more, if something changes in the constructor to add an extra call, the test now has a chance for a false positive. And removing the call would cause the test to fail because the test is now wrong instead of what is being tested is wrong.

By using reset() in the helper method, you avoid both of these issues. However, you need to be careful that it will also reset any stubbing you have done, so be warned. The major reason reset() is discouraged is to prevent

bar = mock(Bar.class);
//do stuff
verify(bar).someMethod();
reset(bar);
//do other stuff
verify(bar).someMethod2();

This is not what the OP is trying to do. The OP, I'm assuming, has a test that verifies the invocation in the constructor. For this test, the reset allows for isolating this single action and it's effect. This one of the few cases with reset() can be helpful as. The other options that don't use it all have cons. The fact that the OP made this post shows that he is thinking about the situation and not just blindly utilizing the reset method.

share|improve this answer
1  
I wish Mockito provided resetInteractions() call to just forget the past interactions for the purpose of verify(..., times(...)) and keep the stubbing. This would make the test situations of {setup; act; verify;} that much easier to deal with. It would be {setup; resetInteractions; act; verify} –  Arkadiy Aug 14 at 15:34

Smart Mockito users hardly use reset feature because they know it could be a sign of poor tests. Normally, you don't need to reset your mocks, just create new mocks for each test method.

Instead of reset() please consider writing simple, small and focused test methods over lengthy, over-specified tests. First potential code smell is reset() in the middle of the test method.

Extracted from http://docs.mockito.googlecode.com/hg/org/mockito/Mockito.html#17

My advise is that you try to avoid using reset(). In my opinion, if you call twice to someMethod, that should be tested (maybe it is a database access, or other long process that you want to take care about).

If you really don't care about that, you could use:

verify(mockedObject, atLeastOnce()).someMethod();

Note that this last could cause a false result, if you call someMethod from getBar, and not after (this is a wrong behaviour, but the test will not fail).

share|improve this answer
2  
Yes, I've seen that exact quote (I linked to it from my question). Currently I'm yet to see a decent argument about why my example above is "bad". Can you supply one? –  Duncan Feb 27 '13 at 9:51
    
If you need to reset your mock objects, it seems like you are trying to test too much stuff in your test. You can split it in two tests, testing smaller stuff. In any case, I don't know why you are verifying inside the getBar method, it's difficult to track what you are testing. I recommend you to design your test thinking in what your class should do (if you must call someMethod exactly two times, at least one time, only one time, never, etc.), and do every verification in the same place. –  greuze Feb 27 '13 at 11:25
    
I've edited my question to highlight that the problem persists even if I don't call verify in my private method (which I agree, probably doesn't belong there). I welcome your comments on whether your answer would change. –  Duncan Feb 27 '13 at 13:07

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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