Skip to content

Fix null pointer crash with bazel coverage on only incompatible tests#15645

Closed
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:fix-all-incompatible-coverage
Closed

Fix null pointer crash with bazel coverage on only incompatible tests#15645
philsc wants to merge 1 commit intobazelbuild:masterfrom
philsc:fix-all-incompatible-coverage

Conversation

@philsc
Copy link
Copy Markdown
Contributor

@philsc philsc commented Jun 9, 2022

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.

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.
@philsc
Copy link
Copy Markdown
Contributor Author

philsc commented Jun 9, 2022

@c-mita, if you wouldn't mind taking a look, I'd appreciate it. This patch covers a code path I overlooked in my original patch (#15419).

@sgowroji sgowroji added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Jun 9, 2022
Copy link
Copy Markdown
Member

@c-mita c-mita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Rules-CPP Issues for C++ rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants