Skip to content

Commit ed4dd95

Browse files
snazysnazy
committed
Fix Jacoco report generation
Externalizes the logic to wait for the Jacoco data file into `DataFileWatch`, which should handle the cases when the data file does not already exist (aka `JacococConfig.reuseDataFile==false`) and if it already exists (aka `JacocoConfig.reuseDataFile==true`). `DataFileWatch` remembers the "initial file size", which is the file size _before_ Jacoco runs. If the file does not already exists, the file size is `-1L`. The implementation waits basically "inifinitely" as long as the file size of the data file changes, which is interpreted as "data file is being updated by Jacoco". If the data file size stays at its "initial file size" for 10 seconds and the Jacoco shutdown hook's not running, the wait's aborted. If the "wait for Jacoco data file" has timed out, an additional message is printed to `stderr`. Also changes the "jacoco running" detection from inspecting the running threads to checking whether the Jacoco MBean is (still) registered. If the "wait for Jacoco data file" has timed out, an additional message is printed to `stderr`. The previous code also registered one shutdown-hook to create the report from the Jacoco data file for every _test case_ being executed. This lead to `number-of-test-cases` shutdown hooks to generate the same Jacoco report from the same Jacoco data file. The correct behavior is to generate one report when the JVM terminates. This is implemented by setting a system property, because every _test class_ results in a new Quarkus class loader running a different class instance of `JacocoProcessor`, using a static field in `JacocoProcessor` would still cause `number-of-test-classes` shutdown hooks. Fixes: quarkusio#30264
1 parent 96eff36 commit ed4dd95

File tree

6 files changed

+472
-92
lines changed

6 files changed

+472
-92
lines changed

‎test-framework/jacoco/deployment/src/main/java/io/quarkus/jacoco/deployment/JacocoProcessor.java‎

Lines changed: 59 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
import java.io.IOException;
55
import java.nio.file.Files;
66
import java.nio.file.Path;
7-
import java.nio.file.Paths;
87
import java.util.Collections;
8+
import java.util.HashMap;
99
import java.util.HashSet;
10+
import java.util.Map;
1011
import java.util.Optional;
1112
import java.util.Set;
1213
import java.util.function.BiFunction;
@@ -44,6 +45,8 @@ FeatureBuildItem feature() {
4445
return new FeatureBuildItem("jacoco");
4546
}
4647

48+
private static final Map<String, BytecodeTransformerBuildItem> transformedClasses = new HashMap<>();
49+
4750
@BuildStep(onlyIf = IsTest.class)
4851
void transformerBuildItem(BuildProducer<BytecodeTransformerBuildItem> transformers,
4952
OutputTargetBuildItem outputTargetBuildItem,
@@ -61,58 +64,74 @@ void transformerBuildItem(BuildProducer<BytecodeTransformerBuildItem> transforme
6164
return;
6265
}
6366

64-
String dataFile = getFilePath(config.dataFile(), outputTargetBuildItem.getOutputDirectory(),
65-
JacocoConfig.JACOCO_QUARKUS_EXEC);
67+
Path outputDir = outputTargetBuildItem.getOutputDirectory().toAbsolutePath();
68+
Files.createDirectories(outputDir);
69+
70+
Path dataFilePath = outputDir.resolve(config.dataFile().orElse(JacocoConfig.JACOCO_QUARKUS_EXEC));
71+
String dataFile = dataFilePath.toString();
72+
6673
System.setProperty("jacoco-agent.destfile", dataFile);
67-
if (!config.reuseDataFile()) {
68-
Files.deleteIfExists(Paths.get(dataFile));
69-
}
74+
System.setProperty("jacoco-agent.jmx", "true");
7075

7176
Instrumenter instrumenter = new Instrumenter(new OfflineInstrumentationAccessGenerator());
72-
Set<String> seen = new HashSet<>();
7377
for (ApplicationArchive archive : applicationArchivesBuildItem.getAllApplicationArchives()) {
7478
for (ClassInfo i : archive.getIndex().getKnownClasses()) {
7579
String className = i.name().toString();
76-
if (seen.contains(className)) {
77-
continue;
78-
}
79-
seen.add(className);
80-
transformers.produce(
81-
new BytecodeTransformerBuildItem.Builder().setClassToTransform(className)
82-
.setCacheable(true)
83-
.setInputTransformer(new BiFunction<String, byte[], byte[]>() {
84-
@Override
85-
public byte[] apply(String className, byte[] bytes) {
86-
try {
87-
byte[] enhanced = instrumenter.instrument(bytes, className);
88-
if (enhanced == null) {
89-
return bytes;
90-
}
91-
return enhanced;
92-
} catch (IOException e) {
93-
if (!log.isDebugEnabled()) {
94-
log.warnf(
95-
"Unable to instrument class %s with JaCoCo: %s, keeping the original class",
96-
className, e.getMessage());
97-
} else {
98-
log.warnf(e,
99-
"Unable to instrument class %s with JaCoCo, keeping the original class",
100-
className);
101-
}
80+
BytecodeTransformerBuildItem bytecodeTransformerBuildItem = transformedClasses.get(className);
81+
if (bytecodeTransformerBuildItem == null) {
82+
bytecodeTransformerBuildItem = new BytecodeTransformerBuildItem.Builder().setClassToTransform(className)
83+
.setCacheable(true)
84+
.setInputTransformer(new BiFunction<>() {
85+
@Override
86+
public byte[] apply(String className, byte[] bytes) {
87+
try {
88+
byte[] enhanced = instrumenter.instrument(bytes, className);
89+
if (enhanced == null) {
10290
return bytes;
10391
}
92+
return enhanced;
93+
} catch (IOException e) {
94+
if (!log.isDebugEnabled()) {
95+
log.warnf(
96+
"Unable to instrument class %s with JaCoCo: %s, keeping the original class",
97+
className, e.getMessage());
98+
} else {
99+
log.warnf(e,
100+
"Unable to instrument class %s with JaCoCo, keeping the original class",
101+
className);
102+
}
103+
return bytes;
104104
}
105-
}).build());
105+
}
106+
}).build();
107+
transformedClasses.put(className, bytecodeTransformerBuildItem);
108+
}
109+
transformers.produce(bytecodeTransformerBuildItem);
106110
}
107111
}
112+
113+
String sysPropName = "io.quarkus.internal.jacoco.report-data-file";
114+
String currentDataFile = System.setProperty(sysPropName, dataFile);
115+
if (currentDataFile != null) {
116+
if (!currentDataFile.equals(dataFile)) {
117+
System.err.println("Quarkus will use the Jacoco data file " + currentDataFile
118+
+ ", not the configured data file " + dataFile + ", because another build item triggered a report.");
119+
}
120+
return;
121+
}
122+
123+
if (!config.reuseDataFile()) {
124+
Files.deleteIfExists(dataFilePath);
125+
}
126+
108127
if (config.report()) {
109128
ReportInfo info = new ReportInfo();
110-
info.dataFile = dataFile;
129+
info.dataFile = dataFilePath;
111130

112-
File targetdir = new File(
113-
getFilePath(config.reportLocation(), outputTargetBuildItem.getOutputDirectory(),
114-
JacocoConfig.JACOCO_REPORT));
115-
info.reportDir = targetdir.getAbsolutePath();
131+
Path targetPath = outputDir.resolve(config.reportLocation().orElse(JacocoConfig.JACOCO_REPORT));
132+
info.reportDir = targetPath.toString();
133+
info.errorFile = targetPath.resolve("error.txt");
134+
Files.deleteIfExists(info.errorFile);
116135
String includes = String.join(",", config.includes());
117136
String excludes = String.join(",", config.excludes().orElse(Collections.emptyList()));
118137
Set<String> classes = new HashSet<>();
@@ -159,7 +178,7 @@ private void addProjectModule(ResolvedDependency module, JacocoConfig config, Re
159178
}
160179
}
161180

162-
private String getFilePath(Optional<String> path, Path outputDirectory, String defaultRelativePath) {
181+
private static String getFilePath(Optional<String> path, Path outputDirectory, String defaultRelativePath) {
163182
return path.orElse(outputDirectory.toAbsolutePath() + File.separator + defaultRelativePath);
164183
}
165184
}

‎test-framework/jacoco/runtime/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@
4747
<artifactId>quarkus-junit5-internal</artifactId>
4848
<scope>test</scope>
4949
</dependency>
50+
<dependency>
51+
<groupId>org.assertj</groupId>
52+
<artifactId>assertj-core</artifactId>
53+
<scope>test</scope>
54+
</dependency>
5055

5156
</dependencies>
5257

‎test-framework/jacoco/runtime/src/main/java/io/quarkus/jacoco/runtime/DataFileWatch.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package io.quarkus.jacoco.runtime;
2+
3+
import java.io.IOException;
4+
import java.nio.file.Files;
5+
import java.nio.file.Path;
6+
import java.util.function.BooleanSupplier;
7+
import java.util.function.Consumer;
8+
import java.util.function.LongSupplier;
9+
10+
class DataFileWatch {
11+
static final int ABORT_TIMEOUT_MILLIS = 10000;
12+
private final LongSupplier clock;
13+
private final Path datafile;
14+
private final long initialDataFileSize;
15+
private final BooleanSupplier jacocoFinished;
16+
private final Consumer<String> errorMsg;
17+
18+
DataFileWatch(Path datafile, BooleanSupplier jacocoFinished, Consumer<String> errorMsg) {
19+
this(System::currentTimeMillis, datafile, jacocoFinished, errorMsg);
20+
}
21+
22+
// Constructor for tests
23+
DataFileWatch(LongSupplier clock, Path datafile, BooleanSupplier jacocoFinished, Consumer<String> errorMsg) {
24+
this.clock = clock;
25+
this.datafile = datafile;
26+
this.jacocoFinished = jacocoFinished;
27+
this.errorMsg = errorMsg;
28+
29+
// Remember the initial size of the data file. If the data file does not already exist,
30+
// the value will be -1, otherwise (if `JacocoConfig.reuseDataFile` is true and the file
31+
// exists) the "old" size of the data file.
32+
// In other words: Jacoco wrote the data file if the file size is different from the
33+
// value of `initialDataFileSize`.
34+
try {
35+
this.initialDataFileSize = dataFileSize();
36+
} catch (IOException e) {
37+
throw new RuntimeException(e);
38+
}
39+
}
40+
41+
boolean waitForDataFile() throws IOException, InterruptedException {
42+
// The jacoco data is also generated by a shutdown hook.
43+
// Wait at most 10 seconds until the report data file appears.
44+
// Also, wait as long as the report data file grows, giving jacoco some "grace time" to write the next chunk of data.
45+
// Exit the loop when either the "abort timeout" is hit or the jacoco thread is no longer running.
46+
long abortTime = nextAbortTime();
47+
long fileSize;
48+
for (long previousFileSize = initialDataFileSize;; previousFileSize = fileSize) {
49+
fileSize = dataFileSize();
50+
if (fileSize != initialDataFileSize) {
51+
if (fileSize != previousFileSize) {
52+
// Give the jacoco thread writing the data file some time to write remaining data.
53+
abortTime = nextAbortTime();
54+
}
55+
if (fileSize > 0L && jacocoFinished.getAsBoolean()) {
56+
// Stop waiting when the Jacoco thread has stopped.
57+
return true;
58+
}
59+
}
60+
if (timedOut(abortTime)) {
61+
errorMsg.accept("Timed out waiting for Jacoco data file " + datafile + " update, file size before test run: "
62+
+ (initialDataFileSize != -1L ? initialDataFileSize : "did not exist") + ", current file size: "
63+
+ (fileSize != -1L ? fileSize : "does not exist"));
64+
return false;
65+
}
66+
waitSleep();
67+
}
68+
}
69+
70+
private boolean timedOut(long abortTime) {
71+
return abortTime - clock.getAsLong() < 0L;
72+
}
73+
74+
private long nextAbortTime() {
75+
return clock.getAsLong() + ABORT_TIMEOUT_MILLIS;
76+
}
77+
78+
// overridden in tests to let those complete quickly
79+
void waitSleep() throws InterruptedException {
80+
Thread.sleep(100L);
81+
}
82+
83+
// visible for testing
84+
long dataFileSize() throws IOException {
85+
return Files.exists(datafile) ? Files.size(datafile) : -1L;
86+
}
87+
}

0 commit comments

Comments
 (0)