[bug, CI] fix issues with forks, force green, and concurrency in codecov report uploading #2225
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes 4 issues with the recent codecov roll-out, where fork testing did not cover all scenarios occurring in the main repo:
Most importantly, it fixes issues with pull requests, where PRs from forks are not being properly uploaded to codecov (and defaulting to main). Unfortunately default github webhooks for workflow_run are not complete enough to get the requisite pull request information unless the PR comes from the repo itself. A workaround uses the github cli along with other more reliable aspects of the webhook (branch name, name and full_name) to construct the necessary information to get the PR number.
Default settings of codecov can lead to failures in CI. As discussed internally, we would like this not to be enforced to start. A configuration yaml file is added to
.github
with very conservative settings. In the case that enforcement will be added, all the necessary information is already included.Concurrency protection is not necessary for this github action: it is a service to PRs and pushes by accomplishing higher-security tasks (i.e. with secrets) on their behalf without using the PR/push code (see ultralytics hack from this week as to why pull_request_target must be avoided). Multiple PRs may run the same action at the same time, and so concurrency is a feature.
If multiple pushes to main occur at the same time, it was using only the latest SHA which comes from the repo, which will not properly map into codecov, instead use the head_ref SHA so that the older pushes maintains their SHAs.
See:
icfaust#8
icfaust#7
PR should start as a draft, then move to ready for review state after CI is passed and all applicable checkboxes are closed.
This approach ensures that reviewers don't spend extra time asking for regular requirements.
You can remove a checkbox as not applicable only if it doesn't relate to this PR in any way.
For example, PR with docs update doesn't require checkboxes for performance while PR with any change in actual code should have checkboxes and justify how this code change is expected to affect performance (or justification should be self-evident).
Checklist to comply with before moving PR from draft:
PR completeness and readability
Testing