From 7f7d616ed0fd0791a1ee0469f5bc44e69e36c507 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 7 Sep 2022 11:23:50 +0200 Subject: [PATCH] Allow Java coverage collection for external targets * Removes a hardcoded filters that result 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. --- .../actions/LazyWritePathsFileAction.java | 2 +- .../shell/bazel/bazel_coverage_java_test.sh | 172 ++++++++++++++++++ tools/test/collect_coverage.sh | 1 - 3 files changed, 173 insertions(+), 2 deletions(-) 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" diff --git a/tools/test/collect_coverage.sh b/tools/test/collect_coverage.sh index f1690d88a589e7..657e790b880796 100755 --- a/tools/test/collect_coverage.sh +++ b/tools/test/collect_coverage.sh @@ -235,7 +235,6 @@ LCOV_MERGER_CMD="${LCOV_MERGER} --coverage_dir=${COVERAGE_DIR} \ --filter_sources=/usr/lib/.+ \ --filter_sources=/usr/include.+ \ --filter_sources=/Applications/.+ \ - --filter_sources=.*external/.+ \ --source_file_manifest=${COVERAGE_MANIFEST}" if [[ $COVERAGE_REPORTED_TO_ACTUAL_SOURCES_FILE ]]; then