Repository: mockito/mockito
Issue: AssertionError in `MockUtil.getMockHandlerOrNull` in our regression test suite
Labels: please contribute

We appear to be hitting the following assertion in our test suites: https://github.com/mockito/mockito/blob/e2e628918518b73de7381692f28556ac6caa4710/src/main/java/org/mockito/internal/util/MockUtil.java#L160

Example error from [the logs](https://github.com/mockito/mockito/actions/runs/3612965923/jobs/6088416494#logs):

```
org.mockito.StaticMockingExperimentTest > stubbing_new FAILED
    java.lang.AssertionError
        at org.mockito.internal.util.MockUtil.getMockHandlerOrNull(MockUtil.java:160)
        at org.mockito.internal.util.MockUtil.isMock(MockUtil.java:147)
        at org.mockito.internal.util.DefaultMockingDetails.isMock(DefaultMockingDetails.java:32)
        at org.mockito.internal.util.DefaultMockingDetails.assertGoodMock(DefaultMockingDetails.java:85)
        at org.mockito.internal.util.DefaultMockingDetails.mockHandler(DefaultMockingDetails.java:77)
        at org.mockito.internal.util.DefaultMockingDetails.getMockHandler(DefaultMockingDetails.java:68)
        at org.mockito.StaticMockingExperimentTest.<init>(StaticMockingExperimentTest.java:44)
```

The assertion was introduced in #2701

Issue comments:
@TimvdLippe I think I do have time to look at it, feel free to assign it to me, thank you.
@TimvdLippe It seems like issue was caused by one test case, `ProgrammaticMockMakerTest`, which was conflicting with default mock maker strategies (by programmatically manipulating mock makers). I have extracted this test into separate test module, `programmatic-test`, and the logs seem to be clear, https://github.com/mockito/mockito/pull/2829
Hey! We hit this error as well, after upgrading to Mockito 5.3.1 from 3.x (using Java 17)

This happened unreproducibly as well because the failing tests are different from the ones using a non-standard mockmaker. 

If this can't be fixed (because it's not reproducible), please improve [the documentation](https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#individual_mock_maker) and/or the assertion with a better error message like "Please do not use different MockMakers for the same target class(?)" or something. I would send a PR myself but I'm not sure myself what's going on there. 

It's also very disconcerting that doing a thing in one test class affects other test classes in when using mockito. Can't this be avoided (I'm writing this knowing I have no clue about Mockito internals)?


Thanks @mr1chter 

> Hey! We hit this error as well, after upgrading to Mockito 5.3.1 from 3.x (using Java 17)

Could you share a bit more details - in you tests you also change the default mock maker from test class to test class? Could you share minimal examples how do you use different mock makers in your tests? 

> It's also very disconcerting that doing a thing in one test class affects other test classes in when using mockito. Can't this be avoided (I'm writing this knowing I have no clue about Mockito internals)?

This particular fix was specific to Mockito but we could certainly improve the documentation / error reporting, just need to understand the context a bit more.
We don't change the MockMaker from the default in almost all cases. There was a single one where I had to change it to the subclass MockMaker because a specific interface could not be mocked[1]. When I did this, some tests left and right of it failed due to the AssertionError, but inconsistently. When running all failing and changed tests in isolation (using gradle test --test Mytest) nothing failed anymore. But running some of them at once leads to AssertionErrors, which was very strange(looks like a race condition or something).

I just dropped in with the debugger and it said that in this specific assertion it gets a `InlineByteBuddyMockMaker` on the left and a `ByteBuddyMockMaker` on the right, `handler.getMockSettings().getMockMaker()` is `null`. The stackstrace is roughly the follows (i slightly indented the places that are not mockito code and replaced the class and method names of those):
```
getMockHandlerOrNull:160, MockUtil (org.mockito.internal.util), MockUtil.java
getMockHandler:115, MockUtil (org.mockito.internal.util), MockUtil.java
isVoid:94, InvocationInfo (org.mockito.internal.stubbing.answers), InvocationInfo.java
validateFor:34, Returns (org.mockito.internal.stubbing.answers), Returns.java
addAnswer:64, InvocationContainerImpl (org.mockito.internal.stubbing), InvocationContainerImpl.java
addAnswer:55, InvocationContainerImpl (org.mockito.internal.stubbing), InvocationContainerImpl.java
thenAnswer:32, OngoingStubbingImpl (org.mockito.internal.stubbing), OngoingStubbingImpl.java
thenReturn:34, BaseStubbing (org.mockito.internal.stubbing), BaseStubbing.java
 lambda$createTestStubs$0:65, CompanyInternalTest (com.forflow.internalstuff), CompanyInternalTest.java
 answer:-1, CompanyInternalTest$$Lambda$511/0x0000000801331208 (com.forflow.control.tacticalplanning.exchange.forecast), Unknown Source
answer:42, StubbedInvocationMatcher (org.mockito.internal.stubbing), StubbedInvocationMatcher.java
handle:103, MockHandlerImpl (org.mockito.internal.handler), MockHandlerImpl.java
handle:29, NullResultGuardian (org.mockito.internal.handler), NullResultGuardian.java
handle:34, InvocationNotifierHandler (org.mockito.internal.handler), InvocationNotifierHandler.java
doIntercept:82, MockMethodInterceptor (org.mockito.internal.creation.bytebuddy), MockMethodInterceptor.java
handle:134, MockMethodAdvice (org.mockito.internal.creation.bytebuddy), MockMethodAdvice.java
 methodUsedBySUT:103, ClassUsedBySUT (com.forflow.internalstuff), ClassUsedBySUT.java

```
As you might see, this is happening in a `when().doReturn()` which is called as part of a stubbed Method response itself. But afaict this can also happen in simpler stubbings on Mocks which used the regular default inline MockMaker.

Regarding documentation, I might open another issue because there are still quite some places that say "subclass mockMaker is the default" or "in the future, inline will be the default" even though this changed.

[1] The interface that could not be mocked(lets call it A) is also weird because it is not special in any way. The mocking fails with a "classic" InternalError: class redefinition failed, invalid class. I followed until this error with the debugger and noticed that it tried to redefine 4 interfaces (three of which are superinterfaces of A) and managed to identify an even more simple interface which can't be mocked.

But somehow, A can be successfully mocked with the inline mockmaker in other test classes.
I'll investigate this further and why it works there.
I got the inline MockMaker to work, sidestepping the problem (because now, no subclass mockmaker is used anywhere).
The workaround was to apply our internal junit extension which sets up a completely mocked environment. This is somewhat overkill for this test (and was not necessary before) but seems to do the trick. In fact, as long as the test mocking A is run after a separate test which had this setup extension applied to it, it works (also some kind of spooky side-effect that shouldn't be there).

So my takeaway is now to advise people to use one mockmaker or another, but not both, as long as those are used within the same Java process anywhere.
Sorry for the wall of text monologue, next time I'll open a proper ticket.
> Sorry for the wall of text monologue, next time I'll open a proper ticket.

Not a problem, thank you for sharing that

>  When running all failing and changed tests in isolation (using gradle test --test Mytest) nothing failed anymore. But running some of them at once leads to AssertionErrors, which was very strange(looks like a race condition or something).

That sounds about right, the order tests are scheduled on forked JVMs introduces randomness, and certainly, just running a test in isolation works all the time.

> Regarding documentation, I might open another issue because there are still quite some places that say "subclass mockMaker is the default" or "in the future, inline will be the default" even though this changed.

Please do, thank you

> But somehow, A can be successfully mocked with the inline mockmaker in other test classes.
I'll investigate this further and why it works there.

:+1

>  In fact, as long as the test mocking A is run after a separate test which had this setup extension applied to it, it works (also some kind of spooky side-effect that shouldn't be there).

Yes, similar approach we took for Mockito (to eliminate the possibility of race and isolate tests for differrent  mock makers).

> So my takeaway is now to advise people to use one mockmaker or another, but not both, as long as those are used within the same Java process anywhere.

Correct, or have some sort of test support for cleaning up the mock maker per test suite (the example that comes into mind is `@DirtiesContext` that Spring framework uses). I think we could explore this route. 

Pull request review comments:

Patch that resolved the issue:
diff --git a/settings.gradle.kts b/settings.gradle.kts
index 3fdc394e6a..551d0fa05f 100644
--- a/settings.gradle.kts
+++ b/settings.gradle.kts
@@ -19,7 +19,8 @@ include("subclass",
     "junitJupiterParallelTest",
     "osgi-test",
     "bom",
-    "errorprone")
+    "errorprone",
+    "programmatic-test")
 
 if (!JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_17) && (System.getenv("ANDROID_SDK_ROOT") != null || File(".local.properties").exists())) {
     include("androidTest")


Test patch:
diff --git a/subprojects/programmatic-test/programmatic-test.gradle b/subprojects/programmatic-test/programmatic-test.gradle
new file mode 100644
index 0000000000..365dad93e3
--- /dev/null
+++ b/subprojects/programmatic-test/programmatic-test.gradle
@@ -0,0 +1,18 @@
+plugins {
+    id 'java'
+}
+
+description = "Test suite for excercising programmatic mock maker in Mockito"
+
+apply from: "$rootDir/gradle/dependencies.gradle"
+
+dependencies {
+    implementation project.rootProject
+    testImplementation libraries.junit4
+    testImplementation libraries.assertj
+}
+
+tasks.javadoc.enabled = false
+
+sourceCompatibility = 11
+targetCompatibility = 11