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

Make bazel coverage work with minimal mode #16556

Closed
wants to merge 3 commits into from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Oct 26, 2022

This PR solves the problem in a different way that #16475 tries to solve:

  1. Use ActionFileSystem to read directory output for coverage data.
  2. Fire event CoverageReport in the action after the coverage report is generated and listen to it in ToplevelArtifactsDownloader to download the report.

@coeuvre coeuvre requested a review from a team as a code owner October 26, 2022 11:47
@coeuvre coeuvre marked this pull request as draft October 26, 2022 11:48
@sgowroji sgowroji added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Oct 26, 2022
copybara-service bot pushed a commit that referenced this pull request Nov 8, 2022
So spawns can read content of directires within action exuection.

Part of #16556.

PiperOrigin-RevId: 486918859
Change-Id: Ida86e4c927093d26f7f96d2f0c2aa0d1d74cc8a4
@coeuvre coeuvre marked this pull request as ready for review November 9, 2022 14:18
@coeuvre coeuvre requested a review from tjgq November 9, 2022 14:18
ShreeM01 added a commit that referenced this pull request Nov 10, 2022
…#16738)

So spawns can read content of directires within action exuection.

Part of #16556.

PiperOrigin-RevId: 486918859
Change-Id: Ida86e4c927093d26f7f96d2f0c2aa0d1d74cc8a4

Co-authored-by: Googler <[email protected]>
@brentleyjones
Copy link
Contributor

@coeuvre @tjgq Do you think we can get this in, and possibly into 6.0?

@sgowroji sgowroji added the awaiting-review PR is awaiting review from an assigned reviewer label Dec 4, 2022
@brentleyjones
Copy link
Contributor

@coeuvre @tjgq friendly ping

@brentleyjones
Copy link
Contributor

@coeuvre Can this get cherry-picked into 6.1.0?

@coeuvre
Copy link
Member Author

coeuvre commented Jan 18, 2023

Yes, I believe this is a safe change.

@coeuvre coeuvre deleted the coverage-bwob branch January 18, 2023 14:06
@coeuvre
Copy link
Member Author

coeuvre commented Jan 18, 2023

@bazel-io fork 6.1.0

coeuvre added a commit to coeuvre/bazel that referenced this pull request Feb 2, 2023
This PR solves the problem in a different way that bazelbuild#16475 tries to solve:

1. bazelbuild#16812 allows skyframe read metadata from ActionFS.
2. Use `ActionFileSystem` to check existence of coverage data.
3. Fire event `CoverageReport` in the action after the coverage report is generated and listen to it in `ToplevelArtifactsDownloader` to download the report.

Closes bazelbuild#16556.

PiperOrigin-RevId: 502854552
Change-Id: I2796baaa962857831ff161423be6dffa6eb73e5c
ShreeM01 added a commit that referenced this pull request Feb 7, 2023
* Returns null if filesystem of test outputs is not ActionFS when processing test attempt event.

Previously, we assert that the filesystem of test outputs is ActionFS when we are processing test attempt event. However this is not true when the test hits action cache.

This CL looses the check to return null.

PiperOrigin-RevId: 501023752
Change-Id: I17cbb26e0a2b5fd30cb781818e42172ac672919e

* Make `bazel coverage` work with minimal mode

This PR solves the problem in a different way that #16475 tries to solve:

1. #16812 allows skyframe read metadata from ActionFS.
2. Use `ActionFileSystem` to check existence of coverage data.
3. Fire event `CoverageReport` in the action after the coverage report is generated and listen to it in `ToplevelArtifactsDownloader` to download the report.

Closes #16556.

PiperOrigin-RevId: 502854552
Change-Id: I2796baaa962857831ff161423be6dffa6eb73e5c

---------

Co-authored-by: kshyanashree <[email protected]>
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
This PR solves the problem in a different way that #16475 tries to solve:

1. #16812 allows skyframe read metadata from ActionFS.
2. Use `ActionFileSystem` to check existence of coverage data.
3. Fire event `CoverageReport` in the action after the coverage report is generated and listen to it in `ToplevelArtifactsDownloader` to download the report.

Closes #16556.

PiperOrigin-RevId: 502854552
Change-Id: I2796baaa962857831ff161423be6dffa6eb73e5c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants