Misleading UnnecessaryStubbingException when Mockito is misused

[gysel]

In a certain case mockito fails to report a missing method on a mock instruction:

when(ormSupport).thenReturn(null);

The reproducible test can be found here:

https://github.com/gysel/mockito-problem/blob/master/src/test/java/ch/mgysel/mockito/MockitoProblem.java

Mockito fails the test with the following error message which is completely misleading:

org.mockito.exceptions.misusing.UnnecessaryStubbingException: 
Unnecessary stubbings detected in test class: MockitoProblem
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at ch.mgysel.mockito.MainComponent.<init>(MainComponent.java:10)
Please remove unnecessary stubbings or use 'silent' option. More info: javadoc for UnnecessaryStubbingException class.
	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:49)
	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:142)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

I would expect Mockito to fail the test with the following message:

org.mockito.exceptions.misusing.MissingMethodInvocationException: 
when() requires an argument which has to be 'a method call on a mock'.
For example:
    when(mock.getArticles()).thenReturn(articles);

Also, this error might show up because:
1. you stub either of: final/private/equals()/hashCode() methods.
   Those methods *cannot* be stubbed/verified.
   Mocking methods declared on non-public parent classes is not supported.
2. inside when() you don't call method on mock but on some other object.

	at ch.mgysel.mockito.MockitoProblem.test(MockitoProblem.java:31)

This is works then I remove the setUp() from the test.

[bric3]

Hi I can't reproduce the issue with recent mockito versions. Just to be sure I also checked the same version of your project : Mockito 2.5.5. And Mockito behaves correctly.

[gysel]

@bric3, can we please reopen this one?

I can still reproduce it with Mockito 2.5.5 as well as with 2.5.6...

clone gysel/mockito-problem and run mvn clean verify

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running ch.mgysel.mockito.MockitoProblemTest
Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.624 sec <<< FAILURE!
unnecessary Mockito stubbings(ch.mgysel.mockito.MockitoProblemTest)  Time elapsed: 0.032 sec  <<< ERROR!
org.mockito.exceptions.misusing.UnnecessaryStubbingException:
Unnecessary stubbings detected in test class: MockitoProblemTest
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at ch.mgysel.mockito.MainComponent.<init>(MainComponent.java:10)
Please remove unnecessary stubbings or use 'silent' option. More info: javadoc for UnnecessaryStubbingException class.
        at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:49)
        at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:142)
        at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
        at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
        at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)


Results :

Tests in error:
  unnecessary Mockito stubbings(ch.mgysel.mockito.MockitoProblemTest): (..)

Tests run: 2, Failures: 0, Errors: 1, Skipped: 0

I expect Mockito to throw a org.mockito.exceptions.misusing.MissingMethodInvocationException in this case!

Please note that this works in other cases. However the particular setUp() of this test uses seems to confuse Mockito.

[bric3]

OK I'll give it another look.

[mockitoguy]

I was able to reproduce the problem. @gysel, thank you very much for fantastic report and super easy way to reproduce the issue.

In this case, we cannot really do much. Mockito syntax is clean but it has trade-offs. Example:

mock1.foo();

//below, Mockito thinks that you're stubbing mock1.foo() method:
when(mock2).thenReturn(null);

In above example, Mockito is not aware that there is incorrect stubbing. doReturn() api for stubbing does not have this problem. When/If we change the syntax in the future (Mockito 3), we should be able to avoid this caveat.

I think there are 2 actionable TODOs:

• improve UnnecessaryStubbingException to hint at possible stubbing issue
• improve documentation, so that next time I can point the user to our docs

Thanks again for reporting! If you have ideas how to improve this scenario better, we would love to hear them.

[bradleywillard]

Why are these UnnecessaryStubbingException's happening? After upgrading from mockito-core-1.9.5 to mockito-core-2.9.0, tons of the exceptions occured. After removing the stubbings, the tests still run fine and show coverage and what not, but I fear some test's meaningfulness has been lost. And is it really safe to just blindly remove these unnecessary stubs? This is difficult to introspect when all the tests are passing fine. Thanks in advance for your insights.

[TimvdLippe]

@bradleywillard If a stubbing was defined but never invoked, the exception is thrown. As such, you can safely delete these stubbings and still have your tests pass. Hopefully that makes it a bit more clear!

[radsz]

@TimvdLippe On few occasions I saw stubbing done in @before function for all tests. Therefore, the unnecessary stubbing may be for one test but not the other. Blindly removing those stubs can potentially have bad impact on tests that were admittedly already not perfect. It is a pity that the unnecessary stubbing is not reported per test as this will allow to see if it makes sense to refactor tests and move stubbing from @before to the test itself.

[nmandya]

I have a scenario where I have a single test class with 4 test methods. I have a @beforeeach method where I am doing 2 stubbings using doReturn().when()...

Of the 4 tests in the class, 3 of them need both the stubbings, but the remaining 1 test needs only 1 of the 2 stubbings. When I run the test, I get an UnnecessaryStubbingException for that one test which does not need both the stubbings.

As per the the documentation of the UnnecessaryStubbingException class - "This means that it is ok to put default stubbing in a 'setup' method or in test class constructor. That default stubbing needs to be used at least once by one of the test methods.". However, I dont see it working that way. Can somebody clarify? Thanks.

[yaitskov]

I see that mockito 2.23.4 thinks wrong stubbing actually invoked.

[nani0102]

Removing @ExtendWith(MockitoExtension.class) annonation and adding MockitoAnnotations.initMocks(this) under init setup i.e using @before solves this problem.It worked for me

[indiv0]

Thanks for the workaround, but I think that just disables the check for unnecessary stubbings. Not a real solution.

[novicedev7291]

I have a similar case as @nmandya provided. What is the workaround for such a case?

@nmandya did you find a workaround for this or any better suggestion?

[bschelberg]

@novicedev7291 the best workaround I can find is to add Mockito.lenient(). before your when() or doReturn().

This seems to be a difference between the MockitoJUnitRunner and the MockitoExtension. I think this is a different issue, and is better tracked in #1540.