Skip to content

Commit

Permalink
Update jacoco coverage runner to consider some edge cases.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
iirina authored and aehlig committed Jul 19, 2017
1 parent f1843df commit 97966cc
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -145,9 +146,10 @@ public int getTabWidth() {
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);
}
Expand All @@ -163,9 +165,10 @@ private Map<String, BranchCoverageDetail> analyzeBranch() throws IOException {
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);
}
Expand All @@ -179,15 +182,18 @@ private Map<String, BranchCoverageDetail> analyzeBranch() throws IOException {
*
* <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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ public IReportVisitor createVisitor(
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
Expand Down Expand Up @@ -102,15 +105,20 @@ public void visitBundle(IBundleCoverage bundle, ISourceFileLocator locator)
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);
}
}
}
}
Expand Down

0 comments on commit 97966cc

Please sign in to comment.