Skip to content

Introduce a gate/check GHA job #97533

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

Merged
merged 1 commit into from
Jul 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,60 @@ jobs:
run: make pythoninfo
- name: Tests
run: xvfb-run make buildbottest TESTOPTS="-j4 -uall,-cpu"

all-required-green: # This job does nothing and is only used for the branch protection
name: All required checks pass
if: always()
Copy link
Contributor Author

@webknjaz webknjaz Sep 24, 2022

Choose a reason for hiding this comment

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

This is mandatory because otherwise it'll get a skipped status in some cases and the branch protection sees that as skipped == success which is undesired.


needs:
- check_source # Transitive dependency, needed to access `run_tests` value
- check-docs
- check_generated_files
- build_win32
- build_win_amd64
- build_macos
- build_ubuntu
- build_ubuntu_ssltests
- test_hypothesis
- build_asan

runs-on: ubuntu-latest

steps:
- name: Check whether the needed jobs succeeded or failed
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe
with:
allowed-failures: >-
build_macos,
build_ubuntu_ssltests,
build_win32,
test_hypothesis,
allowed-skips: >-
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This setting tells the action that if the listed jobs (comma-separated) got skipped, that's okay. But the jobs are allowed to be skipped based on the same condition they've got set. If the first job requests test runs, this list will compute as empty and none of the jobs will be allowed to be skipped (meaning that skips would turn into failures).

Copy link
Member

Choose a reason for hiding this comment

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

Do I understand this right?

  • If check-docs starts, it must pass; if they skip, that's fine.

  • If check_generated_files or any of the other jobs in its group start, they must all pass; if they all skip, that's fine.

  • If test_hypothesis starts, it must pass; if they skip, that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hugovk almost: this is paired with the setting above ☝️ — essentially, if something is allowed to be skipped, and it is skipped, the action will treat it as non-failure.
But if something is allowed to be skipped and is not skipped, whether its failure is to be treated as such is controlled by the allowed-failures input.

Also, not they are not grouped inside the action, the templating here just produces a comma-separated list of things and the action doesn't know it was in some group externally. So yes, such a relation kinda exists, but I would say that it's indirect.

And since there's no “groups”, if any of the jobs that are allowed to be skipped run, each individual job is evaluated separately from the “group”, based on the fact of failures being allowed or not.

N.B. Whether a job is allowed to fail is also contributed by the continue-on-error setting, which is mostly useful for controlling individual matrix jobs. If a job was allowed to fail prior to this one, the action will not “see” if it was red and treat it as green.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • If test_hypothesis starts, it must pass; if they skip, that's fine.

With the context I added earlier, seeing that test_hypothesis is listed in allowed-failures unconditionally, it will never cause failures. This reflects the current branch protection configuration as seen in the Checks widget in PRs.

${{
!fromJSON(needs.check_source.outputs.run-docs)
&& '
check-docs,
'
|| ''
}}
${{
needs.check_source.outputs.run_tests != 'true'
&& '
check_generated_files,
build_win32,
build_win_amd64,
build_macos,
build_ubuntu,
build_ubuntu_ssltests,
build_asan,
'
|| ''
}}
${{
!fromJSON(needs.check_source.outputs.run_hypothesis)
&& '
test_hypothesis,
'
|| ''
}}
jobs: ${{ toJSON(needs) }}