Skip to content

Commit 39f6776

Browse files
committed
CI: keep one-OS dispatches off ci-ok
A dispatch that ran only one OS still reported `ci-ok`, the required check, on the branch head, so a Windows-only run could make a PR whose Linux jobs failed look green. Such a run now reports as `ci-ok (<os> only)`; every other run still reports `ci-ok`. Dispatches for different OSes on one branch also shared a concurrency group, so a third dispatch replaced the pending second one. Each OS-filtered dispatch now gets its own group. Assisted-by: Claude Code:claude-opus-5-5
1 parent 1943512 commit 39f6776

3 files changed

Lines changed: 28 additions & 10 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@ on:
3333
# Run only one OS's jobs (plus the clippy and release-readiness
3434
# preflights every leg waits on). A fix for a Windows-only nightly
3535
# failure dispatches `-f os=windows` instead of re-running ~95 Linux
36-
# and macOS jobs its PR run and the merge queue already cover. Only
37-
# a dispatch sets inputs.os; on every other event it is empty, so
38-
# the `inputs.os != ...` terms below are always true there.
36+
# and macOS jobs (~140 job-minutes) its PR run and the merge queue
37+
# already cover. hosted-e2e is a Linux job and skips too. Only a
38+
# dispatch sets inputs.os; on every other event it is empty, so the
39+
# `inputs.os != ...` terms below are always true there. Each
40+
# OS-filtered dispatch gets its own concurrency group, so
41+
# dispatching one per OS runs them all.
3942
description: 'Only run jobs for this OS (all = the full nightly matrix)'
4043
type: choice
4144
default: all
@@ -56,7 +59,7 @@ permissions:
5659
# cancelled the real run, skipped every job, and left ci-ok green on a head
5760
# nothing had tested.
5861
concurrency:
59-
group: ci-${{ github.event.pull_request.number || (github.event_name == 'push' && github.sha) || github.ref }}${{ github.event_name == 'schedule' && '-nightly' || '' }}${{ github.event.pull_request.draft && '-draft' || '' }}
62+
group: ci-${{ github.event.pull_request.number || (github.event_name == 'push' && github.sha) || github.ref }}${{ github.event_name == 'schedule' && '-nightly' || '' }}${{ github.event.pull_request.draft && '-draft' || '' }}${{ inputs.os && inputs.os != 'all' && format('-{0}', inputs.os) || '' }}
6063
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
6164

6265
# Linux jobs run on Depot's runners (depot-ubuntu-*-4: 4 vCPU, like GitHub's
@@ -2895,7 +2898,11 @@ jobs:
28952898
# every job above, so adding a job here is how it becomes merge-blocking.
28962899
# Skipped jobs (the nightly `full` tier) pass; failed or cancelled ones fail.
28972900
ci-ok:
2898-
name: ci-ok # registered as a required check; do not rename
2901+
# Registered as a required check; do not rename. A one-OS dispatch
2902+
# (inputs.os, #1818) skips the other OSes, so its verdict gets another
2903+
# name: a green `ci-ok` on the branch head must mean every PR-tier job
2904+
# passed, and a skipped required check would count as passing too.
2905+
name: ${{ inputs.os && inputs.os != 'all' && format('ci-ok ({0} only)', inputs.os) || 'ci-ok' }}
28992906
if: always()
29002907
needs: [clippy, node-addon, lint-ecosystems, release-readiness, test, test-release, coverage, docker-base, coverage-docker, coverage-merge, dispatch-tests, e2e-build, windows-compile-check, e2e-build-windows, e2e-build-macos, e2e, e2e-windows, e2e-extended, e2e-full, e2e-docker, yarn-classic-matrix, yarn-berry-e2e, yarn-berry-full, cargo-vex-matrix, cargo-vex-matrix-windows, cargo-vex-matrix-full, cargo-old-toolchains, hosted-e2e, e2e-macos, yarn-berry-e2e-macos, cargo-vex-matrix-macos]
29012908
# GitHub-hosted, not Depot: the check is one 2 s python3 call that runs

‎scripts/tests/test_ci_e2e_tiers.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,15 @@ def test_full_jobs_run_nightly_only_and_share_steps(self):
7777
# Nightly, dispatch (release-qa) and v5 landings, like
7878
# e2e-docker: not on PRs, the merge queue or a lean push to
7979
# main, where its 41 legs ran on every landing.
80-
# A job with no OS-gated ancestor carries the Linux-only
81-
# term for `workflow_dispatch -f os=...` (#1818).
82-
gate = ("github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'"
80+
# yarn-berry-full waits only on clippy, so it carries the
81+
# Linux-only term for `workflow_dispatch -f os=...` itself
82+
# (#1818); the others skip with e2e-build.
83+
gate = (" if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'"
8384
" || (github.event_name == 'push' && vars.CI_SCOPE == 'full')"
8485
" || github.head_ref == 'release/v5-prerelease'")
85-
self.assertTrue(f" if: {gate}\n" in text or f" if: ({gate}) && (inputs.os != 'windows'"
86-
" && inputs.os != 'macos')\n" in text, text)
86+
if job == "yarn-berry-full":
87+
gate = gate.replace(" if: ", " if: (") + ") && (inputs.os != 'windows' && inputs.os != 'macos')"
88+
self.assertIn(gate + "\n", text)
8789
self.assertIn(f"steps: *{anchor}", text)
8890
self.assertIn(f"steps: &{anchor}", TEXT)
8991

‎scripts/tests/test_ci_scheduling.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,15 @@ def test_late_draft_run_cannot_cancel_the_ready_run(self):
298298
# ci-ok green with every job skipped.
299299
group = re.search(r"^concurrency:\n group: (.*)$", TEXT, re.M)[1]
300300
self.assertIn("github.event.pull_request.draft && '-draft'", group)
301+
# One dispatch per OS must not cancel the pending one before it.
302+
self.assertIn("${{ inputs.os && inputs.os != 'all' && format('-{0}', inputs.os) || '' }}", group)
303+
304+
def test_one_os_dispatch_cannot_post_a_green_ci_ok(self):
305+
# A dispatch on a PR head that skipped the other OSes must not report
306+
# under the required check's name (#1818); every other event does.
307+
names = [line for line in JOBS["ci-ok"] if line.startswith(" name:")]
308+
self.assertEqual(names, [" name: ${{ inputs.os && inputs.os != 'all'"
309+
" && format('ci-ok ({0} only)', inputs.os) || 'ci-ok' }}"])
301310

302311
def test_row_reader_preserves_both_os_siblings(self):
303312
jobs = reader.jobs("""jobs:

0 commit comments

Comments
 (0)