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

[ci][microcheck] use commit instead of branch for failed prs #45384

Merged
merged 1 commit into from
May 16, 2024

Conversation

can-anyscale
Copy link
Collaborator

@can-anyscale can-anyscale commented May 16, 2024

Use commit instead of branch to identify failed prs, to account for all PR revisions. We seems to be under-coverage in large PRs where people make several bugs across multiple PR revisions.

Test:

  • CI

@can-anyscale can-anyscale requested a review from a team as a code owner May 16, 2024 13:46
@can-anyscale can-anyscale changed the title [ci][microcheck] use pull request instead of branch for failed prs [ci][microcheck] use commit instead of branch for failed prs May 16, 2024
stub_test_result(ResultStatus.ERROR, "w00t"),
stub_test_result(ResultStatus.SUCCESS, "hi"),
stub_test_result(ResultStatus.ERROR, "f00"),
stub_test_result(ResultStatus.ERROR, "w00t", commit="w00t.v1"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the commit looks like "woot.v1" here? does not really look like a commit?

Comment on lines +146 to +148
Get the failed PRs for a test. We use the commit to account for all revisions
of a PR.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not returning set of "PRs" any more? should we change it to _get_failed_runs or _get_failed_commits?

@can-anyscale
Copy link
Collaborator Author

update the function names from prs to commits; acknowledge that this change will bias PRs that have commits more; however, since we are using a very high coverage number (95-100), the bias is minimal vs. the required increase coverage for microcheck to catch more issues

@can-anyscale can-anyscale enabled auto-merge (squash) May 16, 2024 18:59
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label May 16, 2024
@can-anyscale can-anyscale merged commit 856453f into master May 16, 2024
8 checks passed
@can-anyscale can-anyscale deleted the can-mcx06 branch May 16, 2024 19:15
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 6, 2024
…ject#45384)

Use commit instead of branch to identify failed prs, to account for all
PR revisions. We seems to be under-coverage in large PRs where people
make several bugs across multiple PR revisions.

Test:
- CI

Signed-off-by: can <[email protected]>
Signed-off-by: Ryan O'Leary <[email protected]>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 6, 2024
…ject#45384)

Use commit instead of branch to identify failed prs, to account for all
PR revisions. We seems to be under-coverage in large PRs where people
make several bugs across multiple PR revisions.

Test:
- CI

Signed-off-by: can <[email protected]>
Signed-off-by: Ryan O'Leary <[email protected]>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Jun 7, 2024
…ject#45384)

Use commit instead of branch to identify failed prs, to account for all
PR revisions. We seems to be under-coverage in large PRs where people
make several bugs across multiple PR revisions.

Test:
- CI

Signed-off-by: can <[email protected]>
GabeChurch pushed a commit to GabeChurch/ray that referenced this pull request Jun 11, 2024
…ject#45384)

Use commit instead of branch to identify failed prs, to account for all
PR revisions. We seems to be under-coverage in large PRs where people
make several bugs across multiple PR revisions.

Test:
- CI

Signed-off-by: can <[email protected]>
Signed-off-by: gchurch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants