Repository: mockito/mockito
Issue: Deep stubbing and UnnecessaryStubbingException
Labels: bug

Mocks that return deep stubs sometimes cause UnnecessaryStubbingExceptions to be thrown, when I don't think that they should. Here's a small self-contained example.

```
package some.package;

import org.jboss.netty.channel.ChannelFuture;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class MockitoDeepStubsTest {

    private final ClassUnderTest classUnderTest = new ClassUnderTest();

    @Mock(answer = Answers.RETURNS_DEEP_STUBS) private ChannelFuture mockChannelFuture;

    @Test
    public void shouldAllowDeepStubsUnnecessaryStubbing() {
        classUnderTest.methodUnderTest(mockChannelFuture);
    }

    public static class ClassUnderTest {

        public void methodUnderTest(ChannelFuture future) {
            future.getChannel().close();
        }

    }
}
```
Running this test prints the following output:
```
org.mockito.exceptions.misusing.UnnecessaryStubbingException: 
Unnecessary stubbings detected in test class: MockitoDeepStubsTest
Clean & maintainable test code requires zero unnecessary code.
Following stubbings are unnecessary (click to navigate to relevant line of code):
  1. -> at some.package.MockitoDeepStubsTest$ClassUnderTest.methodUnderTest(MockitoDeepStubsTest.java:25)
Please remove unnecessary stubbings. More info: javadoc for UnnecessaryStubbingException class.

	at org.mockito.internal.exceptions.Reporter.formatUnncessaryStubbingException(Reporter.java:838)
	at org.mockito.internal.junit.UnnecessaryStubbingsReporter.validateUnusedStubs(UnnecessaryStubbingsReporter.java:30)
	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:45)
	at org.mockito.runners.MockitoJUnitRunner.run(MockitoJUnitRunner.java:104)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:117)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:262)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:84)
	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 com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
```

Versions: Mockito 2.2.8
JDK: OpenJDK 1.8.0_111
OS: CentOS 7
Netty: 3.10.6.Final

I know that I can suppress the error with the Silent runner, but I like the unnecessary stubbing checking and don't really want to disable it. For now, we're working around it, but this doesn't seem like correct behaviour.

Issue comments:
Fantastic bug report: reproducible sample, clear motivation. Thanks! I'm looking at it now.

I was able to repro this with a smaller example that does not use netty. It's a bug that needs to be fixed. Thank you for reporting!

``` java
@RunWith(MockitoJUnitRunner.class)
public class MockitoDeepStubsTest {

    private final ClassUnderTest classUnderTest = new ClassUnderTest();

    @Mock(answer = Answers.RETURNS_DEEP_STUBS) private ChannelFuture mockChannelFuture;

    @Test
    public void shouldAllowDeepStubsUnnecessaryStubbing() {
        classUnderTest.methodUnderTest(mockChannelFuture);
    }

    public static class ClassUnderTest {
        void methodUnderTest(ChannelFuture future) {
            future.getChannel().close();
        }
    }

    interface ChannelFuture {
        Channel getChannel();
    }

    interface Channel {
        void close();
    }
}
```

@bschelberg, thank you for reporting! Stay tuned for Mockito 2.3.0 that will come with even better support for cleaner and easier to debug tests!

Pull request review comments:
Could you add one more call after `getFoo()` to make sure it does not crash on a second invocation? Just to make sure and base on the original bug report.
Absolutely, good idea.

(My test does reproduce the incorrect the behavior).

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/stubbing/InvocationContainerImpl.java b/src/main/java/org/mockito/internal/stubbing/InvocationContainerImpl.java
index f283f5227c..5fd76c096e 100644
--- a/src/main/java/org/mockito/internal/stubbing/InvocationContainerImpl.java
+++ b/src/main/java/org/mockito/internal/stubbing/InvocationContainerImpl.java
@@ -53,7 +53,10 @@ public void addConsecutiveAnswer(Answer answer) {
         addAnswer(answer, true);
     }
 
-    public void addAnswer(Answer answer, boolean isConsecutive) {
+    /**
+     * Adds new stubbed answer and returns the invocation matcher the answer was added to.
+     */
+    public StubbedInvocationMatcher addAnswer(Answer answer, boolean isConsecutive) {
         Invocation invocation = invocationForStubbing.getInvocation();
         mockingProgress().stubbingCompleted();
         AnswersValidator answersValidator = new AnswersValidator();
@@ -65,6 +68,7 @@ public void addAnswer(Answer answer, boolean isConsecutive) {
             } else {
                 stubbed.addFirst(new StubbedInvocationMatcher(invocationForStubbing, answer));
             }
+            return stubbed.getFirst();
         }
     }
 
diff --git a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java
index 9367a89091..84cd9574bf 100644
--- a/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java
+++ b/src/main/java/org/mockito/internal/stubbing/defaultanswers/ReturnsDeepStubs.java
@@ -68,10 +68,16 @@ private Object deepStub(InvocationOnMock invocation, GenericMetadataSupport retu
         }
 
         // record deep stub answer
-        return recordDeepStubAnswer(
+        StubbedInvocationMatcher stubbing = recordDeepStubAnswer(
                 newDeepStubMock(returnTypeGenericMetadata, invocation.getMock()),
                 container
         );
+
+        // deep stubbing creates a stubbing and immediately uses it
+        // so the stubbing is actually used by the same invocation
+        stubbing.markStubUsed(stubbing.getInvocation());
+
+        return stubbing.answer(invocation);
     }
 
     /**
@@ -110,9 +116,9 @@ private ReturnsDeepStubs returnsDeepStubsAnswerUsing(final GenericMetadataSuppor
         return new ReturnsDeepStubsSerializationFallback(returnTypeGenericMetadata);
     }
 
-    private Object recordDeepStubAnswer(final Object mock, InvocationContainerImpl container) {
-        container.addAnswer(new DeeplyStubbedAnswer(mock), false);
-        return mock;
+    private StubbedInvocationMatcher recordDeepStubAnswer(final Object mock, InvocationContainerImpl container) {
+        DeeplyStubbedAnswer answer = new DeeplyStubbedAnswer(mock);
+        return container.addAnswer(answer, false);
     }
 
     protected GenericMetadataSupport actualParameterizedType(Object mock) {


Test patch:
diff --git a/src/test/java/org/mockitousage/junitrunner/DeepStubbingWithJUnitRunnerTest.java b/src/test/java/org/mockitousage/junitrunner/DeepStubbingWithJUnitRunnerTest.java
new file mode 100644
index 0000000000..77ea85b879
--- /dev/null
+++ b/src/test/java/org/mockitousage/junitrunner/DeepStubbingWithJUnitRunnerTest.java
@@ -0,0 +1,41 @@
+package org.mockitousage.junitrunner;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DeepStubbingWithJUnitRunnerTest {
+
+    private final SomeClass someClass = new SomeClass();
+
+    @Mock(answer = Answers.RETURNS_DEEP_STUBS) private Root root;
+
+    @Test
+    public void deep_stubs_dont_trigger_unnecessary_stubbing_exception() {
+        //when
+        someClass.someMethod(root);
+
+        //then unnecessary stubbing exception is not thrown
+    }
+
+    public static class SomeClass {
+        void someMethod(Root root) {
+            root.getFoo().getBar();
+        }
+    }
+
+    interface Root {
+        Foo getFoo();
+    }
+
+    interface Foo {
+        Bar getBar();
+    }
+
+    interface Bar {
+
+    }
+}
\ No newline at end of file