From 42dc8c1ce03ce05b5130b59abcb5663ae1badd8a Mon Sep 17 00:00:00 2001 From: Philipp Schrader Date: Fri, 10 Jun 2022 04:16:33 -0700 Subject: [PATCH] Fix null pointer crash with `bazel coverage` on only incompatible tests This is a follow-up to 2f1ff6fa17c3c30b2533bffe81f40eab06b453b9. That patch accidentally introduced a crash when running coverage on tests where all tests are incompatible. FATAL: bazel crashed due to an internal error. Printing stack trace: java.lang.NullPointerException: Null reportGenerator at com.google.devtools.build.lib.bazel.coverage.AutoValue_CoverageArgs.(AutoValue_CoverageArgs.java:68) at com.google.devtools.build.lib.bazel.coverage.CoverageArgs.create(CoverageArgs.java:58) at com.google.devtools.build.lib.bazel.coverage.CoverageReportActionBuilder.createCoverageActionsWrapper(CoverageReportActionBuilder.java:226) at com.google.devtools.build.lib.bazel.coverage.BazelCoverageReportModule$1.createCoverageReportActionsWrapper(BazelCoverageReportModule.java:98) at com.google.devtools.build.lib.analysis.BuildView.createResult(BuildView.java:558) at com.google.devtools.build.lib.analysis.BuildView.update(BuildView.java:492) at com.google.devtools.build.lib.buildtool.AnalysisPhaseRunner.runAnalysisPhase(AnalysisPhaseRunner.java:227) at com.google.devtools.build.lib.buildtool.AnalysisPhaseRunner.execute(AnalysisPhaseRunner.java:137) at com.google.devtools.build.lib.buildtool.BuildTool.buildTargets(BuildTool.java:266) at com.google.devtools.build.lib.buildtool.BuildTool.processRequest(BuildTool.java:506) at com.google.devtools.build.lib.buildtool.BuildTool.processRequest(BuildTool.java:474) at com.google.devtools.build.lib.runtime.commands.TestCommand.doTest(TestCommand.java:148) at com.google.devtools.build.lib.runtime.commands.TestCommand.exec(TestCommand.java:113) at com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.execExclusively(BlazeCommandDispatcher.java:584) at com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.exec(BlazeCommandDispatcher.java:231) at com.google.devtools.build.lib.server.GrpcServerImpl.executeCommand(GrpcServerImpl.java:550) at com.google.devtools.build.lib.server.GrpcServerImpl.lambda$run$1(GrpcServerImpl.java:614) at io.grpc.Context$1.run(Context.java:566) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) at java.base/java.lang.Thread.run(Unknown Source) This patch fixes the crash by handling the situation properly. This results in all tests being skipped and reported as being skipped. A new test validates the correct behaviour. (cherry picked from commit 3dd2b932d42fe86112899550d21452409cb3c4b0) --- .../coverage/CoverageReportActionBuilder.java | 4 ++ .../bazel_coverage_compatibility_test.sh | 37 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java index 2057361cf26273..6e09f9743caeef 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java @@ -211,6 +211,10 @@ public CoverageReportActionsWrapper createCoverageActionsWrapper( reportGenerator = testParams.getCoverageReportGenerator(); } } + // If all tests are incompatible, there's nothing to do. + if (reportGenerator == null) { + return null; + } builder.addAll(baselineCoverageArtifacts.toList()); ImmutableList coverageArtifacts = builder.build(); diff --git a/src/test/shell/bazel/bazel_coverage_compatibility_test.sh b/src/test/shell/bazel/bazel_coverage_compatibility_test.sh index cb2089e0657f00..37de3ba026488e 100755 --- a/src/test/shell/bazel/bazel_coverage_compatibility_test.sh +++ b/src/test/shell/bazel/bazel_coverage_compatibility_test.sh @@ -28,6 +28,7 @@ constraint_setting(name = "incompatible_setting") constraint_value( name = "incompatible", constraint_setting = ":incompatible_setting", + visibility = ["//visibility:public"], ) sh_test( @@ -40,6 +41,11 @@ sh_test( srcs = ["incompatible_test.sh"], target_compatible_with = [":incompatible"], ) + +exports_files([ + "compatible_test.sh", + "incompatible_test.sh", +]) EOF cat < compatible_test.sh #!/bin/bash @@ -51,10 +57,25 @@ exit 1 EOF chmod +x compatible_test.sh chmod +x incompatible_test.sh + + mkdir all_incompatible/ + cat < all_incompatible/BUILD +sh_test( + name = "incompatible1_test", + srcs = ["//:incompatible_test.sh"], + target_compatible_with = ["//:incompatible"], +) + +sh_test( + name = "incompatible2_test", + srcs = ["//:incompatible_test.sh"], + target_compatible_with = ["//:incompatible"], +) +EOF } -# Validates that coverage skips incompatible tests. This is a regression test for -# https://github.com/bazelbuild/bazel/issues/15385. +# Validates that coverage skips incompatible tests. This is a regression test +# for https://github.com/bazelbuild/bazel/issues/15385. function test_sh_test_coverage() { set_up_sh_test_coverage bazel coverage --test_output=all --combined_report=lcov //:all &>$TEST_log \ @@ -62,6 +83,18 @@ function test_sh_test_coverage() { expect_log "INFO: Build completed successfully" expect_log "//:compatible_test .* PASSED" expect_log "//:incompatible_test .* SKIPPED" + expect_log "Executed 1 out of 2 tests: 1 were skipped" +} + +# Validates that coverage correctly handles all tests being incompatible. +function test_sh_test_coverage() { + set_up_sh_test_coverage + bazel coverage --test_output=all --combined_report=lcov //all_incompatible:all &>$TEST_log \ + || fail "Coverage for //:all failed" + expect_log "INFO: Build completed successfully" + expect_log "//all_incompatible:incompatible1_test .* SKIPPED" + expect_log "//all_incompatible:incompatible2_test .* SKIPPED" + expect_log "Executed 0 out of 2 tests: 2 were skipped" } run_suite "test tests"