Throw explicit exception if experimental_split_coverage_postprocessin…

…g is used without experimental_fetch_all_coverage_outputs This closes bazelbuild#13185 According to bazelbuild@a445cda, The flag --experimental_split_coverage_postprocessing depends on the flag --experimental_fetch_all_coverage_outputs being enabled, but if the former one is set without the latter one, Bazel throws a quite confusing NullPointerException. Now we throw an explicit exception with proper error message.
blindpirate · Aug 21, 2022 · 2b2d3e1 · 2b2d3e1
1 parent 5116e92
commit 2b2d3e1
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 7 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -111,13 +111,10 @@ public TestRunnerSpawn createTestRunnerSpawn(
       throws ExecException, InterruptedException {
     if (action.getExecutionSettings().getInputManifest() == null) {
       String errorMessage = "cannot run local tests with --nobuild_runfile_manifests";
-      throw new TestExecException(
-          errorMessage,
-          FailureDetail.newBuilder()
-              .setTestAction(
-                  TestAction.newBuilder().setCode(TestAction.Code.LOCAL_TEST_PREREQ_UNMET))
-              .setMessage(errorMessage)
-              .build());
+      throw createTestExecException(
+          TestAction.Code.LOCAL_TEST_PREREQ_UNMET,
+          "cannot run local tests with --nobuild_runfile_manifests"
+      );
     }
     Map<String, String> testEnvironment =
         createEnvironment(
@@ -761,6 +758,16 @@ public void finalizeCancelledTest(List<FailedAttemptResult> failedAttempts) thro
     }
   }
 
+  private static TestExecException createTestExecException(TestAction.Code errorCode, String errorMessage) {
+    return new TestExecException(
+        errorMessage,
+        FailureDetail.newBuilder()
+            .setTestAction(TestAction.newBuilder().setCode(errorCode))
+            .setMessage(errorMessage)
+            .build()
+    );
+  }
+
   private final class BazelTestAttemptContinuation extends TestAttemptContinuation {
     private final TestRunnerAction testAction;
     @Nullable private final TestMetadataHandler testMetadataHandler;
@@ -884,6 +891,15 @@ public TestAttemptContinuation execute()
             .setHasCoverage(testAction.isCoverageMode());
 
         if (testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing()) {
+          if (testAction.getCoverageDirectoryTreeArtifact() == null) {
+            closeSuppressed(e, streamed);
+            closeSuppressed(e, fileOutErr);
+            // Otherwise we'll get a NPE https://github.com/bazelbuild/bazel/issues/13185
+            throw createTestExecException(
+                TestAction.Code.LOCAL_TEST_PREREQ_UNMET,
+                "coverageDirectoryTreeArtifact is null: --experimental_split_coverage_postprocessing depends on --experimental_fetch_all_coverage_outputs being enabled"
+            );
+          }
           actionExecutionContext
               .getMetadataHandler()
               .getMetadata(testAction.getCoverageDirectoryTreeArtifact());

diff --git a/src/test/shell/bazel/bazel_coverage_cc_test_gcc.sh b/src/test/shell/bazel/bazel_coverage_cc_test_gcc.sh
@@ -875,6 +875,79 @@ EOF
   assert_contains 'name: "test.lcov"' bep.txt
 }
 
+function test_coverage_with_experimental_split_coverage_postprocessing_only() {
+  local -r LCOV=$(which lcov)
+  if [[ ! -x ${LCOV:-/usr/bin/lcov} ]]; then
+    echo "lcov not installed. Skipping test."
+    return
+  fi
+  cat << EOF > WORKSPACE
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+  name = "com_google_googletest",
+  urls = ["https://github.com/google/googletest/archive/5c08f92c881b666998a4f7852c3cf9e393bf33a7.zip"],
+  strip_prefix = "googletest-5c08f92c881b666998a4f7852c3cf9e393bf33a7",
+)
+
+http_archive(
+    name = "rules_cc",
+    urls = ["https://github.com/bazelbuild/rules_cc/releases/download/0.0.2/rules_cc-0.0.2.tar.gz"],
+    sha256 = "58bff40957ace85c2de21ebfc72e53ed3a0d33af8cc20abd0ceec55c63be7de2",
+)
+EOF
+
+  cat << EOF > BUILD
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
+
+cc_library(
+    name = "hello-greet",
+    srcs = ["hello-greet.cc"],
+    hdrs = ["hello-greet.h"],
+)
+cc_test(
+  name = "hello-test",
+  srcs = ["hello-test.cc"],
+  deps = [
+    "@com_google_googletest//:gtest_main",
+    ":hello-greet",
+  ],
+)
+EOF
+
+  cat << EOF > hello-greet.h
+#ifndef LIB_HELLO_GREET_H_
+#define LIB_HELLO_GREET_H_
+
+#include <string>
+
+std::string get_greet(const std::string &thing);
+
+#endif
+EOF
+
+  cat << EOF > hello-greet.cc
+#include <string>
+
+std::string get_greet(const std::string& who) {
+  return "Hello " + who;
+}
+EOF
+
+  cat << EOF > hello-test.cc
+#include <gtest/gtest.h>
+#include "hello-greet.h"
+
+TEST(HelloTest, GetGreet) {
+  EXPECT_EQ(get_greet("Bazel"), "Hello Bazel");
+}
+EOF
+  bazel coverage --test_output=all --experimental_split_coverage_postprocessing //:hello-test \
+                &>$TEST_log && fail "Expected test failure" || :
+
+  assert_contains '--experimental_split_coverage_postprocessing depends on --experimental_fetch_all_coverage_outputs being enabled' $TEST_log
+}
+
 function test_coverage_doesnt_fail_on_empty_output() {
     if is_gcov_missing_or_wrong_version; then
         echo "Skipping test." && return