Spy doesn't forward hashcode/equals to actual object

[cdalexndr]

Mockito 3.11.1

public class MockitoTest {

    private static class ValueHolder {
        private final String value;

        public ValueHolder( String value ) {
            this.value = value;
        }

        public String getValue() {
            return value;
        }

        @Override public boolean equals( Object o ) {
            if (this == o) return true;
            if (!(o instanceof ValueHolder)) return false;
            ValueHolder that = (ValueHolder) o;
            return getValue().equals( that.getValue() );
        }

        @Override
        public int hashCode() {
            return Objects.hash( getValue() );
        }
    }

    @Test
    public void test() {
        ValueHolder value = new ValueHolder( "test" );
        ValueHolder spy = Mockito.spy( value );
        assert value.equals( spy );
        assert spy.equals( value ); //fails
        assert value.hashCode() == spy.hashCode(); //fails
    }
}

• The mockito message in the stacktrace have useful information, but it didn't help
• The problematic code (if that's possible) is copied here;
  Note that some configuration are impossible to mock via Mockito
• Provide versions (mockito / jdk / os / any other relevant information)
• Provide a Short, Self Contained, Correct (Compilable), Example of the issue
  (same as any question on stackoverflow.com)
• Read the contributing guide

[saurabh7248]

To create a mock a subclass of a mock is created through SubclassBytecodeGenerator.mockClass() here we can see that hashCode method is intercepted and handled by MockMethodInterceptor.ForHashCode. Here the code returns System.identityHashCode(thiz) as the hashcode where thiz is the object returned by Mockito.spy(). Same is the case of equals, where this==that is returned when equals is called.

@TimvdLippe shoud we add handling to delegate the call to hashCode and equals of spied object ?

[TimvdLippe]

Hm, I guess we could? I wonder if any tests start failing after that, but if they all pass I am okay with making that change yeah 👍

[saurabh7248]

I have opened a pull request here.
@TimvdLippe could you please review it.

[cdalexndr]

This issue needs to be reopened because it still reproduces with v3.12.4

Was fixed in v3.12.0 by 123beb8, but 1a8946f reverted the fix.
e1f9855 also removed the test added by initial fix.

[cdalexndr]

@raphw why was 1a8946f needed? (Note that the type cache was fixed 2h before your commit: 4f81d4f)

[raphw]

We use the same generated classes for both spy and mock classes. This is why we had to unify the behaviour. Internally, a spy is nothing but a mock with a prefefined delegatory answer. This is why a spy needs to behave like a mock.

Generally, I agree with that this behaviour would to be preferred. But it would require a lot of internal refactoring as we often rely on the hashcode/equals properties of mocks.

[dominiktopp]

Stumbled upon this today.

If this cannot be fixed in the near future I would suggest to at least document this e.g. in de JavaDoc of @Spy annotation and Mockito.spy method.