From 1345938867e5c0c381f46cccf889c7b2b20a867a Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Tue, 15 Mar 2022 13:28:57 +0100
Subject: [PATCH] Fix coverage runfiles directory issue (#15047)

As discovered in https://github.com/bazelbuild/bazel/pull/15030 there
was another bug with the short circuiting coverage logic where tests
were not run from the correct directory. In that PR this issue is fixed
directly, with this change we instead make missing LCOV_MERGER be
deferred until after all test setup and run, which I think is more
future proof to other changes here, and also allows users to know their
tests are being run with coverage, and output the correct files, even in
the case they don't setup the coverage merger infrastructure.

Closes #15031.

PiperOrigin-RevId: 434715347

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
---
 .../bazel/bazel_coverage_starlark_test.sh     | 18 +++++++-
 tools/test/collect_coverage.sh                | 42 ++++++++-----------
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/src/test/shell/bazel/bazel_coverage_starlark_test.sh b/src/test/shell/bazel/bazel_coverage_starlark_test.sh
index e97c1500c5e14c..34fcd7f40d1b95 100755
--- a/src/test/shell/bazel/bazel_coverage_starlark_test.sh
+++ b/src/test/shell/bazel/bazel_coverage_starlark_test.sh
@@ -40,8 +40,22 @@ function test_starlark_rule_without_lcov_merger() {
     cat <<EOF > rules.bzl
 def _impl(ctx):
     output = ctx.actions.declare_file(ctx.attr.name)
-    ctx.actions.write(output, "", is_executable = True)
-    return [DefaultInfo(executable=output)]
+    ctx.actions.write(output, """\
+#!/bin/bash
+
+if [[ ! -r extra ]]; then
+  echo "extra file not found" >&2
+  exit 1
+fi
+
+if [[ -z \$COVERAGE ]]; then
+  echo "COVERAGE environment variable not set, coverage not run."
+  exit 1
+fi
+""", is_executable = True)
+    extra_file = ctx.actions.declare_file("extra")
+    ctx.actions.write(extra_file, "extra")
+    return [DefaultInfo(executable=output, runfiles=ctx.runfiles(files=[extra_file]))]
 
 custom_test = rule(
     implementation = _impl,
diff --git a/tools/test/collect_coverage.sh b/tools/test/collect_coverage.sh
index 3c475d9c9c3148..78f2b3548f2f1b 100755
--- a/tools/test/collect_coverage.sh
+++ b/tools/test/collect_coverage.sh
@@ -30,21 +30,6 @@ 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
-  # Execute the test.
-  "$@"
-  TEST_STATUS=$?
-  exit "$TEST_STATUS"
-fi
-
 function resolve_links() {
   local name="$1"
 
@@ -101,15 +86,6 @@ export JAVA_COVERAGE_FILE=$COVERAGE_DIR/jvcov.dat
 export COVERAGE=1
 export BULK_COVERAGE_RUN=1
 
-
-for name in "$LCOV_MERGER"; do
-  if [[ ! -e $name ]]; then
-    echo --
-    echo Coverage runner: cannot locate file $name
-    exit 1
-  fi
-done
-
 # Setting up the environment for executing the C++ tests.
 if [[ -z "$GCOV_PREFIX_STRIP" ]]; then
   # TODO: GCOV_PREFIX_STRIP=3 is incorrect on MacOS in the default setup
@@ -202,6 +178,24 @@ if [[ "$CC_CODE_COVERAGE_SCRIPT" ]]; then
     eval "${CC_CODE_COVERAGE_SCRIPT}"
 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.
+  exit 0
+fi
+
+for name in "$LCOV_MERGER"; do
+  if [[ ! -e $name ]]; then
+    echo --
+    echo Coverage runner: cannot locate file $name
+    exit 1
+  fi
+done
+
 # Export the command line that invokes LcovMerger with the flags:
 # --coverage_dir          The absolute path of the directory where the
 #                         intermediate coverage reports are located.