Skip to content

Commit

Permalink
Fix null pointer crash with bazel coverage on only incompatible tests
Browse files Browse the repository at this point in the history
This is a follow-up to 2f1ff6f. 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.<init>(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.

Closes #15645.

PiperOrigin-RevId: 454132787
Change-Id: Id1cd4109f96d90bbdc114b0bbe7f5c5046d47c27
  • Loading branch information
philsc authored and copybara-github committed Jun 10, 2022
1 parent c945c16 commit 3dd2b93
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,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<Artifact> coverageArtifacts = builder.build();
Expand Down
37 changes: 35 additions & 2 deletions src/test/shell/bazel/bazel_coverage_compatibility_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ constraint_setting(name = "incompatible_setting")
constraint_value(
name = "incompatible",
constraint_setting = ":incompatible_setting",
visibility = ["//visibility:public"],
)
sh_test(
Expand All @@ -40,6 +41,11 @@ sh_test(
srcs = ["incompatible_test.sh"],
target_compatible_with = [":incompatible"],
)
exports_files([
"compatible_test.sh",
"incompatible_test.sh",
])
EOF
cat <<EOF > compatible_test.sh
#!/bin/bash
Expand All @@ -51,17 +57,44 @@ exit 1
EOF
chmod +x compatible_test.sh
chmod +x incompatible_test.sh

mkdir all_incompatible/
cat <<EOF > 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 \
|| fail "Coverage for //:all failed"
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"

0 comments on commit 3dd2b93

Please sign in to comment.