Skip to content
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

JaCoCo not working with @QuarkusIntegrationTests #18559

Closed
xtaixe opened this issue Jul 9, 2021 · 25 comments · Fixed by #20319
Closed

JaCoCo not working with @QuarkusIntegrationTests #18559

xtaixe opened this issue Jul 9, 2021 · 25 comments · Fixed by #20319
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@xtaixe
Copy link

xtaixe commented Jul 9, 2021

Describe the bug

After upgrading to Quarkus 2 we saw a test coverage loss of 6% for both unit and integration tests.

I've tried to change how we have everything setup following https://quarkus.io/guides/tests-with-coverage (since we were still using offline instrumentation), starting with our integration tests, but I can't get things to work right (JaCoCo complaints about classes not matching execution data). I've created a small reproducer (linked below) by setting up the necessary parts as in https://quarkus.io/guides/tests-with-coverage and then having the following test (since we can't use @QuarkusIntegrationTest for a couple of reasons):

public class AppIT {

    @Test
    void myTest() throws Exception {
        new ProcessBuilder()
                .command("java", System.getProperty("quarkus.test.argLine"), "-jar", "target/quarkus-app/quarkus-run.jar")
                .inheritIO()
                .start()
                .waitFor();
    }

}

I also tried then with a REST resource and a proper @QuarkusIntegrationTest (reproducer also linked below) and got the same result: JaCoCo still complaining and showing a coverage of 0%.

Expected behavior

Can get JaCoCo working together with Quarkus, finding the right classes and reporting the right coverage.

Actual behavior

[INFO] --- jacoco-maven-plugin:0.8.7:report (report-it) @ app ---
[INFO] Loading execution data file /Users/xtaixe/dev/quarkus-reproducer/app/target/jacoco-quarkus.exec
[INFO] Analyzed bundle 'app' with 2 classes
[WARNING] Classes in bundle 'app' do not match with execution data. For report generation the same class files must be used as at runtime.

And JaCoCo reports an incorrect coverage (0% in the reproducers).

To Reproduce

quarkus-reproducer-1.zip
quarkus-reproducer-2.zip

Environment (please complete the following information):

Output of uname -a or ver

Darwin xtaixe 20.5.0 Darwin Kernel Version 20.5.0: Sat May 8 05:10:33 PDT 2021; root:xnu-7195.121.3~9/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.11" 2021-04-20
OpenJDK Runtime Environment GraalVM CE 21.1.0 (build 11.0.11+8-jvmci-21.1-b05)
OpenJDK 64-Bit Server VM GraalVM CE 21.1.0 (build 11.0.11+8-jvmci-21.1-b05, mixed mode, sharing)

Quarkus version or git rev

2.0.1.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.1 (05c21c65bdfed0f71a2f2ada8b84da59348c4c5d)
Maven home: /Users/xtaixe/.mvnvm/apache-maven-3.8.1
Java version: 11.0.11, vendor: GraalVM Community, runtime: /Library/Java/JavaVirtualMachines/graalvm-ce-java11-21.1.0/Contents/Home
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "11.4", arch: "x86_64", family: "mac"
@xtaixe xtaixe added the kind/bug Something isn't working label Jul 9, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 9, 2021

/cc @geoand

@famod
Copy link
Member

famod commented Jul 9, 2021

/cc @stuartwdouglas

PS: Maybe we should add a labeling rule for "jacoco".

@stuartwdouglas
Copy link
Member

Did this ever work? It looks like you are manually running a process with the Jacoco args, which will use the transformed classes, but it will only generate the report from the classes in target/classes, which are not transformed.

@xtaixe
Copy link
Author

xtaixe commented Jul 12, 2021

@stuartwdouglas no, this is an attempt to move to the Quarkus documented way of working with Jacoco, since currently we are still using offline instrumentation but with the upgrade to Quarkus 2 (no other changes) we lost some coverage.

I see what you mean, but isn't this the way things are documented to work also for @QuarkusIntegrationTest (see reproducer 2) which is also not working or am I missing something? Or how should things be configured for Jacoco to work with integration tests?

@geoand
Copy link
Contributor

geoand commented Jul 12, 2021

I see what you mean, but isn't this the way things are documented to work also for @QuarkusIntegrationTest (see reproducer 2) which is also not working or am I missing something? Or how should things be configured for Jacoco to work with integration tests?

What is not working?

@xtaixe
Copy link
Author

xtaixe commented Jul 12, 2021

Jacoco finding the right classes and reporting the right coverage for integration tests as documented in the guide. Unless I did something wrong the reproducer is configured as in the guide but not working properly.

@xtaixe
Copy link
Author

xtaixe commented Jul 14, 2021

I've been looking at JaCoCo configuration parameters and at the Quarkus JaCoCo extension and I'm not sure I know enough about how things work behind the scenes to get things working... I tried relying on the extension to generate the report for integration tests instead of the JaCoCo maven plugin, but it didn't work... I wonder if in this case is just that the extension is somehow not triggered for integration tests or not at the right time...

I'm pretty sure though things don't work as documented in https://quarkus.io/guides/tests-with-coverage#coverage-for-integration-tests (for integration tests, as @stuartwdouglas pointed out). So, @stuartwdouglas @geoand any chance you can take a look at providing JaCoCo support for integration tests (or if there's a configuration that should currently work)? Hopefully, if things work for @QuarkusIntegrationTests, I'll be able to get them working for our own...

@mykhailokulakov
Copy link

mykhailokulakov commented Jul 15, 2021

Having a quite similar issue here. After upgrading to quarkus 2.0.0-2.0.2 (from 1.13.6), jacoco-report folder with the report is not being generated in the gradle "check" phase

@bvarner
Copy link
Contributor

bvarner commented Sep 2, 2021

As mentioned in #9623, I had issues when upgrading to 2.0 with integration tests and jacoco.

However. Since that issue was closed, I've been having nothing but success in my projects with regards to jacoco and integration test coverage. I am using the jacoco-maven-plugin, as I've not been able to get the extension to work and append the results.

I'd suggest bumping to a newer release of quarkus and trying again.

@xtaixe
Copy link
Author

xtaixe commented Sep 3, 2021

@bvarner

  1. Are you using @QuarkusIntegrationTests?
  2. Are you using offline instrumentation? If not, could you explain or upload a small example (maybe based on the second reproducer attached in the description above) about how you configure the jacoco-maven-plugin to get it working and report code covered by @QuarkusIntegrationTests?

I already tried with the latest Quarkus version I haven't been able to get JaCoco to work with @QuarkusIntegrationTests (that second reproducer still shows a coverage of 0 for the class that is only covered by a @QuarkusIntegrationTest), so I'd really appreciate some guidance on this.

@bvarner
Copy link
Contributor

bvarner commented Sep 3, 2021

1.) Yes. I'm using @QuarkusIntegrationTest.
* My classes are named *IT.java
* They are running via the maven-failsafe-plugin.

                <plugin>
                <artifactId>maven-failsafe-plugin</artifactId>
                <version>${surefire-plugin.version}</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>integration-test</goal>
                            <goal>verify</goal>
                        </goals>
                        <configuration>
                            <skip>${skipTests}</skip>
                            <systemPropertyVariables>
                                <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
                                <maven.home>${maven.home}</maven.home>
                                <quarkus.test.native-image-profile>it</quarkus.test.native-image-profile>
                                <quarkus.test.argLine>${argLine}</quarkus.test.argLine>
                            </systemPropertyVariables>
                        </configuration>
                    </execution>
                </executions>
            </plugin>
  1. Yes, with the reports redirected using jacoco-maven-plugin...
            <plugin>
                <groupId>org.jacoco</groupId>
                <artifactId>jacoco-maven-plugin</artifactId>
                <version>${jacoco-plugin.version}</version>
                <executions>
                    <execution>
                        <id>default-prepare-agent-integration</id>
                        <goals>
                            <goal>prepare-agent-integration</goal>
                        </goals>
                        <configuration>
                            <skip>${skipTests}</skip>
                            <destFile>${project.build.directory}/jacoco-quarkus.exec</destFile>
                            <append>true</append>
                            <exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>
                        </configuration>
                    </execution>
                    <execution>
                        <id>report-it</id>
                        <phase>post-integration-test</phase>
                        <goals>
                            <goal>report</goal>
                        </goals>
                        <configuration>
                            <skip>${skipTests}</skip>
                            <dataFile>${project.build.directory}/jacoco-quarkus.exec</dataFile>
                            <outputDirectory>${project.build.directory}/jacoco-report</outputDirectory>
                        </configuration>
                    </execution>
                    <!-- Here you can add quality gates as well, if you don't have another tool for that. -->
                </executions>
            </plugin>

This will get everything into the jacoco-quarkus.exec datafile, suitable for importing into an IDE, or for later maven plugins to consume.

I have a philosophical issue with running Integration Tests as part of the surefire (unit test) phase. Mainly that I require extra setup & tear-down for those fixtures, which I can get with maven lifecycles -- Also separating those things out lets me completely exercise and test my database migrations as well. This 'ram everything into surefire' stuff Quarkus does by default is rather off-putting, and causes issues when you're trying to actually do a real integration tests.

I understand your frustration. This gave me fits for some time before I got it all setup and working.

The magic sauce was:

  • Separate the IT phase.
  • Instrument properly and have it append the report to the existing quarkus data file.

@xtaixe
Copy link
Author

xtaixe commented Sep 6, 2021

I don't get it. I don't see any significant difference and it doesn't work...

@bvarner any chance you could run this reproducer (same as second one above but updated to 2.2.1) and report if it works for you or not or if you spot any configuration errors? It contains a simple resource that is only tested by a @QuarkusIntegrationTest. When I run mvn verify JaCoCo complains that:

[WARNING] Classes in bundle 'app' do not match with execution data. For report generation the same class files must be used as at runtime.
[WARNING] Execution data for class GreetingResource does not match.

And shows a coverage of 0%:
image

@xtaixe
Copy link
Author

xtaixe commented Sep 9, 2021

quarkus-reproducer-single-module.zip Another reproducer. This time a single Maven module project (in case multi module was the issue). Same result...

@stuartwdouglas @geoand, before I give up on this, the reproducer is really simple, any chance you could take a look? Otherwise, is there a better chance that creating an example/quickstart (that hopefully includes @QuarkusIntegrationTests) is prioritized, since the one at https://github.com/quarkusio/quarkus-quickstarts/tree/main/tests-with-coverage-quickstart still uses offline instrumentation and doesn't match the current guide (which points at it)?

Thanks.

@geoand
Copy link
Contributor

geoand commented Sep 9, 2021

So the problem is that you seeing decreased code coverage when moving to Quarkus 2.x?

@xtaixe
Copy link
Author

xtaixe commented Sep 9, 2021

@geoand no, sorry if that was confusing. That was the trigger to trying to migrate from offline instrumentation to use JaCoCo as explained in https://quarkus.io/guides/tests-with-coverage, but the drop was caused by something else.

The problem is that we still need to move away from offline instrumentation and I can't get JaCoCo to work for @QuarkusIntegrationTest as explained in https://quarkus.io/guides/tests-with-coverage and there's no sample/quickstart project showing that it works since https://github.com/quarkusio/quarkus-quickstarts/tree/main/tests-with-coverage-quickstart is outdated. If you take a look at the last reproducer which is really simple and follows https://quarkus.io/guides/tests-with-coverage, you should see that for the parts of the code that are only tested by the @QuarkusIntegrationTest JaCoCo reports a coverage of 0% (cause it says that execution and report classes don't match).

@geoand
Copy link
Contributor

geoand commented Sep 10, 2021

My initial guess was that this happens because Quarkus loads the applications in a special classloader.
However this turns out to not be true, because the same problem occurs when legacy-jar or uber-jar are used as the package types.

I have actually never used Jacoco myself, so I had to debug the code and see what was going on.
It turns out that Jacoco assumes the bytecode of the compiled application classes is in target/classes.
This will work fine for classes that have not been transformed by Quarkus, but it's wrong for those that are (one of which is your GreetingResource).
Looking at the source of org.jacoco.maven.ReportSupport#processProject, there doesn't seem to be a way to configure the Jacoco Maven Plugin to use alternate "source" of .class files.
That would need to be taken up with the Jacoco team - ideally Jacoco would be able to use a set of URIs as the "source", the idea being that it could be configured to use one or multiple jars instead of a just a class directory.

Alternatively, @stuartwdouglas maybe we could consider copying (and replacing) the transformed classes into the build target output as well? WDYT?

@famod
Copy link
Member

famod commented Sep 11, 2021

That would need to be taken up with the Jacoco team - ideally Jacoco would be able to use a set of URIs as the "source", the idea being that it could be configured to use one or multiple jars instead of a just a class directory.

FWIW, I wouldn't hold my breath w.r.t. new jacoco-maven-plugin features: jacoco/jacoco#902 (comment)

@geoand
Copy link
Contributor

geoand commented Sep 12, 2021

I see, thanks!

In that case, the alternative I proposed should work and it's likely not hard to do.
I'll put it on my list.

geoand added a commit to geoand/quarkus that referenced this issue Sep 12, 2021
To make this work, essentially what we do is to overwrite
the build system output with the transformed bytecode
See quarkusio#18559 (comment)
for more details.

Fixes: quarkusio#18559
@geoand
Copy link
Contributor

geoand commented Sep 12, 2021

#20075 takes care of the problem

geoand added a commit to geoand/quarkus that referenced this issue Sep 12, 2021
To make this work, essentially what we do is to overwrite
the build system output with the transformed bytecode
See quarkusio#18559 (comment)
for more details.

Fixes: quarkusio#18559
geoand added a commit to geoand/quarkus that referenced this issue Sep 12, 2021
This is mainly done in order to allow code coverage tools
to skip the constructor

Relates to: quarkusio#18559
@geoand
Copy link
Contributor

geoand commented Sep 12, 2021

In addition to #20075, #20077 will boost the code coverage by removing the false negative on the default constructor

@xtaixe
Copy link
Author

xtaixe commented Sep 20, 2021

@geoand thanks for looking into this!

It turns out that Jacoco assumes the bytecode of the compiled application classes is in target/classes

My understanding is that the JaCoCo extension was solving that somehow, but it's not working for integration tests. Maybe @stuartwdouglas knows more?

That said, @geoand, your solution seem really simple to make that part work... Does JaCoCo work out-of-the-box then or is the JaCoCo extension still needed? I hope we can get this solved soon. Thanks!

@geoand
Copy link
Contributor

geoand commented Sep 20, 2021

It turns out that #20075 could cause subtle issues so it was not merged. Stuart has proposed an alternative that might work, but I have not looked into it.

@xtaixe xtaixe changed the title JaCoCo not working properly after upgrade to Quarkus 2 JaCoCo not working with @QuarkusIntegrationTests Sep 20, 2021
geoand added a commit to geoand/quarkus that referenced this issue Sep 21, 2021
To make this work, essentially what we do is to overwrite
the build system output with the transformed bytecode
See quarkusio#18559 (comment)
for more details.

Fixes: quarkusio#18559
geoand added a commit to geoand/quarkus that referenced this issue Sep 22, 2021
To make this work, essentially what we do is to overwrite
the build system output with the transformed bytecode
See quarkusio#18559 (comment)
for more details.

Fixes: quarkusio#18559
geoand added a commit that referenced this issue Sep 22, 2021
Make jacoco work with @QuarkusIntegrationTest
@quarkus-bot quarkus-bot bot added this to the 2.4 - main milestone Sep 22, 2021
@geoand geoand modified the milestones: 2.4 - main, 2.3.0.Final Sep 28, 2021
geoand added a commit to geoand/quarkus that referenced this issue Sep 28, 2021
To make this work, essentially what we do is to overwrite
the build system output with the transformed bytecode
See quarkusio#18559 (comment)
for more details.

Fixes: quarkusio#18559
(cherry picked from commit 18939d3)
@xtaixe
Copy link
Author

xtaixe commented Mar 17, 2022

Hey, finally got time to properly look into this again and I got it working, but quarkus.package.write-transformed-bytecode-to-build-output seems to be incompatible with the Quarkus JaCoCo extension. When using both, JaCoCo complains again about classes not being the same and in this case it fails to report the proper coverage for classes covered by NON integration tests.

The only way I got both unit and integration tests reporting the right coverage together (on a very simple maven single module project) was by:

  1. Using quarkus.package.write-transformed-bytecode-to-build-output.
  2. Removing the Quarkus JaCoCo extension.
  3. Removing <exclClassLoaders>*QuarkusClassLoader</exclClassLoaders> from the jacoco-maven-plugin configurations.

Attaching working single module project for reference: tests-with-coverage-quickstart-single-module.zip

On a multi module project with an aggregated JaCoCo report though, there's always some incompatibilities between the classes that were unit tested and the classes that JaCoCo sees when doing the report (or the other way around if for example I tell Quarkus to not remove "unused" classes). After coming to know that the classes are copied during the package phase, I'm not sure it's going to be possible to aggregate unit and integration test reports at all, and I have no idea why everything just works in a single module project.

cc @geoand and @stuartwdouglas in case you have some comments/suggestions or think it's worth to create some issues.

@pipinet
Copy link

pipinet commented Aug 21, 2022

Hey, finally got time to properly look into this again and I got it working, but quarkus.package.write-transformed-bytecode-to-build-output seems to be incompatible with the Quarkus JaCoCo extension. When using both, JaCoCo complains again about classes not being the same and in this case it fails to report the proper coverage for classes covered by NON integration tests.

The only way I got both unit and integration tests reporting the right coverage together (on a very simple maven single module project) was by:

  1. Using quarkus.package.write-transformed-bytecode-to-build-output.
  2. Removing the Quarkus JaCoCo extension.
  3. Removing <exclClassLoaders>*QuarkusClassLoader</exclClassLoaders> from the jacoco-maven-plugin configurations.

Attaching working single module project for reference: tests-with-coverage-quickstart-single-module.zip

On a multi module project with an aggregated JaCoCo report though, there's always some incompatibilities between the classes that were unit tested and the classes that JaCoCo sees when doing the report (or the other way around if for example I tell Quarkus to not remove "unused" classes). After coming to know that the classes are copied during the package phase, I'm not sure it's going to be possible to aggregate unit and integration test reports at all, and I have no idea why everything just works in a single module project.

cc @geoand and @stuartwdouglas in case you have some comments/suggestions or think it's worth to create some issues.

can not work with muti module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
8 participants
@stuartwdouglas @bvarner @mykhailokulakov @geoand @pipinet @famod @xtaixe and others