Skip to content

Commit f93ee0d

Browse files
authored
Merge pull request quarkusio#30315 from snazy/jacoco-eof
Mitigate Jacoco report generation issues
2 parents bcb313f + ed4dd95 commit f93ee0d

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

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)