Prefer non-synthetic constructors in MockMethodAdvice.ConstructorShortcut #2045
Conversation
hoisie
force-pushed
the
prefer-non-synthetic
branch
from
01e511b
to
2b95469
Compare
hoisie
changed the title
…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
hoisie
force-pushed
the
prefer-non-synthetic
branch
from
2b95469
to
6c220a1
Compare
Codecov Report
@@ Coverage Diff @@
## release/3.x #2045 +/- ##
==============================================
Coverage 84.89% 84.90%
Complexity 2704 2704
==============================================
Files 325 325
Lines 8204 8208 +4
Branches 979 980 +1
==============================================
+ Hits 6965 6969 +4
Misses 968 968
Partials 271 271
Continue to review full report at Codecov.
|
|
I hope it's temporary and we get to fix the root problem some time in the nesr future. |
I can try, though it would essentially recreating a Robolectric-like environment, so it may be challenging, considering there are so many moving parts to a Robolectric environment. I'll could try to create a custom child-first classloader that instuments a class to insert a constructor and see if I can replicate the failure. FWIW I did add a regression test on the Robolectric side: https://github.com/robolectric/robolectric/blob/master/integration_tests/mockito-experimental/src/test/java/org/robolectric/integrationtests/mockito/experimental/MockitoSpyTest.java |
|
Let's keep the test on the Robolectric side, that makes sense to me. Thanks! |
|
I found the actual problem: #2046 We excluded synthetic constructors by Byte Buddy's default. facepalm |
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 #2040