From 8f956511bb115c39ac683a1e78971fcf9dce5deb Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 2 Nov 2022 06:39:04 -0700 Subject: [PATCH] Allow Java coverage collection for external targets * 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 https://github.com/bazelbuild/bazel/issues/16228 Work towards https://github.com/bazelbuild/bazel/issues/16208 Closes #16268. PiperOrigin-RevId: 485580267 Change-Id: I830d3cf903462ca1799ef5f659a620dbdb92f349 --- .../actions/LazyWritePathsFileAction.java | 2 +- .../shell/bazel/bazel_coverage_java_test.sh | 172 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) 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 index e3462bab2c8093..600c9fff5d5d9d 100644 --- 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 index 27f86db018b32c..a7b0e31619be8f 100755 --- 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:: ()V +FN:6,com/example/Math::isEven (I)Z +FNDA:0,com/example/Math:: ()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:: ()V +FN:6,com/example/Collatz::getCollatzFinal (I)I +FNDA:0,com/example/Collatz:: ()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:: ()V +FN:6,com/example/Math::isEven (I)Z +FNDA:0,com/example/Math:: ()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"