From ceca8e4c08245b1dad7d19c68e6481710ce52b2a Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 10 Sep 2022 13:07:30 +0200 Subject: [PATCH] Collect coverage for other languages When using `--combined_report=lcov`, the coverage report for a `go_test` will also include coverage for data dependencies executed by the test, e.g. `java_binary` or `cc_binary` tools run in an integration test. Note: This commit does *not* make it so that coverage is collected for `go_binary`s executed by `go_test`s - Go doesn't provide exit hooks, so this would be rather involved to implement. --- go/private/rules/test.bzl | 18 ++- go/tools/builders/BUILD.bazel | 6 - go/tools/builders/lcov_merger.sh | 21 --- go/tools/bzltestutil/lcov.go | 13 +- tests/core/coverage/lcov_coverage_test.go | 120 +++++++++++++++++- .../coverage/lcov_test_main_coverage_test.go | 2 + 6 files changed, 138 insertions(+), 42 deletions(-) delete mode 100755 go/tools/builders/lcov_merger.sh diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index 1dfd155561..110e3e1082 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -402,11 +402,21 @@ _go_test_kwargs = { default = ["//go/tools/bzltestutil"], cfg = go_transition, ), - # Workaround for bazelbuild/bazel#6293. See comment in lcov_merger.sh. + # Required for Bazel to collect coverage of instrumented C/C++ binaries + # executed by go_test. + # This is just a shell script and thus cheap enough to depend on + # unconditionally. + "_collect_cc_coverage": attr.label( + default = "@bazel_tools//tools/test:collect_cc_coverage", + cfg = "exec", + ), + # Required for Bazel to merge coverage reports for Go and other + # languages into a single report per test. + # Using configuration_field ensures that the tool is only built when + # run with bazel coverage, not with bazel test. "_lcov_merger": attr.label( - executable = True, - default = "//go/tools/builders:lcov_merger", - cfg = "target", + default = configuration_field(fragment = "coverage", name = "output_generator"), + cfg = "exec", ), "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index bb796883e7..40500a49f7 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -149,12 +149,6 @@ go_reset_target( visibility = ["//visibility:public"], ) -sh_binary( - name = "lcov_merger", - srcs = ["lcov_merger.sh"], - visibility = ["//visibility:public"], -) - filegroup( name = "all_builder_srcs", testonly = True, diff --git a/go/tools/builders/lcov_merger.sh b/go/tools/builders/lcov_merger.sh deleted file mode 100755 index 26169caf9a..0000000000 --- a/go/tools/builders/lcov_merger.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/env 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. - -# This is a workaround for bazelbuild/bazel#6293. Since Bazel 0.18.0, Bazel -# expects tests to have an "$lcov_merger' or "_lcov_merger" attribute that -# points to an executable. If this is missing, the test driver fails. - -exit 0 diff --git a/go/tools/bzltestutil/lcov.go b/go/tools/bzltestutil/lcov.go index f9210222ba..8b94b162a3 100644 --- a/go/tools/bzltestutil/lcov.go +++ b/go/tools/bzltestutil/lcov.go @@ -33,13 +33,7 @@ import ( // Bazel. // The conversion emits line and branch coverage, but not function coverage. func ConvertCoverToLcov() error { - // The value of test.coverprofile has been set to - // ${COVERAGE_OUTPUT_PATH}.cover by the generated TestMain. We have to strip - // the ".cover" suffix here so that the generated expectedLcov report is at the - // location where Bazel's expectedLcov merge picks it up. inPath := flag.Lookup("test.coverprofile").Value.String() - outPath := strings.TrimSuffix(inPath, ".cover") - in, err := os.Open(inPath) if err != nil { // This can happen if there are no tests and should not be an error. @@ -48,7 +42,8 @@ func ConvertCoverToLcov() error { } defer in.Close() - out, err := os.Create(outPath) + // All *.dat files in $COVERAGE_DIR will be merged by Bazel's lcov_merger tool. + out, err := os.CreateTemp(os.Getenv("COVERAGE_DIR"), "go_coverage.*.dat") if err != nil { return err } @@ -62,8 +57,8 @@ var _coverLinePattern = regexp.MustCompile(`^(?P.+):(?P\d+)\.(? const ( _pathIdx = 1 _startLineIdx = 2 - _endLineIdx = 4 - _countIdx = 7 + _endLineIdx = 4 + _countIdx = 7 ) func convertCoverToLcov(coverReader io.Reader, lcovWriter io.Writer) error { diff --git a/tests/core/coverage/lcov_coverage_test.go b/tests/core/coverage/lcov_coverage_test.go index 4f2c5af8c4..95800f85f2 100644 --- a/tests/core/coverage/lcov_coverage_test.go +++ b/tests/core/coverage/lcov_coverage_test.go @@ -47,6 +47,18 @@ go_test( srcs = ["lib_test.go"], deps = [":lib"], ) + +java_binary( + name = "Tool", + srcs = ["Tool.java"], +) + +go_test( + name = "lib_with_tool_test", + srcs = ["lib_with_tool_test.go"], + data = [":Tool"], + deps = [":lib"], +) -- src/lib.go -- package lib @@ -90,6 +102,40 @@ func TestLib(t *testing.T) { t.Error("Expected a newline in the output") } } +-- src/Tool.java -- +public class Tool { + public static void main(String[] args) { + if (args.length != 0) { + System.err.println("Expected no arguments"); + System.exit(1); + } + System.err.println("Hello, world!"); + } +} +-- src/lib_with_tool_test.go -- +package lib_test + +import ( + "os/exec" + "strings" + "testing" + + "example.com/lib" +) + +func TestLib(t *testing.T) { + if !strings.Contains(lib.HelloFromLib(false), "\n") { + t.Error("Expected a newline in the output") + } +} + +func TestTool(t *testing.T) { + err := exec.Command("Tool").Run() + if err != nil { + t.Error(err) + } +} + `, }) } @@ -120,7 +166,7 @@ func testLcovCoverage(t *testing.T, extraArgs ...string) { if err != nil { t.Fatal(err) } - for _, expectedIndividualCoverage := range expectedIndividualCoverages { + for _, expectedIndividualCoverage := range expectedGoCoverage { if !strings.Contains(string(individualCoverageData), expectedIndividualCoverage) { t.Errorf( "%s: does not contain:\n\n%s\nactual content:\n\n%s", @@ -146,8 +192,54 @@ func testLcovCoverage(t *testing.T, extraArgs ...string) { } } -var expectedIndividualCoverages = []string{ +func TestLcovCoverageWithTool(t *testing.T) { + args := append([]string{ + "coverage", + "--combined_report=lcov", + "//src:lib_with_tool_test", + }) + + if err := bazel_testing.RunBazel(args...); err != nil { + t.Fatal(err) + } + + individualCoveragePath := filepath.FromSlash("bazel-testlogs/src/lib_with_tool_test/coverage.dat") + individualCoverageData, err := ioutil.ReadFile(individualCoveragePath) + if err != nil { + t.Fatal(err) + } + expectedCoverage := append(expectedGoCoverage, expectedToolCoverage) + for _, expected := range expectedCoverage { + if !strings.Contains(string(individualCoverageData), expected) { + t.Errorf( + "%s: does not contain:\n\n%s\nactual content:\n\n%s", + individualCoveragePath, + expected, + string(individualCoverageData), + ) + } + } + + combinedCoveragePath := filepath.FromSlash("bazel-out/_coverage/_coverage_report.dat") + combinedCoverageData, err := ioutil.ReadFile(combinedCoveragePath) + if err != nil { + t.Fatal(err) + } + for _, include := range []string{ + "SF:src/lib.go\n", + "SF:src/other_lib.go\n", + "SF:src/Tool.java\n", + } { + if !strings.Contains(string(combinedCoverageData), include) { + t.Errorf("%s: does not contain %q\n", combinedCoverageData, include) + } + } +} + +var expectedGoCoverage = []string{ `SF:src/other_lib.go +FNF:0 +FNH:0 DA:3,1 DA:4,1 DA:5,0 @@ -158,6 +250,8 @@ LF:5 end_of_record `, `SF:src/lib.go +FNF:0 +FNH:0 DA:9,1 DA:10,1 DA:11,1 @@ -171,3 +265,25 @@ LH:8 LF:9 end_of_record `} + +const expectedToolCoverage = `SF:src/Tool.java +FN:1,Tool:: ()V +FN:3,Tool::main ([Ljava/lang/String;)V +FNDA:0,Tool:: ()V +FNDA:1,Tool::main ([Ljava/lang/String;)V +FNF:2 +FNH:1 +BRDA:3,0,0,1 +BRDA:3,0,1,0 +BRF:2 +BRH:1 +DA:1,0 +DA:3,1 +DA:4,0 +DA:5,0 +DA:7,1 +DA:8,1 +LH:3 +LF:6 +end_of_record +` diff --git a/tests/core/coverage/lcov_test_main_coverage_test.go b/tests/core/coverage/lcov_test_main_coverage_test.go index 3597713de3..57aafd28f8 100644 --- a/tests/core/coverage/lcov_test_main_coverage_test.go +++ b/tests/core/coverage/lcov_test_main_coverage_test.go @@ -108,6 +108,8 @@ func TestLcovCoverageWithTestMain(t *testing.T) { } const expectedIndividualCoverage = `SF:src/lib.go +FNF:0 +FNH:0 DA:3,1 DA:4,1 DA:5,0