From ffa12adb44e86772ae48eeb5a387f172130407fa Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Wed, 1 Dec 2021 09:51:18 -0600 Subject: [PATCH] Exit collect_coverage.sh early if LCOV_MERGER is not set. (#14359) 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 #13978 Closes #14352. PiperOrigin-RevId: 413369871 (cherry picked from commit aa52f2ddf9bab1ebd18e5431124061e813bfcd80) Co-authored-by: Charles Mita --- src/test/shell/bazel/BUILD | 9 ++ .../bazel/bazel_coverage_starlark_test.sh | 110 ++++++++++++++++++ tools/test/collect_coverage.sh | 12 ++ 3 files changed, 131 insertions(+) create mode 100755 src/test/shell/bazel/bazel_coverage_starlark_test.sh diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 6fd0acf47ba61c..77f83b834c7b03 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -563,6 +563,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 new file mode 100755 index 00000000000000..dd9e20b0ba517b --- /dev/null +++ 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 < 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 < 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 < 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 < 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 < 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 index 540c4c61de5cbf..febdb703a00dcd 100755 --- 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"