Fix JaCoCo instrumented constructor skipped. #8057
Conversation
|
Can a constructor and a shadow constructor be added to JacocoTester and ShadowJacocoTester |
sandbox/src/main/java/org/robolectric/internal/bytecode/ClassInstrumentor.java
Outdated
Show resolved
Hide resolved
sandbox/src/main/java/org/robolectric/internal/bytecode/ClassInstrumentor.java
Outdated
Show resolved
Hide resolved
Detect JaCoCo instrumentation from the original constructor by checking if the beginning one or two instructions are from JaCoCo. Simply by instructions' type and content. An then strip JaCoCo constructor probes data initialization instructions from `callSuper` instructions for JaCoCo instrumentation was detected. And insert these instructions into the beginning of the original construction. Overall, we assume that the JaCoCo instrumentation detecting and JaCoCo initialization instructions are in general cases and obey the JaCoCo implementations. Note that there can be many instructions before the call super() or this(), so this can still break the instruction compatibility? Tested by: JaCoCo offline integration tests. Signed-off-by: ZSmallX <diosmallx@gmail.com>
ZSmallX
force-pushed
the
fix_jacoco_instrumentec_constructor
branch
from
f3020ea
to
943b71e
Compare
| } | ||
| } | ||
|
|
||
| throw new RuntimeException("huh? JaCoCo instrumented?!"); |
There was a problem hiding this comment.
Is it fine to throw RuntimeException here?
There was a problem hiding this comment.
That could be a violation between robolectric's JaCoCo detection and JaCoCo instructions strip and split when it comes to line 421.
I think it would be better to fail early to avoid the previous issue when we has assumed that all the JaCoCo detection and JaCoCo instructions strip and split are breaked.
There was a problem hiding this comment.
But some users can accept no-code-coverage or other things, but they can't accept exceptions when running tests.
Yeap, that should be a test over constructor and shadow constructor to test all 3 robolectric instrumented methods: redirector constructor, constructor and |
Strip JaCoCo constructor probes data initialization instructions from
callSuperinstructions for JaCoCo instrumentation was detected. And insert these instructions into the beginning of the original constructor.Note that there can be many instructions before the call super() or this(), so this can still break the instruction compatibility?
Tested by: JaCoCo offline integration tests.
Fixes #8001