Fix LCOV report generation to include uncovered executable lines

Author: nikita-tkachenko-datadog

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java

@@ -38,6 +38,7 @@
 import org.jacoco.core.analysis.CoverageBuilder;
 import org.jacoco.core.analysis.IBundleCoverage;
 import org.jacoco.core.analysis.ICounter;
+import org.jacoco.core.analysis.ILine;
 import org.jacoco.core.analysis.IPackageCoverage;
 import org.jacoco.core.analysis.ISourceFileCoverage;
 import org.jacoco.core.analysis.ISourceNode;
@@ -427,7 +428,7 @@ private static long getLocalCoveragePercentage(IBundleCoverage coverageBundle) {
   private long mergeAndUploadCoverageReport(IBundleCoverage coverageBundle) {
     RepoIndex repoIndex = repoIndexProvider.getIndex();
 
-    Map<String, BitSet> mergedCoverageData = new TreeMap<>();
+    Map<String, LinesCoverage> mergedCoverageData = new TreeMap<>();
 
     int totalLines = 0, coveredLines = 0;
     for (IPackageCoverage packageCoverage : coverageBundle.getPackages()) {
@@ -444,16 +445,16 @@ private long mergeAndUploadCoverageReport(IBundleCoverage coverageBundle) {
           continue;
         }
 
-        BitSet sourceFileCoveredLines = getCoveredLines(sourceFile);
+        LinesCoverage linesCoverage = getLinesCoverage(sourceFile);
         // backendCoverageData contains data for all modules in the repo,
         // but coverageBundle bundle only has source files that are relevant for the given module,
         // so we are not taking into account any backend coverage that is not relevant
-        sourceFileCoveredLines.or(
+        linesCoverage.coveredLines.or(
             backendCoverageData.getOrDefault(pathRelativeToIndexRoot, EMPTY_BIT_SET));
 
-        mergedCoverageData.put(pathRelativeToIndexRoot, sourceFileCoveredLines);
+        mergedCoverageData.put(pathRelativeToIndexRoot, linesCoverage);
 
-        coveredLines += sourceFileCoveredLines.cardinality();
+        coveredLines += linesCoverage.coveredLines.cardinality();
         totalLines += sourceFile.getLineCounter().getTotalCount();
       }
     }
@@ -463,7 +464,7 @@ private long mergeAndUploadCoverageReport(IBundleCoverage coverageBundle) {
     return Math.round((100d * coveredLines) / totalLines);
   }
 
-  private void uploadMergedCoverageReport(Map<String, BitSet> mergedCoverageData) {
+  private void uploadMergedCoverageReport(Map<String, LinesCoverage> mergedCoverageData) {
     if (coverageReportUploader == null) {
       return;
     }
@@ -477,21 +478,25 @@ private void uploadMergedCoverageReport(Map<String, BitSet> mergedCoverageData)
     }
   }
 
-  private static BitSet getCoveredLines(ISourceNode coverage) {
-    BitSet bitSet = new BitSet();
+  private static LinesCoverage getLinesCoverage(ISourceNode coverage) {
+    LinesCoverage linesCoverage = new LinesCoverage();
 
     int firstLine = coverage.getFirstLine();
     if (firstLine == -1) {
-      return bitSet;
+      return linesCoverage;
     }
 
     int lastLine = coverage.getLastLine();
-    for (int line = firstLine; line <= lastLine; line++) {
-      if (coverage.getLine(line).getStatus() >= ICounter.FULLY_COVERED) {
-        bitSet.set(line);
+    for (int lineIdx = firstLine; lineIdx <= lastLine; lineIdx++) {
+      ILine line = coverage.getLine(lineIdx);
+      if (line.getStatus() > ICounter.EMPTY) {
+        linesCoverage.executableLines.set(lineIdx);
+        if (line.getStatus() >= ICounter.FULLY_COVERED) {
+          linesCoverage.coveredLines.set(lineIdx);
+        }
       }
     }
 
-    return bitSet;
+    return linesCoverage;
   }
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LcovReportWriter.java

@@ -1,53 +1,60 @@
 package datadog.trace.civisibility.coverage.report;
 
+// LcovReportWriter.java
+
 import java.io.IOException;
 import java.io.StringWriter;
 import java.io.UncheckedIOException;
 import java.io.Writer;
-import java.util.BitSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.TreeMap;
-import org.jetbrains.annotations.NotNull;
+import javax.annotation.Nonnull;
 
-/** Serializes coverage data into LCOV format. */
 public final class LcovReportWriter {
+  private LcovReportWriter() {}
 
-  public static void write(Map<String, BitSet> coverage, Writer out) throws IOException {
+  public static void write(Map<String, LinesCoverage> coverage, Writer out) throws IOException {
     Objects.requireNonNull(coverage, "coverage");
     Objects.requireNonNull(out, "out");
 
-    Map<String, BitSet> sorted = (coverage instanceof TreeMap) ? coverage : toTreeMap(coverage);
+    Map<String, LinesCoverage> sorted =
+        (coverage instanceof TreeMap) ? coverage : toTreeMap(coverage);
 
-    for (Map.Entry<String, BitSet> e : sorted.entrySet()) {
+    for (Map.Entry<String, LinesCoverage> e : sorted.entrySet()) {
       String path = e.getKey();
-      BitSet bits = e.getValue();
+      LinesCoverage lc = e.getValue();
       if (path == null || path.isEmpty()) {
         continue;
       }
-      if (bits == null) {
-        bits = new BitSet();
+      if (lc == null) {
+        lc = new LinesCoverage();
       }
 
       out.write("SF:" + path + "\n");
 
-      int hits = 0;
-      for (int i = bits.nextSetBit(1); i >= 0; i = bits.nextSetBit(i + 1)) {
-        out.write("DA:" + i + ",1\n");
-        hits++;
+      int lf = 0; // lines found (instrumented)
+      int lh = 0; // lines hit (executed at least once)
+
+      for (int line = lc.executableLines.nextSetBit(1);
+          line >= 0;
+          line = lc.executableLines.nextSetBit(line + 1)) { // skip bit 0
+        lf++;
+        int count = lc.coveredLines.get(line) ? 1 : 0;
+        lh += count;
+        out.write("DA:" + line + "," + count + "\n");
       }
 
-      int lf = Math.max(0, bits.length() - 1); // exclude bit 0
-      out.write("LH:" + hits + "\n");
+      out.write("LH:" + lh + "\n");
       out.write("LF:" + lf + "\n");
       out.write("end_of_record\n");
     }
   }
 
-  @NotNull
-  private static TreeMap<String, BitSet> toTreeMap(Map<String, BitSet> coverage) {
-    TreeMap<String, BitSet> treeMap = new TreeMap<>();
-    for (Map.Entry<String, BitSet> e : coverage.entrySet()) {
+  @Nonnull
+  private static TreeMap<String, LinesCoverage> toTreeMap(Map<String, LinesCoverage> coverage) {
+    TreeMap<String, LinesCoverage> treeMap = new TreeMap<>();
+    for (Map.Entry<String, LinesCoverage> e : coverage.entrySet()) {
       if (e.getKey() == null) {
         continue;
       }
@@ -56,7 +63,7 @@ private static TreeMap<String, BitSet> toTreeMap(Map<String, BitSet> coverage) {
     return treeMap;
   }
 
-  public static String toString(Map<String, BitSet> coverage) {
+  public static String toString(Map<String, LinesCoverage> coverage) {
     try {
       StringWriter sw = new StringWriter();
       write(coverage, sw);

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LinesCoverage.java

@@ -0,0 +1,8 @@
+package datadog.trace.civisibility.coverage.report;
+
+import java.util.BitSet;
+
+public final class LinesCoverage {
+  public final BitSet coveredLines = new BitSet();
+  public final BitSet executableLines = new BitSet();
+}

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/LcovReportWriterTest.groovy

@@ -5,40 +5,34 @@ import spock.lang.Specification
 class LcovReportWriterTest extends Specification {
 
   def "empty map produces empty output"() {
-    given:
-    Map<String, BitSet> m = Collections.emptyMap()
-
     expect:
-    LcovReportWriter.toString(m) == ""
+    LcovReportWriter.toString(Collections.emptyMap()) == ""
   }
 
-  def "single file simple lines"() {
+  def "single file with executable and covered lines"() {
     given:
-    def bs = new BitSet()
-    bs.set(1)
-    bs.set(3)
-    Map<String, BitSet> m = Collections.singletonMap("src/Foo.java", bs)
+    def lc = new LinesCoverage()
+    lc.executableLines.set(1); lc.executableLines.set(2); lc.executableLines.set(3)
+    lc.coveredLines.set(1); lc.coveredLines.set(3)
+    def m = Collections.singletonMap("src/Foo.java", lc)
 
-    when:
-    def out = LcovReportWriter.toString(m)
-
-    then:
-    out == """SF:src/Foo.java
+    expect:
+    LcovReportWriter.toString(m) == """SF:src/Foo.java
 DA:1,1
+DA:2,0
 DA:3,1
 LH:2
 LF:3
 end_of_record
 """
   }
 
-  def "ignores bit 0 and handles large lines"() {
+  def "ignores bit 0 in both sets and handles very large lines"() {
     given:
-    def bs = new BitSet()
-    bs.set(0)        // ignored
-    bs.set(2)
-    bs.set(10000)
-    Map<String, BitSet> m = Collections.singletonMap("a.java", bs)
+    def lc = new LinesCoverage()
+    lc.executableLines.set(0); lc.executableLines.set(2); lc.executableLines.set(10000)
+    lc.coveredLines.set(0); lc.coveredLines.set(2); lc.coveredLines.set(10000)
+    def m = Collections.singletonMap("a.java", lc)
 
     when:
     def out = LcovReportWriter.toString(m)
@@ -47,18 +41,24 @@ end_of_record
     out.contains("SF:a.java\n")
     out.contains("DA:2,1\n")
     out.contains("DA:10000,1\n")
-    !out.contains("DA:0,1\n")
+    !out.contains("DA:0,")
     out.contains("LH:2\n")
-    out.contains("LF:10000\n") // length()=10001 -> LF=10000
+    out.contains("LF:2\n")
   }
 
   def "multiple files are sorted and lines within a file are sorted"() {
     given:
-    def b1 = new BitSet(); b1.set(5); b1.set(2)
-    def b2 = new BitSet(); b2.set(1)
-    def m = new LinkedHashMap<String, BitSet>()
-    m.put("z/FileZ.java", b2)
-    m.put("a/FileA.java", b1)
+    def a = new LinesCoverage()
+    a.executableLines.set(2); a.executableLines.set(5)
+    a.coveredLines.set(5)
+
+    def z = new LinesCoverage()
+    z.executableLines.set(1)
+    z.coveredLines.set(1)
+
+    def m = new LinkedHashMap<String, LinesCoverage>()
+    m.put("z/FileZ.java", z)
+    m.put("a/FileA.java", a)
 
     when:
     def out = LcovReportWriter.toString(m)
@@ -68,22 +68,19 @@ end_of_record
     def idxZ = out.indexOf("SF:z/FileZ.java\n")
     idxA >= 0 && idxZ > idxA
 
-    and: "lines sorted within a/FileA.java (2 then 5)"
+    and: "lines sorted within a/FileA.java (2 then 5) with correct hit counts"
     def blockA = out.substring(idxA, idxZ)
-    blockA.indexOf("DA:2,1\n") >= 0 &&
-      blockA.indexOf("DA:5,1\n") > blockA.indexOf("DA:2,1\n")
+    blockA.indexOf("DA:2,0\n") >= 0 &&
+      blockA.indexOf("DA:5,1\n") > blockA.indexOf("DA:2,0\n")
   }
 
-  def "null BitSet is treated as empty"() {
+  def "null LinesCoverage is treated as empty"() {
     given:
-    def m = new LinkedHashMap<String, BitSet>()
+    def m = new LinkedHashMap<String, LinesCoverage>()
     m.put("empty.java", null)
 
-    when:
-    def out = LcovReportWriter.toString(m)
-
-    then:
-    out == """SF:empty.java
+    expect:
+    LcovReportWriter.toString(m) == """SF:empty.java
 LH:0
 LF:0
 end_of_record
@@ -92,14 +89,44 @@ end_of_record
 
   def "skips empty or null path entries"() {
     given:
-    def bs = new BitSet()
-    bs.set(1)
-
-    def m = new LinkedHashMap<String, BitSet>()
-    m.put("", bs)
-    m.put(null, bs)
+    def lc = new LinesCoverage()
+    lc.executableLines.set(1); lc.coveredLines.set(1)
+    def m = new LinkedHashMap<String, LinesCoverage>()
+    m.put("", lc)
+    m.put(null, lc)
 
     expect:
     LcovReportWriter.toString(m) == ""
   }
+
+  def "covered lines outside executable set are ignored in DA and LH"() {
+    given:
+    def lc = new LinesCoverage()
+    lc.executableLines.set(1); lc.executableLines.set(2)
+    lc.coveredLines.set(3) // not executable -> ignored
+    def m = Collections.singletonMap("X.java", lc)
+
+    expect:
+    LcovReportWriter.toString(m) == """SF:X.java
+DA:1,0
+DA:2,0
+LH:0
+LF:2
+end_of_record
+"""
+  }
+
+  def "no executable lines even if covered bits exist -> no DA, LF=LH=0"() {
+    given:
+    def lc = new LinesCoverage()
+    lc.coveredLines.set(1); lc.coveredLines.set(2)
+    def m = Collections.singletonMap("Y.java", lc)
+
+    expect:
+    LcovReportWriter.toString(m) == """SF:Y.java
+LH:0
+LF:0
+end_of_record
+"""
+  }
 }

dd-smoke-tests/maven/src/test/resources/test_successful_maven_run/coverage_report.ftl

@@ -1,6 +1,7 @@
 SF:src/main/java/datadog/smoke/Calculator.java
+DA:3,0
 DA:5,1
 DA:9,1
 LH:2
-LF:9
+LF:3
 end_of_record