-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Announce combined coverage report on the BES #22171
Conversation
@coeuvre Could you review this? |
The feature makes perfect sense and I'm excited to see it submitted! I do think we can implement it without adding a new BEP event type, which is always preferred when possible. In particular, the Contributing a file to this collection is a bit tricky, but the hooks are there to do this. In general you will extend One roadmap, which is preferred but has a few missing details that you'd need to fill in, is:
Another roadmap is:
|
Could code coverage collection be done before the absolute end of the build, and thus would be delayed by adding it to |
RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event.
This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally. Also removes an unused `CoverageReport` event that appears to have become obsolete. RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event.
This reverts commit 62f5e78.
@michaeledgar I implemented your first suggestion and could also look into #21475, but any insights from your side would be highly appreciated. |
@michaeledgar Gentle ping, would be great to get this into 7.2.0 |
@brentleyjones - So for #21745, please correct me if I'm wrong! But it seems to be dealing with the code coverage artifacts produced by individual targets. If accurate, those should definitely not be delayed until the end of the build (skymeld or no skymeld) and the fix is definitely to make sure those events are posted promptly! For this issue: My understanding after the first review is that it aims to convey a coverage report that is only available after the execution phase is complete. (In the first version, the event was posted after @meum - Commit 680facf looks great to me! It may not surprise you to learn that this functionality already exists inside Blaze, with a few extra hacks that prevent its direct open-sourcing. But we can someday mash those hacks to fit the open-source contribution you've made, and will have extra motivation to do so now! |
@bazel-io fork 7.2.0 |
This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally. Also removes an obsolete `CoverageReport` event. RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. Closes bazelbuild#22171. PiperOrigin-RevId: 632641499 Change-Id: I0c323a371ec2fdd085bea23a772a85b72c52093f
This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally. Also removes an obsolete `CoverageReport` event. RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. Closes #22171. PiperOrigin-RevId: 632641499 Change-Id: I0c323a371ec2fdd085bea23a772a85b72c52093f Commit bb1fb53 Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally. Also removes an obsolete `CoverageReport` event. RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. Closes bazelbuild#22171. PiperOrigin-RevId: 632641499 Change-Id: I0c323a371ec2fdd085bea23a772a85b72c52093f
This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally.
Also removes an obsolete
CoverageReport
event.RELNOTES: The combined coverage report produced via
--combined_report=lcov
is now announced on the BES via the newCoverageReport
event.