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

Test results BEP events are delayed when using coverage and Skymeld is enabled #21475

Closed
brentleyjones opened this issue Feb 22, 2024 · 5 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Performance Issues for Performance teams type: bug

Comments

@brentleyjones
Copy link
Contributor

brentleyjones commented Feb 22, 2024

Description of the bug:

In Bazel 7, when using --experimental_merged_skyframe_analysis_execution (the default), and using bazel coverage (but not bazel test), test results are delayed until the end of the build. If using --noexperimental_merged_skyframe_analysis_execution, or using bazel test, test results appear as soon as the tests are done running.

Which category does this issue belong to?

Core

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

$ git checkout git@github.com:bazelbuild/bazel.git
$ cd bazel
$ USE_BAZEL_VERSION=7.0.2 bazel coverage --bes_results_url=https://app.buildbuddy.io/invocation/ --bes_backend=grpcs://remote.buildbuddy.io --cache_test_results=no //src/test/java/com/google/devtools/build/lib/...
$ USE_BAZEL_VERSION=7.0.2 bazel clean
$ USE_BAZEL_VERSION=7.0.2 bazel coverage --bes_results_url=https://app.buildbuddy.io/invocation/ --bes_backend=grpcs://remote.buildbuddy.io --cache_test_results=no --noexperimental_merged_skyframe_analysis_execution //src/test/java/com/google/devtools/build/lib/...

In the first run, testResult events aren't published until the end of the build. In the second run they are published as soon as they are done running.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

I saw the message in 37247d5, and it seems that there was a change regarding how the combined report was generated. Maybe that is delaying these related events?

Any other information, logs, or outputs that you want to share?

No response

@github-actions github-actions bot added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Feb 22, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 22, 2024

Cc @joeleba

@joeleba joeleba added team-Performance Issues for Performance teams and removed team-Core Skyframe, bazel query, BEP, options parsing, bazelrc labels Feb 23, 2024
@meisterT meisterT changed the title Test results BEP events are delayed when using coverage and Skyframe is enabled Test results BEP events are delayed when using coverage and Skymeld is enabled Feb 27, 2024
@joeleba joeleba added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Feb 27, 2024
@joeleba
Copy link
Member

joeleba commented Apr 17, 2024

@brentleyjones thanks for filing the bug. This is because of c2a6f0c. Realistically speaking I don't think I can invest time on this anytime soon. Is this issue blocking you or it's mostly an annoyance?

@sluongng
Copy link
Contributor

We do have customer(s) affected by this. They are currently having to apply a workaround which is to turn off Skymeld via the experiment flag as Brentley mentioned above. To quote one customer directly:

I think there is a regression in Bazel 7 related to using bazel coverage and BES - the test results seem to be only reported on BES after all tests completes, rather than as soon as each test finishes. This is visible in BuildBuddy UI by the fact that the list of passed tests (eg. in the targets tab) don't show up anymore as soon as each test finishes, but rather only all at once at the end of the run. (We also consume it via --build_event_json_file in our custom wrapper, and we see the same issue there)

@joeleba do you have pointers on how this should be fixed?

@brentleyjones
Copy link
Contributor Author

brentleyjones commented Apr 17, 2024

I originally filed this for a BuildBuddy customer. That customer or someone from BuildBuddy will have to speak to that.

As for my new employer, this is a semi-block. We use these test results to report information to CI. Having these delayed greatly impacts our Time to First Feedback metrics.

bazel-io pushed a commit to bazel-io/bazel that referenced this issue May 7, 2024
Baseline coverage artifacts are now requested in `CompletionFunction` to ensure that they are built before the `TargetCompleteEvent` is generated. This makes it unnecessary to delay sending these events until after a Skymeld build has had the chance to request all coverage artifacts directly, which could only be done after the analysis & execution phase, thus delaying events until the end of the build.

Fixes bazelbuild#21475

Closes bazelbuild#22238.

PiperOrigin-RevId: 631414420
Change-Id: Idc77b6f5c8b5b775e6c69e35c5563f63b3bf974f
joeleba pushed a commit to joeleba/bazel that referenced this issue May 10, 2024
Baseline coverage artifacts are now requested in `CompletionFunction` to ensure that they are built before the `TargetCompleteEvent` is generated. This makes it unnecessary to delay sending these events until after a Skymeld build has had the chance to request all coverage artifacts directly, which could only be done after the analysis & execution phase, thus delaying events until the end of the build.

Fixes bazelbuild#21475

Closes bazelbuild#22238.

PiperOrigin-RevId: 631414420
Change-Id: Idc77b6f5c8b5b775e6c69e35c5563f63b3bf974f
joeleba pushed a commit to joeleba/bazel that referenced this issue May 10, 2024
Baseline coverage artifacts are now requested in `CompletionFunction` to ensure that they are built before the `TargetCompleteEvent` is generated. This makes it unnecessary to delay sending these events until after a Skymeld build has had the chance to request all coverage artifacts directly, which could only be done after the analysis & execution phase, thus delaying events until the end of the build.

Fixes bazelbuild#21475

Closes bazelbuild#22238.

PiperOrigin-RevId: 631414420
Change-Id: Idc77b6f5c8b5b775e6c69e35c5563f63b3bf974f
Kila2 pushed a commit to Kila2/bazel that referenced this issue May 13, 2024
Baseline coverage artifacts are now requested in `CompletionFunction` to ensure that they are built before the `TargetCompleteEvent` is generated. This makes it unnecessary to delay sending these events until after a Skymeld build has had the chance to request all coverage artifacts directly, which could only be done after the analysis & execution phase, thus delaying events until the end of the build.

Fixes bazelbuild#21475

Closes bazelbuild#22238.

PiperOrigin-RevId: 631414420
Change-Id: Idc77b6f5c8b5b775e6c69e35c5563f63b3bf974f
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.2.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.2.0rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Performance Issues for Performance teams type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants