Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

philsc
Copy link
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
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
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