Exit collect_coverage.sh early if LCOV_MERGER is not set.

c-mita · copybara-github · commit aa52f2ddf9ba · 2021-12-01T03:44:08.000-08:00
Rules that provide InstrumentedFilesInfo but do not define the _lcov_merger attribute will result in this script being called without the required LCOV_MERGER environment variable being set. Since _all_ rules now provide an InstrumentedFilesInfo provider by default, but are not required to provide the attribute, this scenario will be encountered far more frequently and should not be an error. Because of the way coverage collection within Google differs to Bazel, it isn't safe to change TestActionBuilder to simply not prepare the collect_coverage.sh script the way it would if no coverage was to be collected, so we have to change the script itself. Fixes bazelbuild#13978 Closes bazelbuild#14352. PiperOrigin-RevId: 413369871
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
@@ -564,6 +564,15 @@ sh_test(
     ],
 )
 
+sh_test(
+    name = "bazel_coverage_starlark_test",
+    srcs = ["bazel_coverage_starlark_test.sh"],
+    data = [":test-deps"],
+    tags = [
+        "no_windows",
+    ],
+)
+
 sh_test(
     name = "bazel_cc_code_coverage_test",
     srcs = ["bazel_cc_code_coverage_test.sh"],
diff --git a/src/test/shell/bazel/bazel_coverage_starlark_test.sh b/src/test/shell/bazel/bazel_coverage_starlark_test.sh
@@ -0,0 +1,110 @@
+#!/bin/bash
+#
+# Copyright 2021 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eu
+
+# Load the test setup defined in the parent directory
+CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "${CURRENT_DIR}/../integration_test_setup.sh" \
+    || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# Returns the path of the code coverage report that was generated by Bazel by
+# looking at the current $TEST_log. The method fails if TEST_log does not
+# contain any coverage report for a passed test.
+function get_coverage_file_path_from_test_log() {
+  local ending_part="$(sed -n -e '/PASSED/,$p' "$TEST_log")"
+
+  local coverage_file_path=$(grep -Eo "/[/a-zA-Z0-9\.\_\-]+\.dat$" <<< "$ending_part")
+  [[ -e "$coverage_file_path" ]] || fail "Coverage output file does not exist!"
+  echo "$coverage_file_path"
+}
+
+function set_up() {
+    touch WORKSPACE
+}
+
+function test_starlark_rule_without_lcov_merger() {
+    cat <<EOF > rules.bzl
+def _impl(ctx):
+    output = ctx.actions.declare_file(ctx.attr.name)
+    ctx.actions.write(output, "", is_executable = True)
+    return [DefaultInfo(executable=output)]
+
+custom_test = rule(
+    implementation = _impl,
+    test = True,
+)
+EOF
+
+    cat <<EOF > BUILD
+load(":rules.bzl", "custom_test")
+
+custom_test(name = "foo_test")
+EOF
+    bazel coverage //:foo_test --combined_report=lcov > $TEST_log \
+        || fail "Coverage run failed but should have succeeded."
+}
+
+function test_starlark_rule_with_custom_lcov_merger() {
+
+    cat <<EOF > lcov_merger.sh
+for var in "\$@"
+do
+    if [[ "\$var" == "--output_file="* ]]; then
+        path="\${var##--output_file=}"
+        mkdir -p "\$(dirname \$path)"
+        echo lcov_merger_called >> \$path
+        exit 0
+    fi
+done
+EOF
+chmod +x lcov_merger.sh
+
+    cat <<EOF > rules.bzl
+def _impl(ctx):
+    output = ctx.actions.declare_file(ctx.attr.name)
+    ctx.actions.write(output, "", is_executable = True)
+    return [DefaultInfo(executable=output)]
+
+custom_test = rule(
+    implementation = _impl,
+    test = True,
+    attrs = {
+        "_lcov_merger": attr.label(default = ":lcov_merger", cfg = "exec"),
+    },
+)
+EOF
+
+    cat <<EOF > BUILD
+load(":rules.bzl", "custom_test")
+
+sh_binary(
+    name = "lcov_merger",
+    srcs = ["lcov_merger.sh"],
+)
+custom_test(name = "foo_test")
+EOF
+
+    bazel coverage --test_output=all //:foo_test --combined_report=lcov > $TEST_log \
+        || fail "Coverage run failed but should have succeeded."
+
+    local coverage_file_path="$( get_coverage_file_path_from_test_log )"
+    cat $coverage_file_path
+    grep "lcov_merger_called"  "$coverage_file_path" \
+        || fail "Coverage report did not contain evidence of custom lcov_merger."
+}
+
+run_suite "test tests"
diff --git a/tools/test/collect_coverage.sh b/tools/test/collect_coverage.sh
@@ -30,6 +30,18 @@ if [[ -n "$VERBOSE_COVERAGE" ]]; then
   set -x
 fi
 
+if [[ -z "$LCOV_MERGER" ]]; then
+  # this can happen if a rule returns an InstrumentedFilesInfo (which all do
+  # following 5b216b2) but does not define an _lcov_merger attribute.
+  # Unfortunately, we cannot simply stop this script being called in this case
+  # due to conflicts with how things work within Google.
+  # The file creation is required because TestActionBuilder has already declared
+  # it.
+  # TODO(cmita): Improve this situation so this early-exit isn't required.
+  touch $COVERAGE_OUTPUT_FILE
+  exit 0
+fi
+
 function resolve_links() {
   local name="$1"
 
