From ef085b6ac13918bcd5019aa200cf42458a75bdc1 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Sun, 28 Oct 2018 21:57:03 +0000 Subject: [PATCH] bazel/rules_r: bazel 0.18 compatibility for coverage Summary: From bazel 0.18.0, an lcov_merger tool is needed for every test rule that collects coverage. See https://github.com/bazelbuild/bazel/issues/6293 for more details. Reviewers: ysaito Reviewed By: ysaito Differential Revision: https://phabricator.grailbio.com/D20750 fbshipit-source-id: f480a5e --- R/internal/tests.bzl | 4 ++++ R/scripts/BUILD | 1 + R/scripts/collect_coverage.R | 8 ++------ R/scripts/lcov_merger.sh | 21 +++++++++++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) create mode 100755 R/scripts/lcov_merger.sh diff --git a/R/internal/tests.bzl b/R/internal/tests.bzl index 4168e69..891d52f 100644 --- a/R/internal/tests.bzl +++ b/R/internal/tests.bzl @@ -113,6 +113,10 @@ r_unit_test = rule( allow_single_file = True, default = "@com_grail_rules_r//R/scripts:collect_coverage.R", ), + "_lcov_merger": attr.label( + allow_single_file = True, + default = "@com_grail_rules_r//R/scripts:lcov_merger.sh", + ), }, doc = ("Rule to keep all deps in the sandbox, and run the test " + "scripts of the specified package. The package itself must " + diff --git a/R/scripts/BUILD b/R/scripts/BUILD index 33d34db..24914f8 100644 --- a/R/scripts/BUILD +++ b/R/scripts/BUILD @@ -19,6 +19,7 @@ exports_files([ "check.sh.tpl", "collect_coverage.R", "instrument.R", + "lcov_merger.sh", "library.sh.tpl", "test.sh.tpl", ]) diff --git a/R/scripts/collect_coverage.R b/R/scripts/collect_coverage.R index 9717bbe..599b9ee 100644 --- a/R/scripts/collect_coverage.R +++ b/R/scripts/collect_coverage.R @@ -15,12 +15,8 @@ # Collect code coverage # NOTE: -# bazel currently always tries to run lcov with gcov to generate lcov trace -# files, which is not applicable here. The only reason collect_coverage.sh from -# bazel succeeds is because it can not find gcov defined in the toolchain for -# the rule and skips the step. This is brittle and will break when bazel -# changes collect_coverage.sh or if we start specifying cc_deps as a dependency -# attribute in instrumented_files. More proper support for coverage in skylark +# This might break in the future if we start specifying cc_deps as a dependency +# attribute in instrumented_files. More proper support for coverage in starlark # rules is in the roadmap -- # https://bazel.build/roadmaps/coverage.html#improve-adding-coverage-support-for-skylark-rules-p2 diff --git a/R/scripts/lcov_merger.sh b/R/scripts/lcov_merger.sh new file mode 100755 index 0000000..0e52216 --- /dev/null +++ b/R/scripts/lcov_merger.sh @@ -0,0 +1,21 @@ +#!/bin/bash +# Copyright 2018 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. + +# Since bazel 0.18.0, an lcov_merger tool is mandatory for tests that +# collect coverage. This tool is run after the tests to generate the +# COVERAGE_OUTPUT_FILE. Since we generate this file from within our +# test runner directly, the lcov_merger tool is a no-op for us. + +exit 0