Skip to content

Commit aa52f2d

Browse files
c-mitacopybara-github
authored andcommitted
Exit collect_coverage.sh early if LCOV_MERGER is not set.
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
1 parent 3f1672e commit aa52f2d

File tree

3 files changed

+131
-0
lines changed

3 files changed

+131
-0
lines changed

src/test/shell/bazel/BUILD

+9
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,15 @@ sh_test(
564564
],
565565
)
566566

567+
sh_test(
568+
name = "bazel_coverage_starlark_test",
569+
srcs = ["bazel_coverage_starlark_test.sh"],
570+
data = [":test-deps"],
571+
tags = [
572+
"no_windows",
573+
],
574+
)
575+
567576
sh_test(
568577
name = "bazel_cc_code_coverage_test",
569578
srcs = ["bazel_cc_code_coverage_test.sh"],
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
#!/bin/bash
2+
#
3+
# Copyright 2021 The Bazel Authors. All rights reserved.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -eu
18+
19+
# Load the test setup defined in the parent directory
20+
CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
21+
source "${CURRENT_DIR}/../integration_test_setup.sh" \
22+
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }
23+
24+
# Returns the path of the code coverage report that was generated by Bazel by
25+
# looking at the current $TEST_log. The method fails if TEST_log does not
26+
# contain any coverage report for a passed test.
27+
function get_coverage_file_path_from_test_log() {
28+
local ending_part="$(sed -n -e '/PASSED/,$p' "$TEST_log")"
29+
30+
local coverage_file_path=$(grep -Eo "/[/a-zA-Z0-9\.\_\-]+\.dat$" <<< "$ending_part")
31+
[[ -e "$coverage_file_path" ]] || fail "Coverage output file does not exist!"
32+
echo "$coverage_file_path"
33+
}
34+
35+
function set_up() {
36+
touch WORKSPACE
37+
}
38+
39+
function test_starlark_rule_without_lcov_merger() {
40+
cat <<EOF > rules.bzl
41+
def _impl(ctx):
42+
output = ctx.actions.declare_file(ctx.attr.name)
43+
ctx.actions.write(output, "", is_executable = True)
44+
return [DefaultInfo(executable=output)]
45+
46+
custom_test = rule(
47+
implementation = _impl,
48+
test = True,
49+
)
50+
EOF
51+
52+
cat <<EOF > BUILD
53+
load(":rules.bzl", "custom_test")
54+
55+
custom_test(name = "foo_test")
56+
EOF
57+
bazel coverage //:foo_test --combined_report=lcov > $TEST_log \
58+
|| fail "Coverage run failed but should have succeeded."
59+
}
60+
61+
function test_starlark_rule_with_custom_lcov_merger() {
62+
63+
cat <<EOF > lcov_merger.sh
64+
for var in "\$@"
65+
do
66+
if [[ "\$var" == "--output_file="* ]]; then
67+
path="\${var##--output_file=}"
68+
mkdir -p "\$(dirname \$path)"
69+
echo lcov_merger_called >> \$path
70+
exit 0
71+
fi
72+
done
73+
EOF
74+
chmod +x lcov_merger.sh
75+
76+
cat <<EOF > rules.bzl
77+
def _impl(ctx):
78+
output = ctx.actions.declare_file(ctx.attr.name)
79+
ctx.actions.write(output, "", is_executable = True)
80+
return [DefaultInfo(executable=output)]
81+
82+
custom_test = rule(
83+
implementation = _impl,
84+
test = True,
85+
attrs = {
86+
"_lcov_merger": attr.label(default = ":lcov_merger", cfg = "exec"),
87+
},
88+
)
89+
EOF
90+
91+
cat <<EOF > BUILD
92+
load(":rules.bzl", "custom_test")
93+
94+
sh_binary(
95+
name = "lcov_merger",
96+
srcs = ["lcov_merger.sh"],
97+
)
98+
custom_test(name = "foo_test")
99+
EOF
100+
101+
bazel coverage --test_output=all //:foo_test --combined_report=lcov > $TEST_log \
102+
|| fail "Coverage run failed but should have succeeded."
103+
104+
local coverage_file_path="$( get_coverage_file_path_from_test_log )"
105+
cat $coverage_file_path
106+
grep "lcov_merger_called" "$coverage_file_path" \
107+
|| fail "Coverage report did not contain evidence of custom lcov_merger."
108+
}
109+
110+
run_suite "test tests"

tools/test/collect_coverage.sh

+12
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ if [[ -n "$VERBOSE_COVERAGE" ]]; then
3030
set -x
3131
fi
3232

33+
if [[ -z "$LCOV_MERGER" ]]; then
34+
# this can happen if a rule returns an InstrumentedFilesInfo (which all do
35+
# following 5b216b2) but does not define an _lcov_merger attribute.
36+
# Unfortunately, we cannot simply stop this script being called in this case
37+
# due to conflicts with how things work within Google.
38+
# The file creation is required because TestActionBuilder has already declared
39+
# it.
40+
# TODO(cmita): Improve this situation so this early-exit isn't required.
41+
touch $COVERAGE_OUTPUT_FILE
42+
exit 0
43+
fi
44+
3345
function resolve_links() {
3446
local name="$1"
3547

0 commit comments

Comments
 (0)