Repository: mockito/mockito
Issue: Verify on Spy Interface incorrect count of actual invocation
Labels: please contribute

Hi,

At first, thanks for this awesome library. 

During a migration from mockito 4.4.0 to mockito 5.1.1 one of my test began to fail. 
It reported that the spied mock has 2 call one a method instead of one. 

I figured that  it has something to do with the new default [mockmaker](https://github.com/mockito/mockito/releases/tag/v5.0.0) since it's working with mockito-subclass.

Still, the message is misleading : it's suggesting that there is an error while the code works as expected.
Either there is a bug in the spy count , either the message should be changed to reflect that verify/spy on interface is impossible. 
(I may also have missed something :p ) 


Short failing example  java 11/ junit 5/ mockito 5.1.1

```java
import org.junit.jupiter.api.Test;

import static org.mockito.Mockito.*;

class MyTest {

    @Test
    void testSpyMockInterfaceMockito5() {
        MInterface spiedInterface = spy(mock(MInterface.class));
        spiedInterface.add(0);
        verify(spiedInterface).add(0);
    }
}

interface MInterface {

    void add(Integer i);
}

/*
## Mockito 5.1.1 
org.mockito.exceptions.verification.TooManyActualInvocations:
mInterface$MockitoMock$b85WZhRq.add(0);
Wanted 1 time:
-> at MyTest.testSpyMockInterfaceMockito5(MyTest.java:21)
But was 2 times:
-> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
-> at MyTest.testSpyMockInterfaceMockito5(MyTest.java:18)

## mockito 4.4.0 ok
## mockito-subclass 5.1.1 ok
 */
```

Issue comments:
I have the same issue on upgrade. I believe the issue is using `spy(mock(Foo.class))` - I see this in your upgrade and in the test that failed on my upgrade. I removed the outer `spy()` and my test works as expected again. Spying on a mock doesn't make sense, I don't think - imo Mockito should error in this scenario.
Indeed removing the spy() made the test pass. 

Still the current message feels wrong, an clear error indicating that spying a mock is forbidden would be better. 
Spying of mocks is not supported. If you would like to improve the messaging here, I would be happy to review a PR for it. You will need to update `spy(T object)` (https://github.com/mockito/mockito/blob/5486617f7ac759d24e99ee9d8d03e969acdd460f/src/main/java/org/mockito/Mockito.java#L2169) to check for `isMock`: https://github.com/mockito/mockito/blob/5486617f7ac759d24e99ee9d8d03e969acdd460f/src/main/java/org/mockito/internal/util/MockUtil.java#L132
Can someone help me understand why this only happens with interfaces and not with implementations? E.g.

This breaks as @bmaggi described:
```java
List<Integer> spy = Mockito.spy(Mockito.mock(List.class));
spy.add(1);
verify(spy).add(1);
```

This seems to work though:
```java
List<Integer> spy = Mockito.spy(Mockito.mock(LinkedList.class));
spy.add(1);
verify(spy).add(1);
```

What makes me think that if we add a validation to the spy(T Object) method, we might break some previously working code like the one in the second example. 
@xgrimauu Indeed it's a good point, if an exception is raised it will break your second example (that is currently working) 

For the moment, I see these options: 
1. Raise an invalid Argument Exception for all mock(will break test with spy(mock(class)))
2. Raise an exception only for mocked Interface (just need to find how ;))  
3. Add a logging message warn 
4. Fix the behavior for mocked interface 

@TimvdLippe What would be the best option ?

I implemented a draft [here](https://github.com/mockito/mockito/compare/main...bmaggi:mockito:feature/forbid_spy_on_mocked_interface?expand=1)

Only raise exception on interface. 

Any suggestion ? 
For information : Following the comment in the PR, I changed the code to forbid all mock.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/mockito/Mockito.java b/src/main/java/org/mockito/Mockito.java
index 428655e4a8..e97adac6cf 100644
--- a/src/main/java/org/mockito/Mockito.java
+++ b/src/main/java/org/mockito/Mockito.java
@@ -10,6 +10,7 @@
 import org.mockito.internal.creation.MockSettingsImpl;
 import org.mockito.internal.framework.DefaultMockitoFramework;
 import org.mockito.internal.session.DefaultMockitoSessionBuilder;
+import org.mockito.internal.util.MockUtil;
 import org.mockito.internal.verification.VerificationModeFactory;
 import org.mockito.invocation.Invocation;
 import org.mockito.invocation.InvocationFactory;
@@ -2167,6 +2168,10 @@ public static <T> T mock(Class<T> classToMock, MockSettings mockSettings) {
      * @return a spy of the real object
      */
     public static <T> T spy(T object) {
+        if (MockUtil.isMock(object)) {
+            throw new IllegalArgumentException(
+                    "Please don't pass mock here. Spy is not allowed on mock.");
+        }
         return MOCKITO_CORE.mock(
                 (Class<T>) object.getClass(),
                 withSettings().spiedInstance(object).defaultAnswer(CALLS_REAL_METHODS));


Test patch:
diff --git a/src/test/java/org/mockitousage/constructor/CreatingMocksWithConstructorTest.java b/src/test/java/org/mockitousage/constructor/CreatingMocksWithConstructorTest.java
index a70b5b25cc..76dbacbad1 100644
--- a/src/test/java/org/mockitousage/constructor/CreatingMocksWithConstructorTest.java
+++ b/src/test/java/org/mockitousage/constructor/CreatingMocksWithConstructorTest.java
@@ -409,6 +409,18 @@ public void handles_bridge_methods_correctly() {
         assertEquals("value", testBug.getValue(0));
     }
 
+    @Test
+    public void forbid_spy_on_mock() {
+        try {
+            spy(mock(List.class));
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertThat(e)
+                    .hasMessageContaining(
+                            "Please don't pass mock here. Spy is not allowed on mock.");
+        }
+    }
+
     public abstract class SomeAbstractClass<T> {
 
         protected abstract String getRealValue(T value);