Update jacoco coverage runner to consider some edge cases.
After running the new coverage implementation on some real targets I found the following.
1. Inner classes with the same name could be added from different jars. Addressed this by analyzing classes with the same name exactly once.
2. Auto-generated files were added to the coverage report. This is resolved first on the blaze/bazel side by adding to the files with the exec paths of the instrumented files only those that are source files and on the coverage runner side by discarding all the files that are not in that file.
PiperOrigin-RevId: 162478894
diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java
index 5d13aaf..f88e168 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java
+++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java
@@ -34,6 +34,7 @@
import java.lang.reflect.Method;
import java.net.URL;
import java.util.Enumeration;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -145,9 +146,10 @@
IBundleCoverage analyzeStructure() throws IOException {
final CoverageBuilder coverageBuilder = new CoverageBuilder();
final Analyzer analyzer = new Analyzer(execFileLoader.getExecutionDataStore(), coverageBuilder);
+ Set<String> alreadyInstrumentedClasses = new HashSet<>();
for (File classesJar : classesJars) {
if (isNewCoverageImplementation) {
- analyzeUninstrumentedClassesFromJar(analyzer, classesJar);
+ analyzeUninstrumentedClassesFromJar(analyzer, classesJar, alreadyInstrumentedClasses);
} else {
analyzer.analyzeAll(classesJar);
}
@@ -163,9 +165,10 @@
new BranchDetailAnalyzer(execFileLoader.getExecutionDataStore());
Map<String, BranchCoverageDetail> result = new TreeMap<>();
+ Set<String> alreadyInstrumentedClasses = new HashSet<>();
for (File classesJar : classesJars) {
if (isNewCoverageImplementation) {
- analyzeUninstrumentedClassesFromJar(analyzer, classesJar);
+ analyzeUninstrumentedClassesFromJar(analyzer, classesJar, alreadyInstrumentedClasses);
} else {
analyzer.analyzeAll(classesJar);
}
@@ -179,15 +182,18 @@
*
* <p>The uninstrumented classes are named using the .class.uninstrumented suffix.
*/
- private void analyzeUninstrumentedClassesFromJar(Analyzer analyzer, File jar) throws IOException {
+ private void analyzeUninstrumentedClassesFromJar(
+ Analyzer analyzer, File jar, Set<String> alreadyInstrumentedClasses) throws IOException {
JarFile jarFile = new JarFile(jar);
JarInputStream jarInputStream = new JarInputStream(new FileInputStream(jar));
for (JarEntry jarEntry = jarInputStream.getNextJarEntry();
jarEntry != null;
jarEntry = jarInputStream.getNextJarEntry()) {
String jarEntryName = jarEntry.getName();
- if (jarEntryName.endsWith(".class.uninstrumented")) {
+ if (jarEntryName.endsWith(".class.uninstrumented")
+ && !alreadyInstrumentedClasses.contains(jarEntryName)) {
analyzer.analyzeAll(jarFile.getInputStream(jarEntry), jarEntryName);
+ alreadyInstrumentedClasses.add(jarEntryName);
}
}
}
diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoLCOVFormatter.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoLCOVFormatter.java
index 20f32e1..2f5acf5 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoLCOVFormatter.java
+++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoLCOVFormatter.java
@@ -67,12 +67,15 @@
private Map<String, ISourceFileCoverage> sourceToFileCoverage = new TreeMap<>();
private String getExecPathForEntryName(String fileName) {
+ if (execPathsOfUninstrumentedFiles.isEmpty()) {
+ return fileName;
+ }
for (String execPath : execPathsOfUninstrumentedFiles) {
if (execPath.endsWith("/" + fileName)) {
return execPath;
}
}
- return fileName;
+ return null;
}
@Override
@@ -102,15 +105,20 @@
for (IClassCoverage clsCoverage : pkgCoverage.getClasses()) {
String fileName = getExecPathForEntryName(
clsCoverage.getPackageName() + "/" + clsCoverage.getSourceFileName());
+ if (fileName == null) {
+ continue;
+ }
if (!sourceToClassCoverage.containsKey(fileName)) {
sourceToClassCoverage.put(fileName, new TreeMap<String, IClassCoverage>());
}
sourceToClassCoverage.get(fileName).put(clsCoverage.getName(), clsCoverage);
}
for (ISourceFileCoverage srcCoverage : pkgCoverage.getSourceFiles()) {
- sourceToFileCoverage.put(
- getExecPathForEntryName(srcCoverage.getPackageName() + "/" + srcCoverage.getName()),
- srcCoverage);
+ String sourceName = getExecPathForEntryName(
+ srcCoverage.getPackageName() + "/" + srcCoverage.getName());
+ if (sourceName != null) {
+ sourceToFileCoverage.put(sourceName, srcCoverage);
+ }
}
}
}
