Skip to content

Commit

Permalink
Provide the default lcov_merger to all Starlark tests
Browse files Browse the repository at this point in the history
Since 5b216b2, all Starlark-defined test rules either need to set the
`_lcov_merger` attribute or fail if run with `bazel coverage`. This is
fixed by attaching the default lcov merger as a late-bound default to
all Starlark rules via a new attribute and falling back to that
attribute in the TestActionBuilder.

This commit ensures backwards compatibility is maintained with existing
Starlark test rules that set the `_lcov_merger` attribute. This is also
verified in the included integration test.

Fixes #13978.
  • Loading branch information
fmeum committed Sep 13, 2021
1 parent f7ee209 commit 43191ee
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) {
BaseRuleClasses.coverageSupportAttribute(
labelCache.get(
toolsRepository + BaseRuleClasses.DEFAULT_COVERAGE_SUPPORT_VALUE))))
.add(
attr(":default_lcov_merger", LABEL)
.cfg(HostTransition.createFactory())
.value(BaseRuleClasses.getCoverageOutputGeneratorLabel()))
// Used in the one-per-build coverage report generation action.
.add(
attr(":coverage_report_generator", LABEL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ private TestParams createTestAction(int shards)
lcovMergerAttr = ":lcov_merger";
} else if (ruleContext.isAttrDefined("$lcov_merger", LABEL)) {
lcovMergerAttr = "$lcov_merger";
} else if (ruleContext.isAttrDefined(":default_lcov_merger", LABEL)) {
lcovMergerAttr = ":default_lcov_merger";
}
if (lcovMergerAttr != null) {
TransitiveInfoCollection lcovMerger = ruleContext.getPrerequisite(lcovMergerAttr);
Expand Down
9 changes: 9 additions & 0 deletions src/test/shell/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,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_thinlto_test",
srcs = ["bazel_thinlto_test.sh"],
Expand Down
137 changes: 137 additions & 0 deletions src/test/shell/bazel/bazel_coverage_starlark_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# 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 test_coverage_custom_rule_without_lcov_merger() {
local workspace="${FUNCNAME[0]}"
mkdir -p "${workspace}"

touch "${workspace}/WORKSPACE"
cat >> "${workspace}/BUILD" << EOF
load(":rules.bzl", "custom_test")
custom_test(
name = "custom_test",
)
EOF
cat >> "${workspace}/rules.bzl" << EOF
def _custom_test_impl(ctx):
executable = ctx.actions.declare_file(ctx.attr.name)
ctx.actions.write(executable, "", is_executable = True)
return [
DefaultInfo(
executable = executable,
),
]
custom_test = rule(
implementation = _custom_test_impl,
test = True,
)
EOF

cd "${workspace}"
bazel test //:custom_test &>"$TEST_log" \
|| fail "Test failed without coverage but should have succeeded"

bazel coverage //:custom_test &>"$TEST_log" \
|| fail "Test failed with coverage but should have succeeded"
}

function test_coverage_custom_rule_with_custom_lcov_merger() {
local workspace="${FUNCNAME[0]}"
mkdir -p "${workspace}"

touch "${workspace}/WORKSPACE"
cat >> "${workspace}/BUILD" << EOF
load(":rules.bzl", "custom_test")
java_binary(
name = "merger",
srcs = ["Merger.java"],
main_class = "Merger",
)
custom_test(
name = "custom_test",
)
EOF
cat >> "${workspace}/rules.bzl" << EOF
def _custom_test_impl(ctx):
executable = ctx.actions.declare_file(ctx.attr.name)
ctx.actions.write(executable, "", is_executable = True)
return [
DefaultInfo(
executable = executable,
),
]
custom_test = rule(
implementation = _custom_test_impl,
test = True,
attrs = {
"_lcov_merger": attr.label(
default = ":merger",
executable = True,
cfg = "host",
),
}
)
EOF
cat >> "${workspace}/Merger.java" << EOF
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
public class Merger {
public static void main(String[] args) throws IOException {
for (String arg : args) {
if (arg.startsWith("--output_file=")) {
String coverageFile = arg.substring("--output_file=".length());
Files.write(Paths.get(coverageFile), "custom_merger_ran".getBytes());
}
}
}
}
EOF

cd "${workspace}"
bazel test //:custom_test &>"$TEST_log" \
|| fail "Test failed without coverage but should have succeeded"

bazel coverage //:custom_test &>"$TEST_log" \
|| fail "Test failed with coverage but should have succeeded"
local coverage_file_path="$( get_coverage_file_path_from_test_log )"
grep -F "custom_merger_ran" "$coverage_file_path" \
|| fail "Failed to find message from custom merger"
}

run_suite "test coverage_starlark_test"

0 comments on commit 43191ee

Please sign in to comment.