Skip to content

Commit 6cd28a2

Browse files
committed
ci: move the ci gate into a tested script and reject non-boolean change flags
The ci aggregator's skip rules now live in .github/scripts/ci-gate.sh, read from NEEDS, with table-driven tests covering every guard. The change-classifier test helper fails on any flag value other than exactly true or false.
1 parent cdd4ec6 commit 6cd28a2

4 files changed

Lines changed: 231 additions & 19 deletions

File tree

‎.github/scripts/ci-gate.sh‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#!/usr/bin/env bash
2+
# The `ci` job's verdict: the single required check, so it must never pass a run whose checks did
3+
# not all pass. Reads the job's `needs` context (`toJSON(needs)`) from $NEEDS and exits non-zero,
4+
# naming each offending job, unless every job succeeded or was skipped as `changes` intended.
5+
#
6+
# A skip passes only for a gated job whose `changes` flag is exactly `false`:
7+
#
8+
# integration, e2e full
9+
# test test
10+
# build build
11+
# desktop-live desktop_live
12+
#
13+
# Every other skip fails: an ungated job (lint, `changes` itself), a gated job whose flag is
14+
# `true`, missing, or anything else (a failed or skipped `changes` leaves no outputs). A failure
15+
# or a cancellation always fails.
16+
#
17+
# Usage: NEEDS='<toJSON(needs)>' ci-gate.sh
18+
set -euo pipefail
19+
20+
needs=${NEEDS:-}
21+
if ! jq -e 'type == "object"' <<< "$needs" > /dev/null 2>&1; then
22+
echo "::error::The needs context is not a JSON object: '$needs'"
23+
exit 1
24+
fi
25+
26+
failed="$(jq -r '
27+
{"integration": "full", "e2e": "full", "test": "test", "build": "build", "desktop-live": "desktop_live"} as $gate
28+
| (.changes.outputs // {}) as $flags
29+
| to_entries[]
30+
| select(.value.result != "success")
31+
| select(.value.result != "skipped" or $gate[.key] == null or $flags[$gate[.key]] != "false")
32+
| "\(.key): \(.value.result)"
33+
' <<< "$needs")"
34+
if [ -n "$failed" ]; then
35+
echo "::error::Checks did not pass:"
36+
echo "$failed"
37+
exit 1
38+
fi
39+
echo "All checks passed."

‎.github/workflows/checks.yml‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -725,9 +725,10 @@ jobs:
725725
run: bunx turbo run build --filter=@sim/app
726726

727727
# One status for every check above: the single check to require on a branch ruleset, so adding,
728-
# sharding or renaming a job never means editing the ruleset. A skip passes only for a gated job
729-
# whose `changes` flag is exactly `false`; any other skip (a failed or skipped `changes`, a lost
730-
# output, an ungated job) fails, as does a failure or a cancellation.
728+
# sharding or renaming a job never means editing the ruleset. .github/scripts/ci-gate.sh decides:
729+
# a skip passes only for a gated job whose `changes` flag is exactly `false`; any other skip (a
730+
# failed or skipped `changes`, a lost output, an ungated job) fails, as does a failure or a
731+
# cancellation. A failed checkout fails the gate too.
731732
# `always()`, not `!cancelled()`: a skipped job satisfies a required check, so a gate that skips
732733
# on a cancelled run would report a cancelled head commit as passing.
733734
ci:
@@ -737,21 +738,9 @@ jobs:
737738
runs-on: *runner-2vcpu
738739
timeout-minutes: 5
739740
steps:
741+
- *checkout-mirror
742+
- *checkout-plain
740743
- name: Require every check to pass
741744
env:
742-
RESULTS: ${{ toJSON(needs) }}
743-
run: |
744-
failed="$(jq -r '
745-
{"integration": "full", "e2e": "full", "test": "test", "build": "build", "desktop-live": "desktop_live"} as $gate
746-
| (.changes.outputs // {}) as $flags
747-
| to_entries[]
748-
| select(.value.result != "success")
749-
| select(.value.result != "skipped" or $gate[.key] == null or $flags[$gate[.key]] != "false")
750-
| "\(.key): \(.value.result)"
751-
' <<< "$RESULTS")"
752-
if [ -n "$failed" ]; then
753-
echo "::error::Checks did not pass:"
754-
echo "$failed"
755-
exit 1
756-
fi
757-
echo "All checks passed."
745+
NEEDS: ${{ toJSON(needs) }}
746+
run: bash .github/scripts/ci-gate.sh

‎scripts/ci-changes.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ function classify(repo: string, base: string): Flags {
4747
const result = spawnSync('bash', [script, base], { cwd: repo, encoding: 'utf8' })
4848
expect(result.status, result.stderr).toBe(0)
4949
const lines = result.stdout.trim().split('\n')
50+
for (const line of lines) expect(line).toMatch(/^[a-z_]+=(true|false)$/)
5051
const flags = Object.fromEntries(lines.map((line) => line.split('=')))
52+
expect(lines).toHaveLength(4)
5153
expect(Object.keys(flags).sort()).toEqual(['build', 'desktop_live', 'full', 'test'])
5254
return {
5355
full: flags.full === 'true',

‎scripts/ci-gate.test.ts‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
import { spawnSync } from 'node:child_process'
2+
import { fileURLToPath } from 'node:url'
3+
import { describe, expect, it } from 'vitest'
4+
5+
const script = fileURLToPath(new URL('../.github/scripts/ci-gate.sh', import.meta.url))
6+
7+
type Result = 'success' | 'failure' | 'cancelled' | 'skipped'
8+
type Job = 'changes' | 'integration' | 'e2e' | 'desktop-live' | 'lint' | 'test' | 'build'
9+
type Results = Partial<Record<Job, Result>>
10+
11+
const GATED: Job[] = ['integration', 'e2e', 'desktop-live', 'test', 'build']
12+
const ALL_FALSE = { full: 'false', test: 'false', build: 'false', desktop_live: 'false' }
13+
14+
/**
15+
* The `needs` context GitHub gives the `ci` job: every job succeeded and `changes` set `outputs`,
16+
* with `results` overriding individual jobs.
17+
*/
18+
function needs(outputs: unknown, results: Results = {}) {
19+
const jobs: Job[] = ['changes', ...GATED, 'lint']
20+
return Object.fromEntries(
21+
jobs.map((job) => [
22+
job,
23+
{
24+
result: results[job] ?? 'success',
25+
outputs: job === 'changes' ? outputs : {},
26+
},
27+
])
28+
)
29+
}
30+
31+
const skipGated: Results = Object.fromEntries(GATED.map((job) => [job, 'skipped' as const]))
32+
33+
interface FailureCase {
34+
name: string
35+
outputs: unknown
36+
results: Results
37+
reported: string[]
38+
}
39+
40+
/** Runs the gate; `reported` holds the `<job>: <result>` lines it failed on. */
41+
function gate(context: string) {
42+
const result = spawnSync('bash', [script], {
43+
encoding: 'utf8',
44+
env: { PATH: process.env.PATH, NEEDS: context },
45+
})
46+
const reported = result.stdout
47+
.split('\n')
48+
.filter((line) => /^[a-z0-9-]+: [a-z]+$/.test(line))
49+
.sort()
50+
return { status: result.status, stdout: result.stdout, reported }
51+
}
52+
53+
describe('CI gate', () => {
54+
it('passes when every job succeeded', () => {
55+
const { status, stdout } = gate(JSON.stringify(needs(ALL_FALSE)))
56+
expect(status).toBe(0)
57+
expect(stdout).toBe('All checks passed.\n')
58+
})
59+
60+
it('passes gated jobs skipped because their flag is exactly false', () => {
61+
const { status, stdout } = gate(JSON.stringify(needs(ALL_FALSE, skipGated)))
62+
expect(status).toBe(0)
63+
expect(stdout).toBe('All checks passed.\n')
64+
})
65+
66+
it.each<FailureCase>([
67+
{
68+
name: 'a skip whose flag is true',
69+
outputs: { ...ALL_FALSE, build: 'true' },
70+
results: skipGated,
71+
reported: ['build: skipped'],
72+
},
73+
{
74+
name: 'a skip whose flag is missing',
75+
outputs: { full: 'false', test: 'false', desktop_live: 'false' },
76+
results: skipGated,
77+
reported: ['build: skipped'],
78+
},
79+
{
80+
name: 'a skip whose flag is a misspelling',
81+
outputs: { ...ALL_FALSE, full: 'flase' },
82+
results: skipGated,
83+
reported: ['e2e: skipped', 'integration: skipped'],
84+
},
85+
{
86+
name: 'a skip whose flag is differently cased',
87+
outputs: { ...ALL_FALSE, test: 'False' },
88+
results: skipGated,
89+
reported: ['test: skipped'],
90+
},
91+
{
92+
name: 'a skip whose flag is empty',
93+
outputs: { ...ALL_FALSE, desktop_live: '' },
94+
results: skipGated,
95+
reported: ['desktop-live: skipped'],
96+
},
97+
{
98+
name: 'a skip whose flag is a boolean, not the string GitHub sets',
99+
outputs: { ...ALL_FALSE, build: false },
100+
results: skipGated,
101+
reported: ['build: skipped'],
102+
},
103+
{
104+
name: 'skips with no changes outputs',
105+
outputs: {},
106+
results: skipGated,
107+
reported: [
108+
'build: skipped',
109+
'desktop-live: skipped',
110+
'e2e: skipped',
111+
'integration: skipped',
112+
'test: skipped',
113+
],
114+
},
115+
{
116+
name: 'skips with a null changes outputs',
117+
outputs: null,
118+
results: skipGated,
119+
reported: [
120+
'build: skipped',
121+
'desktop-live: skipped',
122+
'e2e: skipped',
123+
'integration: skipped',
124+
'test: skipped',
125+
],
126+
},
127+
{
128+
name: 'a failed changes job',
129+
outputs: {},
130+
results: { ...skipGated, changes: 'failure' },
131+
reported: [
132+
'build: skipped',
133+
'changes: failure',
134+
'desktop-live: skipped',
135+
'e2e: skipped',
136+
'integration: skipped',
137+
'test: skipped',
138+
],
139+
},
140+
{
141+
name: 'a skipped changes job, even with flags that would allow the skips',
142+
outputs: ALL_FALSE,
143+
results: { ...skipGated, changes: 'skipped' },
144+
reported: ['changes: skipped'],
145+
},
146+
{
147+
name: 'a failed job whose flag is false',
148+
outputs: ALL_FALSE,
149+
results: { test: 'failure' },
150+
reported: ['test: failure'],
151+
},
152+
{
153+
name: 'a failed ungated job',
154+
outputs: ALL_FALSE,
155+
results: { lint: 'failure' },
156+
reported: ['lint: failure'],
157+
},
158+
{
159+
name: 'a cancelled job whose flag is false',
160+
outputs: ALL_FALSE,
161+
results: { e2e: 'cancelled' },
162+
reported: ['e2e: cancelled'],
163+
},
164+
{
165+
name: 'a skipped lint, which is never gated',
166+
outputs: ALL_FALSE,
167+
results: { ...skipGated, lint: 'skipped' },
168+
reported: ['lint: skipped'],
169+
},
170+
])('fails on $name', ({ outputs, results, reported }) => {
171+
const result = gate(JSON.stringify(needs(outputs, results)))
172+
expect(result.status, result.stdout).toBe(1)
173+
expect(result.stdout).toContain('::error::Checks did not pass:')
174+
expect(result.reported).toEqual(reported)
175+
})
176+
177+
it.each(['', 'not json', '[]', 'null'])('fails on a needs context of %j', (context) => {
178+
const { status, stdout } = gate(context)
179+
expect(status).toBe(1)
180+
expect(stdout).toContain('::error::The needs context is not a JSON object')
181+
})
182+
})

0 commit comments

Comments
 (0)