Verify on Spy Interface incorrect count of actual invocation

[bmaggi]

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 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

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
 */

[sharpau]

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.

[bmaggi]

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.

[TimvdLippe]

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) (

mockito/src/main/java/org/mockito/Mockito.java

Line 2169 in 5486617

public static <T> T spy(T object) {

) to check for isMock:

mockito/src/main/java/org/mockito/internal/util/MockUtil.java

Line 132 in 5486617

public static boolean isMock(Object mock) {

[xgrimauu]

Can someone help me understand why this only happens with interfaces and not with implementations? E.g.

This breaks as @bmaggi described:

List<Integer> spy = Mockito.spy(Mockito.mock(List.class));
spy.add(1);
verify(spy).add(1);

This seems to work though:

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.

[bmaggi]

@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 ?

[bmaggi]

I implemented a draft here

Only raise exception on interface.

Any suggestion ?

[bmaggi]

For information : Following the comment in the PR, I changed the code to forbid all mock.