ci: rework label-gated suites as advisory checks + fix label concurrency#379
Draft
nebasuke wants to merge 3 commits into
Draft
ci: rework label-gated suites as advisory checks + fix label concurrency#379nebasuke wants to merge 3 commits into
nebasuke wants to merge 3 commits into
Conversation
coverage, integration-tests, sanitizer and slang-tests are opt-in extra coverage gated on `ci:*` labels — you add the label to run them and read the result on the PR, but they should not block merging. Bring all four to one simple, advisory shape: - Drop the dummy `label-check` job; inline its label gate directly onto `cooldown-check` so the dependency graph has one less skipped node. - Remove the `pr-checks` (re-actors/alls-green) aggregators and their `allowed-skips`. Those only existed to expose a stable *required* check that turned green-via-skip when the label was absent — actively misleading for an advisory suite (a green that means "didn't run"). The real job now posts its own status; absent label => clearly skipped. - Drop the `merge_group:` triggers so these no longer run in / gate the merge queue. - Drop `checks: write` on sanitizer/slang, added with the aggregator and now unused. - Standardize the concurrency key across all four. BRANCH PROTECTION: remove PR Checks (Coverage/Integration/Sanitizer/Slang) from the required-status-checks list together with this change. sanitizer and slang previously reported in the merge queue via merge_group; if they stay required after the trigger is removed, the merge queue blocks on checks that no longer report.
A `labeled` event fires once per label added, and GitHub can't filter trigger events by label name, so each label-gated suite re-triggers on labels it doesn't gate on. With a shared concurrency group those no-op runs cancel the real run started by the matching label — so adding several labels at once (or a second label while one is in flight) could kill an opted-in run mid-test. Put the triggering label into the concurrency group: a `labeled` event for an unrelated label lands in its own group and can't touch the matching label's run. Other PR events (push/synchronize) share the `main` group and still cancel-on-new. The opted-in suite now reliably runs to completion.
release.yaml has no concurrency block and gates the PR dry-run on plain `contains(labels, 'ci:release')`, so every `labeled` event while the label is present launched a fresh full multi-platform build — adding several labels at once could kick off multiple concurrent release pipelines. Add a per-PR concurrency group that collapses the dry-runs into one run. cancel-in-progress is gated on `pull_request` so real releases (tag push / workflow_dispatch) get a per-ref group and are never cancelled.
260dd1c to
2f84111
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The four label-gated suites (
coverage,integration-tests,sanitizer,slang-tests) are advisory opt-in checks, not merge gates: add theci:*label to run a suite (e.g. the long integration tests) and read the result on the PR, but they don't block merging or run in the merge queue. This PR unifies them on one simple advisory shape and fixes a label-concurrency bug that could silently cancel an opted-in run.Changes
6a48dc45): drop the dummylabel-checkjob (gate inlined ontocooldown-check); remove thepr-checks/re-actors/alls-greenaggregators and theirallowed-skips, themerge_group:triggers, and the now-unusedchecks: writeon sanitizer/slang. Each suite posts its own status; absent label → clearly skipped rather than a misleading green-via-skip.8d50c89e): put the triggering label into the concurrency group, so alabeledevent for an unrelated label lands in its own group and can't cancel the run started by the matching label. Fixes the "add two labels → one suite gets cancelled/skipped" flakiness. Other PR events (push/synchronize) share themaingroup and cancel-on-new as before.2f841114):release.yamlhad no concurrency block and gates the dry-run on plaincontains(labels, 'ci:release'), so everylabeledevent launched a fresh full multi-platform build. Add a per-PR concurrency group;cancel-in-progressis gated onpull_requestso real releases (tag push /workflow_dispatch) get a per-ref group and are never cancelled.Remove
PR Checks (Coverage),PR Checks (Integration),PR Checks (Sanitizer),PR Checks (Slang)from the required status checks list.sanitizerandslangpreviously reported in the merge queue viamerge_group; if they stay required after that trigger is removed, the merge queue will block on checks that never report.Test plan
ci:*label: the four suites show as skipped; nothing blocks merge.ci:integration, thenci:coverage: both real runs complete; neither cancels the other.ci:*labels at once: each suite's real run completes.unlabeledis not a trigger).release.yaml: severalci:*labels at once → one dry-run pipeline, not several.