-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mockito.spy(Activity).getBaseContext() returns null on Robolectric 4.4 and Java8 #2040
Comments
This seems to work with slightly older versions of mockito-inline, e.g. 3.4.2. I'll try a bisect and see what commit may have broken it. |
I did a bisect and this seems to have started failing in 47ef058, The workaround is to use mockito-inline < 3.5.0 I believe. |
I just had a look and the problem is likely with Robolectric. Maybe they can have a look at the problem? I am to foreign to both Android and Robolectric to really understand why this no longer works but it likely is related to the timing of the field copy. Worst case, we need to offer a switch to create spies using reflection if required but if we can get a hint why this timing is a problem, we might be able to address this. |
Thanks for the initial investigation @raphw, I am one of the Robolectric maintainers and can try investigating this. It's odd that on Java 11 this occasionally works (but it is flaky according to robolectric/robolectric#5916), but on Java 8 it seems to always fail. |
I'd like to understand in what field value the two objects differ to produce this result. They should be identical unless some field is not discoverable. If you could check what state difference is responsible, I can probably find out how it occurred. |
Out of curiosity is there some kind of helper/debug util in Mockito to dump an object to a String? In the meantime I'll try to just serialize it to a json-type format. |
No, we can dump class files but you could likely use a serializer such as Jackson for this? |
Weird, out of 269 fields, the only difference is a field called 'mBase'. On the spy object it is null. 132c132
< class android.content.ContextWrapper:mBase: android.app.ContextImpl@6d340ec8
---
> class android.content.ContextWrapper:mBase: null
269a270
> MainActivity: https://gist.github.com/hoisie/5a7b93acc90a68917fc97de1c8364fc5 Is there a way to turn on debug logs during this field copy to see how 'mBase' is being copied? |
This is the class that contains the 'mBase' field: FWIW, it's a long class hierarchy, |
I just noticed ContextThemeWrapper, which is the parent of ContextWrapper, calls the ContextWrapper(null) constructor, which could be setting mBase to null in ContextWrapper:
Could this be causing |
How does one dump the class bytecode of the transient spy class created? I am curious what that looks like. |
-Dnet.bytebuddy.dump=/some/folder |
For some reason, when I do |
No, the constructors are prefixed and then the original chain is shortcut. Basically: public MyClass { MyClass() {
// some code
} } Is turned into: public MyClass { MyClass() {
if (Mockito.isMockConstruction()) {
// Mockito stuff, basically copy all fields of the spied at object
return;
}
// some code
} } As a consequence, you cannot set a constructor breakpoint. I had a look at the field state and for both activity and mockActivity, |
Out of curiosity how do you inject print statements to the field copy ByteBuddy logic? I wanted to add some print statements to verify that the 'mBase' before/after values are the same.
How can you tell that? From what I see, it seems like 'mBase' is null for the spy activity: Do you think it's initially non-null and then gets set to null? |
You are right, I did not debug properly (IntelliJ refuses to run the sample project for me, I am back on the command line). When I inspect the transformed
When is the |
I created a special debug build (println-constructor, you can run it by using your local Maven repo and the publishToMavenLocal task of Mockito) which shows what is happening by printing all field assignments. It follows the pattern
This verifies that the field is actually set. It's very strange that the field is null again once it is used. Since this problem occurs only with Robolectric, there must be some mechanism where the value disappears. I also added a statement for the spy when it is constructed in its entirety and the field seems to be set correctly:
|
It's very strange, I optimized the example for output now and the fields seem to be set now that I changed the code: Field field = Class.forName("android.content.ContextWrapper").getDeclaredField("mBase");
field.setAccessible(true);
MainActivity activity = Robolectric.buildActivity(MainActivity.class).setup().get();
Object fieldBefore = field.get(activity);
Object methodBefore = activity.getBaseContext();
MainActivity spyActivity = Mockito.spy(activity);
throw new AssertionError("activity field: " + field.get(activity)
+ " - mock activity field: " + field.get(spyActivity)
+ " - activity getter: " + activity.getBaseContext()
+ " - mock activity getter: " + spyActivity.getBaseContext()
+ " - activity field before: " + fieldBefore
+ " - activity getter before: " + methodBefore
+ " - original: " + activity
+ " - mock: " + spyActivity); My gutt fealing is that that's some sort of initialization issue for the original object that is triggered later then Mockito expects it. |
Maybe I turned blind but simply running: MainActivity activity = Robolectric.buildActivity(MainActivity.class).setup().get();
MainActivity spyActivity = Mockito.spy(activity);
throw new AssertionError(activity.getBaseContext() + " - " + spyActivity.getBaseContext()); of the current release/3.x branch yields both fields being set correctly. Maybe this is something we already fixed in Mockito by accident? |
Okay, I verified that Mockito creates the spy correctly in any case. But something happens to the object if passed through Robolectics's runner logic. This test will function correctly:
While this test will yield yield an empty spy: private MainActivity activity;
private MainActivity mockActivity;
@Before
public void setup() {
activity = Robolectric.buildActivity(MainActivity.class).setup().get();
mockActivity = spy(activity);
}
@Test
public void test1() {
assertEquals("compare base context between activity and mockActivity",
activity.getBaseContext(),
mockActivity.getBaseContext());
} How can that be? |
Comparing the output of the println branch: if the mock is created in the |
I added a simpler test case to the Robolectric tree (currently It is very strange, I've also seen situations where the test starts passing randomly, and I usually have to do a Thanks for all your investigation and putting up that println branch, I'll try some print debugging as well. |
After adding some additional debug statements, it seems like the mockito instrumentation for
|
From looking at the bytecode, I see that ContextThemeWrapper calls ContextWrapper():
However, in ContextWrapper, the no-arg constructor is not insrumented:
The one-arg ContextWrapper constructor is instrumented. though:
|
This seems to be a simpler repro of the issue (non-Robolectric): ** EDIT ** NVM, still looking into a non-Robolectric repro: |
The issue seems to be that mockito is not instrumenting the no-arg The one-arg Do you know what might cause Mockito to skip over the no-arg |
Is that constructor added later by Robolectrics? Maybe there's some confusion around the creation order. It instruments the constructors in-flight but takes basis on the reflective model. You can set a breakpoint in the |
Yes, it does seem like Robolectric adds the no-arg constructor: (why does it do this? I cannot say exactly, that code was added many years ago...) |
It's certainly related to that. Byte Buddy uses the reflection API to process the members. How is it even possible that the reflection API is unaware of a constructor? A class cannot add members after its loaded, the JVM does not allow it. I assume this also explains why it makes a difference where the mock is created. Does Robolectrics transform classes from one class loader into another? I have a feeling this is somewhat related to parallel versions of the same class. |
As a workaround, Mockito could prefer non-synthetic constructors for the time being. |
Another possible solution: could Mockito only select super-constructors to call that are known to be instrumented? It should be possible to keep track of that. |
|
Yes, I believe this is happening. When an Android test is run, initially the Android SDK stubs are on the classpath, which contains a small subset of the real Android framework Jar that are all no-ops. Robolectric, during runtime, pulls in the real Android framework JARs and instruments the Android classes to use the real Android code (plus mixes in the shadows, which override the framework method implementations). |
BTW I think preferring non-synthetic constructors seems like a fine workaround for now. I will attempt to stop adding the no-arg constructor to Android objects. I'm not sure to what extent it is still being used. |
…tcut After moving spy creation to instrumenting constructor chains in ByteBuddyMockMaker, creating spies for Robolectric-instrumented Android classes started failing in some situations due to fields not being copied on base classes (specifically, ContextWrapper.mBase). The problem is that AsmVisitorWrapper.ForDeclaredMethods does not visit the synthetic constructrs that are added by Robolectric during runtime. While the visibility issuer is still being explored, a workaround is to prefer non-synthetic constructors when selecting which parent constructor to call in MockMethodAdvice.ConstructorShortcut. Fixes mockito#2040
…tcut After moving spy creation to instrumenting constructor chains in ByteBuddyMockMaker, creating spies for Robolectric-instrumented Android classes started failing in some situations due to fields not being copied on base classes (specifically, ContextWrapper.mBase). The problem is that AsmVisitorWrapper.ForDeclaredMethods does not visit the synthetic constructrs that are added by Robolectric during runtime. While the visibility issuer is still being explored, a workaround is to prefer non-synthetic constructors when selecting which parent constructor to call in MockMethodAdvice.ConstructorShortcut. Fixes mockito#2040
…tcut After moving spy creation to instrumenting constructor chains in ByteBuddyMockMaker, creating spies for Robolectric-instrumented Android classes started failing in some situations due to fields not being copied on base classes (specifically, ContextWrapper.mBase). The problem is that AsmVisitorWrapper.ForDeclaredMethods does not visit the synthetic constructrs that are added by Robolectric during runtime. While the visibility issuer is still being explored, a workaround is to prefer non-synthetic constructors when selecting which parent constructor to call in MockMethodAdvice.ConstructorShortcut. Fixes mockito#2040
Yes, it's in the changed class file but I wonder why the reflection API does not contain those constructors, that's why Byte Buddy cannot map them. I assume that the class is reloaded somehow into another class loader what then causes this mismatch. The problem in short is that ContectWrapper. class.getDeclaredConstructors() does not add the added no argument constructor. |
…ude-synthetic Do not exclude synthetic constructors from instrumentation. Fixes #2040.
Thanks for all your hard work @raphw! I am glad this is fixed. |
Right back at you! Thanks for your patient debugging help! |
Description
Since robolectric 4.4, Mockito.spy(Activity).getBaseContext() returns null
Steps to Reproduce
Failed unittest result : https://scans.gradle.com/s/wbmmbnzatcyai/tests/:app:testDevelopDebugUnitTest/com.example.myapplication.UnitTest1/test1#1
Robolectric & Android Version
Link to a public git repo demonstrating the problem:
https://github.com/ganadist/VersionCodeDemo/blob/mockito_spy_robolectric_4_4/app/src/test/java/com/example/myapplication/UnitTest1.java#L27-L38
This issue is copied from robolectric/robolectric#5916
The text was updated successfully, but these errors were encountered: