Allow Java coverage collection for external targets

fmeum · copybara-github · commit 8f956511bb11 · 2022-11-02T06:40:18.000-07:00
* Removes a hardcoded filter that results in the LCOV merger filtering out coverage of external targets even if these are matched by `--instrumentation_filter`. * Lets `LazyWritePathsFileAction` write out exec rather than root-relative paths, as advertised by its javadocs, so that the paths match those that the LCOV mergers filters by. Work towards bazelbuild#16228 Work towards bazelbuild#16208 Closes bazelbuild#16268. PiperOrigin-RevId: 485580267 Change-Id: I830d3cf903462ca1799ef5f659a620dbdb92f349
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java
@@ -94,7 +94,7 @@ private String getContents() {
         continue;
       }
       if (file.isSourceArtifact() || includeDerivedArtifacts) {
-        stringBuilder.append(file.getRootRelativePathString());
+        stringBuilder.append(file.getExecPathString());
         stringBuilder.append("\n");
       }
     }
diff --git a/src/test/shell/bazel/bazel_coverage_java_test.sh b/src/test/shell/bazel/bazel_coverage_java_test.sh
@@ -1143,4 +1143,176 @@ end_of_record"
   assert_coverage_result "$coverage_result_num_lib_header" "$coverage_file_path"
 }
 
+function setup_external_java_target() {
+  cat >> WORKSPACE <<'EOF'
+local_repository(
+    name = "other_repo",
+    path = "other_repo",
+)
+EOF
+
+  cat > BUILD <<'EOF'
+java_library(
+    name = "math",
+    srcs = ["src/main/com/example/Math.java"],
+    visibility = ["//visibility:public"],
+)
+EOF
+
+  mkdir -p src/main/com/example
+  cat > src/main/com/example/Math.java <<'EOF'
+package com.example;
+
+public class Math {
+
+  public static boolean isEven(int n) {
+    return n % 2 == 0;
+  }
+}
+EOF
+
+  mkdir -p other_repo
+  touch other_repo/WORKSPACE
+
+  cat > other_repo/BUILD <<'EOF'
+java_library(
+    name = "collatz",
+    srcs = ["src/main/com/example/Collatz.java"],
+    deps = ["@//:math"],
+)
+
+java_test(
+    name = "test",
+    srcs = ["src/test/com/example/TestCollatz.java"],
+    test_class = "com.example.TestCollatz",
+    deps = [":collatz"],
+)
+EOF
+
+  mkdir -p other_repo/src/main/com/example
+  cat > other_repo/src/main/com/example/Collatz.java <<'EOF'
+package com.example;
+
+public class Collatz {
+
+  public static int getCollatzFinal(int n) {
+    if (n == 1) {
+      return 1;
+    }
+    if (Math.isEven(n)) {
+      return getCollatzFinal(n / 2);
+    } else {
+      return getCollatzFinal(n * 3 + 1);
+    }
+  }
+
+}
+EOF
+
+  mkdir -p other_repo/src/test/com/example
+  cat > other_repo/src/test/com/example/TestCollatz.java <<'EOF'
+package com.example;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+
+public class TestCollatz {
+
+  @Test
+  public void testGetCollatzFinal() {
+    assertEquals(Collatz.getCollatzFinal(1), 1);
+    assertEquals(Collatz.getCollatzFinal(5), 1);
+    assertEquals(Collatz.getCollatzFinal(10), 1);
+    assertEquals(Collatz.getCollatzFinal(21), 1);
+  }
+
+}
+EOF
+}
+
+function test_external_java_target_can_collect_coverage() {
+  setup_external_java_target
+
+  bazel coverage --test_output=all @other_repo//:test --combined_report=lcov \
+   --instrumentation_filter=// &>$TEST_log \
+      || echo "Coverage for //:test failed"
+
+  local coverage_file_path="$(get_coverage_file_path_from_test_log)"
+  local expected_result_math='SF:src/main/com/example/Math.java
+FN:3,com/example/Math::<init> ()V
+FN:6,com/example/Math::isEven (I)Z
+FNDA:0,com/example/Math::<init> ()V
+FNDA:1,com/example/Math::isEven (I)Z
+FNF:2
+FNH:1
+BRDA:6,0,0,1
+BRDA:6,0,1,1
+BRF:2
+BRH:2
+DA:3,0
+DA:6,1
+LH:1
+LF:2
+end_of_record'
+  local expected_result_collatz="SF:external/other_repo/src/main/com/example/Collatz.java
+FN:3,com/example/Collatz::<init> ()V
+FN:6,com/example/Collatz::getCollatzFinal (I)I
+FNDA:0,com/example/Collatz::<init> ()V
+FNDA:1,com/example/Collatz::getCollatzFinal (I)I
+FNF:2
+FNH:1
+BRDA:6,0,0,1
+BRDA:6,0,1,1
+BRDA:9,0,0,1
+BRDA:9,0,1,1
+BRF:4
+BRH:4
+DA:3,0
+DA:6,1
+DA:7,1
+DA:9,1
+DA:10,1
+DA:12,1
+LH:5
+LF:6
+end_of_record"
+
+  assert_coverage_result "$expected_result_math" "$coverage_file_path"
+  assert_coverage_result "$expected_result_collatz" "$coverage_file_path"
+
+  assert_coverage_result "$expected_result_math" bazel-out/_coverage/_coverage_report.dat
+  assert_coverage_result "$expected_result_collatz" bazel-out/_coverage/_coverage_report.dat
+}
+
+function test_external_java_target_coverage_not_collected_by_default() {
+  setup_external_java_target
+
+  bazel coverage --test_output=all @other_repo//:test --combined_report=lcov &>$TEST_log \
+      || echo "Coverage for //:test failed"
+
+  local coverage_file_path="$(get_coverage_file_path_from_test_log)"
+  local expected_result_math='SF:src/main/com/example/Math.java
+FN:3,com/example/Math::<init> ()V
+FN:6,com/example/Math::isEven (I)Z
+FNDA:0,com/example/Math::<init> ()V
+FNDA:1,com/example/Math::isEven (I)Z
+FNF:2
+FNH:1
+BRDA:6,0,0,1
+BRDA:6,0,1,1
+BRF:2
+BRH:2
+DA:3,0
+DA:6,1
+LH:1
+LF:2
+end_of_record'
+
+  assert_coverage_result "$expected_result_math" "$coverage_file_path"
+  assert_not_contains "SF:external/other_repo/" "$coverage_file_path"
+
+  assert_coverage_result "$expected_result_math" bazel-out/_coverage/_coverage_report.dat
+  assert_not_contains "SF:external/other_repo/" bazel-out/_coverage/_coverage_report.dat
+}
+
 run_suite "test tests"

Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ private String getContents() {`
`94`	`94`	`continue;`
`95`	`95`	`}`
`96`	`96`	`if (file.isSourceArtifact() \|\| includeDerivedArtifacts) {`
`97`		`- stringBuilder.append(file.getRootRelativePathString());`
	`97`	`+ stringBuilder.append(file.getExecPathString());`
`98`	`98`	`stringBuilder.append("\n");`
`99`	`99`	`}`
`100`	`100`	`}`