Repository: mockito/mockito
Issue: Wanted but not invoked on 2.12.0, but not on 2.11.0
Labels: bug

After updating Mockito from 2.11.0 to 2.12.0 the following test fails with `Wanted but not invoked`.

The weird thing is this happens only when running tests using Gradle. Android Studio runs the same test as successful. Just like #1183.

```java
package package.redacted;

import org.junit.Test;
import org.mockito.Mockito;

public class ViewModelTest {

    class View {
        private Runnable actionCallback = null;

        void callAction(Runnable callback) {
            actionCallback = callback;
        }

        void simulateActionCalled() {
            if (actionCallback != null) {
                actionCallback.run();
            }
        }

        void showResult() {
        }
    }

    class ViewModel {

        public ViewModel(final View view) {
            view.callAction(new Runnable() {
                @Override
                public void run() {
                    view.showResult();
                }
            });
        }
    }

    @Test public void test() {
        View view = Mockito.spy(new View());
        ViewModel viewModel = new ViewModel(view);

        view.simulateActionCalled();

        Mockito.verify(view).showResult();
    }
}
```
```
Wanted but not invoked:
view.showResult();
-> at package.redacted.ViewModelTest$View.showResult(ViewModelTest.java:22)

However, there were exactly 2 interactions with this mock:
view.callAction(
    package.redacted.ViewModelTest$ViewModel$1@26645040
);
-> at package.redacted.ViewModelTest$ViewModel.<init>(ViewModelTest.java:28)

view.simulateActionCalled();
-> at package.redacted.ViewModelTest.test(ViewModelTest.java:41)


	at package.redacted.ViewModelTest$View.showResult(ViewModelTest.java:22)
	at package.redacted.ViewModelTest.test(ViewModelTest.java:43)
	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.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:114)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:57)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:66)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
	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.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
	at com.sun.proxy.$Proxy1.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:108)
	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.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:146)
	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:128)
	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:404)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
	at java.lang.Thread.run(Thread.java:748)
```

Issue comments:
I'm observing a similar issue. Only happens with `mockito-inline`. Don't have time right now to distill the affected tests into a minimal reproduction case. Maybe @raphw has a hunch.
Alright, decided to look for a minimal reproduction case after all. This is what I came up with:
```java
package com.example

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;

import org.testng.annotations.Test;

public final class MockitoIssue1254Test {
    @Test
    public void test() {
        Dummy d = spy(new Dummy());
        d.foo();
        verify(d).bar(eq("baz"));
    }

    static class Dummy {
        public void foo() {
            bar("baz");
        }

        // Also fails if public.
        void bar(String s) {}
    }
}
```

Causes:
```
Wanted but not invoked:
dummy.bar("baz");
-> at com.example.MockitoIssue1254Test$Dummy.bar(MockitoIssue1254Test.java:23)

However, there was exactly 1 interaction with this mock:
dummy.foo();
-> at com.example.MockitoIssue1254Test.test(MockitoIssue1254Test.java:13)
```

This test does pass with Mockito 2.11.0. It also passes with Mockito 2.12.0, provided `mockito-inline` is disabled.
Probably another error in the logic for self-invocation detection. I will have a look!
Ok, this was just me forgetting how my implementation intially worked. I fixed it locally and will push a fix soon.
Same for mocked methods on Spy. On 2.12.0 real method is called.
When is this going to be on Maven Central?
We could trigger a new release, @TimvdLippe, would you find some time?
Yeah I can try it tomorrow :)

Op di 5 dec. 2017 om 20:55 schreef Rafael Winterhalter <
notifications@github.com>:

> We could trigger a new release, @TimvdLippe
> <https://github.com/timvdlippe>, would you find some time?
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/mockito/mockito/issues/1254#issuecomment-349422044>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AFrDb3sUMqw_c_exn9HiMDi5ls1QmDHdks5s9Z_LgaJpZM4QZVVK>
> .
>

Version 2.13.0 should be published and available soon in Maven Central!

P.S. @mockitoguy the `[ci maven-central-release]` was working flawlessly :tada: 
Danke! Bedankt! Many thanks!
Can confirm that finally our suite of 13+ K Kotlin tests work after fixing #1183 and this one ✨ Thanks!
>P.S. @mockitoguy the [ci maven-central-release] was working flawlessly 🎉

Awesome! Thank you for pushing out the version!

Pull request review comments:
With the issue fixed, this comment is no longer true. Would leave it out to avoid head-scratching.
Indeed, poorly copy-pasted. Thanks.
Can you add a code comment explaining why we need this? Please make it thorough, it would be great to share understanding! :)
Can we make the message more actionable? Is there anything the user can do? File a ticket?
Nice test coverage!

I'm wondering if we should rather put the inline tests in main mockito codebase. The use cases should work for every Mockito mock maker, inline or not. We do run all our tests with inline mock as far as I remember.
Do we? I would not recall that. If so, yes, we should probably move the class.
It is my library, I can add a method to get hold of the object. But I think we are safe here as we shade the code, no real danger here.
Added a comment.
Nevermind, we do of course inline it and can therefore change the code.
^ The comment is still there ;).

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
index 7d9f347f6c..02882a2274 100644
--- a/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
+++ b/src/main/java/org/mockito/internal/creation/bytebuddy/MockMethodAdvice.java
@@ -107,18 +107,20 @@ public Callable<?> handle(Object instance, Method origin, Object[] arguments) th
         return new ReturnValueWrapper(interceptor.doIntercept(instance,
                 origin,
                 arguments,
-            realMethod,
+                realMethod,
                 new LocationImpl(t)));
     }
 
     @Override
     public boolean isMock(Object instance) {
-        return interceptors.containsKey(instance);
+        // We need to exclude 'interceptors.target' explicitly to avoid a recursive check on whether
+        // the map is a mock object what requires reading from the map.
+        return instance != interceptors.target && interceptors.containsKey(instance);
     }
 
     @Override
     public boolean isMocked(Object instance) {
-        return !selfCallInfo.isSelfInvocation(instance) && isMock(instance);
+        return selfCallInfo.checkSuperCall(instance) && isMock(instance);
     }
 
     @Override
@@ -160,12 +162,8 @@ public Object invoke() throws Throwable {
             if (!Modifier.isPublic(origin.getDeclaringClass().getModifiers() & origin.getModifiers())) {
                 origin.setAccessible(true);
             }
-            Object previous = selfCallInfo.replace(instance);
-            try {
-                return tryInvoke(origin, instance, arguments);
-            } finally {
-                selfCallInfo.set(previous);
-            }
+            selfCallInfo.set(instance);
+            return tryInvoke(origin, instance, arguments);
         }
 
     }
@@ -252,14 +250,19 @@ public Object call() {
 
     private static class SelfCallInfo extends ThreadLocal<Object> {
 
-        Object replace(Object instance) {
+        Object replace(Object value) {
             Object current = get();
-            set(instance);
+            set(value);
             return current;
         }
 
-        boolean isSelfInvocation(Object instance) {
-            return get() == instance;
+        boolean checkSuperCall(Object value) {
+            if (value == get()) {
+                set(null);
+                return false;
+            } else {
+                return true;
+            }
         }
     }
 
diff --git a/src/main/java/org/mockito/internal/util/concurrent/WeakConcurrentMap.java b/src/main/java/org/mockito/internal/util/concurrent/WeakConcurrentMap.java
index c1476936e2..b411a73a9d 100644
--- a/src/main/java/org/mockito/internal/util/concurrent/WeakConcurrentMap.java
+++ b/src/main/java/org/mockito/internal/util/concurrent/WeakConcurrentMap.java
@@ -26,7 +26,7 @@ public class WeakConcurrentMap<K, V> extends ReferenceQueue<K> implements Runnab
 
     private static final AtomicLong ID = new AtomicLong();
 
-    final ConcurrentMap<WeakKey<K>, V> target;
+    public final ConcurrentMap<WeakKey<K>, V> target;
 
     private final Thread thread;
 


Test patch:
diff --git a/subprojects/inline/src/test/java/org/mockitoinline/RecursionTest.java b/subprojects/inline/src/test/java/org/mockitoinline/RecursionTest.java
new file mode 100644
index 0000000000..2a4494e03e
--- /dev/null
+++ b/subprojects/inline/src/test/java/org/mockitoinline/RecursionTest.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2017 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockitoinline;
+
+import org.junit.Test;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import static org.mockito.Mockito.spy;
+
+public class RecursionTest {
+
+    @Test
+    public void testMockConcurrentHashMap() {
+        ConcurrentMap<String, String> map = spy(new ConcurrentHashMap<String, String>());
+        map.putIfAbsent("a", "b");
+    }
+}
diff --git a/subprojects/inline/src/test/java/org/mockitoinline/SuperCallTest.java b/subprojects/inline/src/test/java/org/mockitoinline/SuperCallTest.java
new file mode 100644
index 0000000000..1fe81969d4
--- /dev/null
+++ b/subprojects/inline/src/test/java/org/mockitoinline/SuperCallTest.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2017 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockitoinline;
+
+import org.junit.Test;
+
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+public final class SuperCallTest {
+
+    @Test
+    public void testSuperMethodCall() {
+        Dummy d = spy(new Dummy());
+        d.foo();
+        verify(d).bar(eq("baz"));
+    }
+
+    static class Dummy {
+
+        public void foo() {
+            bar("baz");
+        }
+
+        // Also fails if public.
+        void bar(String s) {
+            return;
+        }
+    }
+}