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 Fixes #16666

Merged
merged 1 commit into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 32 additions & 67 deletions docs/src/main/asciidoc/tests-with-coverage.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ Now we need to add Jacoco to our project. To do this we need to add the followin
This Quarkus extension takes care of everything that would usually be done via the Jacoco maven plugin, so no additional
config is required.

WARNING: Using both the extension and the plugin requires special configuration, if you add both you will get lots of errors about classes
already being instrumented. The configuration needed is detailed below.

== Running the tests with coverage

Run `mvn verify`, the tests will be run and the results will end up in `target/jacoco-reports`. This is all that is needed,
Expand All @@ -189,86 +192,48 @@ include::{generated-dir}/config/quarkus-jacoco-jacoco-config.adoc[opts=optional,
The Quarkus automatic Jacoco config will only work for tests that are annotated with `@QuarkusTest`. If you want to check
the coverage of other tests as well then you will need to fall back to the Jacoco maven plugin.

Because Quarkus uses class file transformation it is not possible to use online transformation with the Jacoco agent.
Instead we need to use offline transformation.

Instead of including the `quarkus-jacoco` extension in your pom you will need the following config:

[source,xml,subs=attributes+]
----
<project>
...
<dependencies>
...
<dependency>
<groupId>org.jacoco</groupId>
<artifactId>org.jacoco.agent</artifactId>
<classifier>runtime</classifier>
<scope>test</scope>
<version>${jacoco.version}</version>
</dependency>
...
</dependencies>
<build>
<plugins>
...
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>${jacoco.version}</version>
<executions>
<execution>
<id>instrument-ut</id>
<goals>
<goal>instrument</goal>
</goals>
</execution>
<execution>
<id>restore-ut</id>
<goals>
<goal>restore-instrumented-classes</goal>
</goals>
</execution>
<execution>
<id>report-ut</id>
<goals>
<goal>report</goal>
</goals>
<configuration>
<outputDirectory>${project.reporting.outputDirectory}/jacoco-reports</outputDirectory>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
----

It also requires a small change in the Surefire configuration. Note below that we specified `jacoco-agent.destfile` as a system property in the default case (unit tests) and for the integration tests.
In addition to including the `quarkus-jacoco` extension in your pom you will need the following config:

[source,xml,subs=attributes+]
----
<project>
...
<build>
<plugins>
...
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire-plugin.version}</version>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<executions>
<execution>
<id>default-prepare-agent</id>
<goals>
<goal>prepare-agent</goal>
</goals>
<configuration>
<systemPropertyVariables>
<jacoco-agent.destfile>${project.build.directory}/jacoco.exec</jacoco-agent.destfile>
<java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
<maven.home>${maven.home}</maven.home>
</systemPropertyVariables>
<exclClassLoaders>*QuarkusClassLoader</exclClassLoaders> <1>
</configuration>
</plugin>
</execution>
<execution>
<id>default-prepare-agent-integration</id> <2>
<goals>
<goal>prepare-agent-integration</goal>
</goals>
<configuration>
<exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
----
<1> This config tells it to ignore `@QuarkusTest` related classes, as they are loaded by `QuarkusClassLoader`
<2> This is only needed if you are using Failsafe to run integration tests

WARNING: This config will only work if at least one `@QuarkusTest` is being run. If you are not using `@QuarkusTest` then
you can simply use the Jacoco plugin in the standard manner with no additional config.


== Conclusion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import org.jboss.jandex.ClassInfo;

import io.quarkus.bootstrap.model.AppArtifactKey;
import io.quarkus.bootstrap.model.AppDependency;
import io.quarkus.bootstrap.resolver.maven.workspace.LocalProject;
import io.quarkus.bootstrap.resolver.model.QuarkusModel;
import io.quarkus.bootstrap.resolver.model.WorkspaceModule;
import io.quarkus.bootstrap.utils.BuildToolHelper;
import io.quarkus.deployment.ApplicationArchive;
import io.quarkus.deployment.IsTest;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
Expand All @@ -31,6 +33,7 @@
import io.quarkus.deployment.builditem.FeatureBuildItem;
import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
import io.quarkus.deployment.pkg.builditem.BuildSystemTargetBuildItem;
import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem;
import io.quarkus.deployment.pkg.builditem.OutputTargetBuildItem;
import io.quarkus.jacoco.runtime.JacocoConfig;
import io.quarkus.jacoco.runtime.ReportCreator;
Expand All @@ -49,6 +52,7 @@ void transformerBuildItem(BuildProducer<BytecodeTransformerBuildItem> transforme
ApplicationArchivesBuildItem applicationArchivesBuildItem,
BuildSystemTargetBuildItem buildSystemTargetBuildItem,
ShutdownContextBuildItem shutdownContextBuildItem,
CurateOutcomeBuildItem curateOutcomeBuildItem,
JacocoConfig config) throws Exception {
String dataFile = outputTargetBuildItem.getOutputDirectory().toAbsolutePath().toString() + File.separator
+ config.dataFile;
Expand All @@ -58,30 +62,32 @@ void transformerBuildItem(BuildProducer<BytecodeTransformerBuildItem> transforme

Instrumenter instrumenter = new Instrumenter(new OfflineInstrumentationAccessGenerator());
Set<String> seen = new HashSet<>();
for (ClassInfo i : indexBuildItem.getIndex().getKnownClasses()) {
String className = i.name().toString();
if (seen.contains(className)) {
continue;
}
seen.add(className);
transformers.produce(
new BytecodeTransformerBuildItem.Builder().setClassToTransform(className)
.setCacheable(true)
.setEager(true)
.setInputTransformer(new BiFunction<String, byte[], byte[]>() {
@Override
public byte[] apply(String className, byte[] bytes) {
try {
byte[] enhanced = instrumenter.instrument(bytes, className);
if (enhanced == null) {
return bytes;
for (ApplicationArchive archive : applicationArchivesBuildItem.getAllApplicationArchives()) {
for (ClassInfo i : archive.getIndex().getKnownClasses()) {
String className = i.name().toString();
if (seen.contains(className)) {
continue;
}
seen.add(className);
transformers.produce(
new BytecodeTransformerBuildItem.Builder().setClassToTransform(className)
.setCacheable(true)
.setEager(true)
.setInputTransformer(new BiFunction<String, byte[], byte[]>() {
@Override
public byte[] apply(String className, byte[] bytes) {
try {
byte[] enhanced = instrumenter.instrument(bytes, className);
if (enhanced == null) {
return bytes;
}
return enhanced;
} catch (IOException e) {
throw new RuntimeException(e);
}
return enhanced;
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}).build());
}).build());
}
}
if (config.report) {
ReportInfo info = new ReportInfo();
Expand All @@ -99,16 +105,23 @@ public byte[] apply(String className, byte[] bytes) {
Set<String> sources = new HashSet<>();
MultiSourceFileLocator sourceFileLocator = new MultiSourceFileLocator(4);
if (BuildToolHelper.isMavenProject(targetdir.toPath())) {
Set<AppArtifactKey> runtimeDeps = new HashSet<>();
for (AppDependency i : curateOutcomeBuildItem.getEffectiveModel().getUserDependencies()) {
runtimeDeps.add(i.getArtifact().getKey());
}
LocalProject project = LocalProject.loadWorkspace(targetdir.toPath());
runtimeDeps.add(project.getKey());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aloubyansky do you know if the gradle code below would also need a similar change? I don't have any multi module gradle projects locally to test on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tried it but looking at the change, it makes sense. BTW, the AppModel already includes getLocalProjectArtifacts() method, it could simplify your loops here.

for (Map.Entry<AppArtifactKey, LocalProject> i : project.getWorkspace().getProjects().entrySet()) {
info.savedData.add(i.getValue().getOutputDir().resolve(config.dataFile).toAbsolutePath().toString());
sources.add(i.getValue().getSourcesSourcesDir().toFile().getAbsolutePath());
File classesDir = i.getValue().getClassesDir().toFile();
if (classesDir.isDirectory()) {
for (final File file : FileUtils.getFiles(classesDir, includes, excludes,
true)) {
if (file.getName().endsWith(".class")) {
classes.add(file.getAbsolutePath());
if (runtimeDeps.contains(i.getKey())) {
info.savedData.add(i.getValue().getOutputDir().resolve(config.dataFile).toAbsolutePath().toString());
sources.add(i.getValue().getSourcesSourcesDir().toFile().getAbsolutePath());
File classesDir = i.getValue().getClassesDir().toFile();
if (classesDir.isDirectory()) {
for (final File file : FileUtils.getFiles(classesDir, includes, excludes,
true)) {
if (file.getName().endsWith(".class")) {
classes.add(file.getAbsolutePath());
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -102,9 +103,17 @@ public void run() {
System.out.println("Generated Jacoco reports in " + targetdir);
System.out.flush();
} catch (Exception e) {
System.err.println("Failed to generate Jacoco reports ");
System.err.println("Failed to generate Jacoco reports");
e.printStackTrace();
System.err.flush();
File error = new File(targetdir, "error.txt");
try (FileOutputStream out = new FileOutputStream(error)) {
PrintStream ps = new PrintStream(out);
ps.println("Failed to generate Jacoco reports");
e.printStackTrace(ps);
} catch (IOException iugnore) {
//ignore
}
}
}

Expand Down