serenity-junit5 dependency will break any tests execution (unit and integration) if jacoco plugin for code coverage is used in project

[beyond-danube]

When there are both serenity-junit5 and jacoco-maven-plugin any test run will fail, even if there are no Serenity JUnit5 tests at all. It's impossible to workaround. Any project that uses jacoco code coverage tool cannot contain serenity-junit5.

Preconditions:
Checkout modified Serenity Example https://github.com/beyond-danube/serenity-junit-starter-junit5-jacoco-issue

Example contains:

1. Serenity integration tests are switched to JUnit4 (Using RunWith instead of ExtendWith)
2. Added example classes and unit tests so Jacoco report is generated in the project

Steps to reproduce:

1. Run mvn clean verify (all good at this point, integration tests use JUnit4, unit tests use JUnit5, both serenity and jacoco reports are generated fine)
2. In pom.xml uncomment serenity-junit5 dependency
3. Run mvn clean verify again

Actual Result:
The whole thing will fail on the test stage with org.junit.platform.launcher.TestExecutionListener: Provider net.serenitybdd.junit5.SerenityTestExecutionListener could not be instantiated even if Serenity JUnit5 were not used anywhere.

Expected Result:
Dependency itself should not affect unrelated test execution.

[wakaleo]

Have you tried making sure Jacoco does not attempt to instrument the Serenity test classes?

[beyond-danube]

I did. I do not fully understand how instrumenting works, so cannot 100% guarantee, however, I've made these checks.

On a compiled project executing jacoco:instrument will generate main classes only. No Serenity test classes will be generated.
The issue occurs on unit test execution, it's a later stage as I get it, When unit tests are executed by surefire plugin, jacoco looks at what code from instrumented classes was hit based on jacoco.exec and generates a coverage report based on that.

The important note here is that serenity-junit works perfectly here, adding serenity-junit5 dependency will introduce the issue (no any serenity JUnit5 tests at this point, just dependency added), so I believe the issue is with SerenityTestExecutionListener that cannot be instantiated for some reason when there is jacoco argLine present.

The sure thing may be the issue is with jacoco itself, and this is how it works. I don't have enough knowledge unfortunately detect an exact root cause.

[wakaleo]

I had a quick look, but really have absolutely no idea what it could be caused by - possibly some incompatibility between byte-buddy and jacoco.

[vivganes]

I took a look a day ago and I guess it is due to the JDK source/target version 16 being used in the pom.xml used by @beyond-danube. I was able to run the code perfectly without any error in JDK 11.

When I ran using JDK 16 using source/target version 16, I got the following error log.

# Created at 2022-08-12T03:17:52.477
org.apache.maven.surefire.api.util.SurefireReflectionException: java.util.ServiceConfigurationError: org.junit.platform.launcher.TestExecutionListener: Provider net.serenitybdd.junit5.SerenityTestExecutionListener could not be instantiated
	at org.apache.maven.surefire.api.util.ReflectionUtils.instantiateOneArg(ReflectionUtils.java:127)
	at org.apache.maven.surefire.booter.ForkedBooter.createProviderInCurrentClassloader(ForkedBooter.java:491)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:428)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:562)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:548)
Caused by: java.util.ServiceConfigurationError: org.junit.platform.launcher.TestExecutionListener: Provider net.serenitybdd.junit5.SerenityTestExecutionListener could not be instantiated
	at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:586)
	at java.base/java.util.ServiceLoader$ProviderImpl.newInstance(ServiceLoader.java:813)
	at java.base/java.util.ServiceLoader$ProviderImpl.get(ServiceLoader.java:729)
	at java.base/java.util.ServiceLoader$3.next(ServiceLoader.java:1403)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.junit.platform.launcher.core.LauncherFactory.registerTestExecutionListeners(LauncherFactory.java:179)
	at org.junit.platform.launcher.core.LauncherFactory.createDefaultLauncher(LauncherFactory.java:137)
	at org.junit.platform.launcher.core.LauncherFactory.create(LauncherFactory.java:125)
	at org.junit.platform.launcher.core.LauncherFactory.create(LauncherFactory.java:109)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.<init>(JUnitPlatformProvider.java:90)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.apache.maven.surefire.api.util.ReflectionUtils.instantiateOneArg(ReflectionUtils.java:123)
	... 5 more
Caused by: java.lang.ExceptionInInitializerError
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at java.base/java.util.ServiceLoader$ProviderImpl.newInstance(ServiceLoader.java:789)
	... 26 more
Caused by: java.lang.UnsupportedOperationException: class redefinition failed: attempted to delete a method
	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:169)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at net.bytebuddy.utility.Invoker$Dispatcher.invoke(Unknown Source)
	at net.bytebuddy.utility.dispatcher.JavaDispatcher$Dispatcher$ForNonStaticMethod.invoke(JavaDispatcher.java:1019)
	at net.bytebuddy.utility.dispatcher.JavaDispatcher$ProxiedInvocationHandler.invoke(JavaDispatcher.java:1149)
	at net.bytebuddy.dynamic.loading.$Proxy26.retransformClasses(Unknown Source)
	at net.bytebuddy.dynamic.loading.ClassReloadingStrategy$Strategy$2.apply(ClassReloadingStrategy.java:399)
	at net.bytebuddy.dynamic.loading.ClassReloadingStrategy.load(ClassReloadingStrategy.java:227)
	at net.bytebuddy.dynamic.TypeResolutionStrategy$Passive.initialize(TypeResolutionStrategy.java:101)
	at net.bytebuddy.dynamic.DynamicType$Default$Unloaded.load(DynamicType.java:6161)
	at net.serenitybdd.junit5.SerenityTestExecutionListener.<clinit>(SerenityTestExecutionListener.java:51)
	... 32 more

The error seems to arise in the static initialization block in https://github.com/serenity-bdd/serenity-core/blob/main/serenity-junit5/src/main/java/net/serenitybdd/junit5/SerenityTestExecutionListener.java#L46

I will spend some more time on this today and post my findings.

[vivganes]

I was able to bypass this issue in JDK 16 after excluding org.junit.jupiter package from jacoco instrumentation using the <exclude> tag in pom.xml.

<exclude>org/junit/jupiter/**/*</exclude>

My analysis is this:

1. The static initialization block in SerenityTestExecutionListener creates a new implementation of Assertions class using Bytebuddy.
2. Jacoco identifies it as a class to be instrumented (I don't know why it does this)
3. During instrumentation, the Bytebuddy and Jacoco have different class structures and hence the error java.lang.UnsupportedOperationException: class redefinition failed: attempted to delete a method

Also found this Stackoverflow answer which seems to have an opinion against doing this in our static block.

@wakaleo is the functionality of the static block to instrument the Assertions class a WIP? I don't see it creating any test failures when commented out.

[wakaleo]

Looks like it is an incompatibility between the jacoco instrumentation and the bytecode instrumentation that Serenity uses - I suspect the code in the static block is necessary (I didn't write this code myself so I'm not 100% sure). Do you have the link to the stackoverflow answer? I think your solution of bypassing jacoco instrumentation of the jupiter classes makes sense (no reason why they should be instrumented).

[vivganes]

@wakaleo This is the stackoverflow answer. Missed to link it in my earlier reply.

Few more points after some more analysis:

1. Jacoco instruments them because our code created the Assertions class from Bytebuddy. It does not instrument the junit classes otherwise.

2. Even though we just added a dependency, the class SerenityTestExecutionListener is loaded without anyone actually calling or instantiating it because of the behavior of junit.

3. The Junit Platform launcher code loads all classes that extend TestExecutionListener. Since the SerenityTestExecutionListener extends TestExecutionListener, it is loaded by the classloader and the static block is executed.

[wakaleo]

I can't see using the -javaagent option as being a viable long-term solution, except as a workaround for jacoco usage, so I think the static code will need to stay as it is. @cliviu do you recall why we need to instrument the Assertions class in https://github.com/serenity-bdd/serenity-core/blob/main/serenity-junit5/src/main/java/net/serenitybdd/junit5/SerenityTestExecutionListener.java#L46?

[wakaleo]

The work-around suggested by @vivganes seems to be the best approach to getting this working - the issue seems to be that jacoco is instrumenting test framework code, which it probably shouldn't be doing, and Serenity also needs to instrument the JUnit Assertions class to ensure that the reporting works correctly.

<exclude>org/junit/jupiter/**/*</exclude>

[beyond-danube]

@wakaleo @vivganes thank you for looking into the issue.

The suggested workaround resolves the case. I have double-checked that with both a synthetic simplified project and the actual
Spring Boot project with unit tests and integration Serenity JUnit5 tests. The whole thing works as expected - Jacoco code coverage and Serenity reports were generated fine.

Even though this behavior is not obvious (since Serenity JUnit4 did not have this issue), the workaround is totally acceptable.