check-ci-graph: a defaults: run: shell: no longer disqualifies a step (the user's ruling)
Main's PR #287 added a workflow-level defaults shell, and the refusal made every suite read as unrun. The two cases that pinned it are deleted; one shows a step under workflow and job defaults is counted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
+11
-2
@@ -288,8 +288,6 @@ The gate reads no shell. It counts a step only when all of these hold:
|
||||
(improvise-1be2);
|
||||
- the call names its own job;
|
||||
- neither the step nor the job carries `if:`, `continue-on-error:` or `shell:`;
|
||||
- no `defaults:` block, at workflow or job level, covers it, since that can
|
||||
change the shell the step runs under;
|
||||
- no job it `needs:`, directly or through other jobs, carries a job-level
|
||||
`if:`. GitHub skips a job whose need was skipped, so a false `if:` two jobs
|
||||
away skips the call as surely as one on its own job (improvise-bbwa).
|
||||
@@ -472,6 +470,17 @@ wider than the exact header `ci_steps` accepts. Reading too wide only adds jobs
|
||||
`pages-deploy` must need, which fails closed. Reading too narrow is what lost
|
||||
`gate`.
|
||||
|
||||
**A `defaults: run: shell:` does not disqualify a step** (the user's ruling,
|
||||
PR #228). An earlier version refused to count any step under a workflow- or
|
||||
job-level `defaults:`, because a default shell can change what the call runs
|
||||
under. The user called that "a silly check". It also broke on the day
|
||||
`ci.yml` gained a real one: PR #287 set a workflow-level
|
||||
`shell: nix develop .#ci --command bash …`, and every suite then read as run by
|
||||
no CI step. A default shell is how the workflow chooses its environment, not a
|
||||
way to skip a call. A shell that failed to run the call would fail the step, red
|
||||
in CI. A step's own `shell:` still disqualifies it. That rule was not part of
|
||||
the ruling.
|
||||
|
||||
**What this does not close, stated rather than guarded.** Once a counted step
|
||||
is reached, the gate trusts that it runs the script it names. An earlier step
|
||||
in the same job that rewrote `scripts/checkers.sh` or its table would defeat
|
||||
|
||||
@@ -1352,3 +1352,25 @@ Five are new:
|
||||
|
||||
The owned-content rival also fails the comment case because `owner` still
|
||||
holds the preceding `runs-on:` key's column when that line is checked.
|
||||
|
||||
### A `defaults: run: shell:` no longer disqualifies a step — `scripts/check-ci-graph.py` (the user's ruling on #228)
|
||||
|
||||
**What changed, and why.** The gate refused to count any step under a
|
||||
workflow- or job-level `defaults:` block. The user ruled it "a silly check".
|
||||
It also broke on contact with main: PR #287 added a workflow-level
|
||||
`defaults: run: shell: nix develop .#ci --command bash …`, and on the merged
|
||||
tree every suite read as run by no CI step. The refusal is removed.
|
||||
|
||||
**Cases.** Two cases pinned the refusal: "a job's defaults: can change the
|
||||
shell, so it does not count" and "workflow-level defaults: disqualifies every
|
||||
step". Both are deleted. The new case "a step under a defaults: run: shell: is
|
||||
counted" carries both a workflow-level default in #287's shape and a
|
||||
job-level one, and expects no complaint.
|
||||
|
||||
**Red first.** Against the code before the removal, the new case drew both
|
||||
steps as "conditional (workflow defaults)" and kb's as "(job defaults, workflow
|
||||
defaults)", so both rows went unrun.
|
||||
|
||||
**Kept.** A step's own `shell:` still disqualifies it; the ruling did not cover
|
||||
that. The w0j4 case refusing a quoted `"defaults":` key is about key spelling,
|
||||
and it stands.
|
||||
|
||||
@@ -78,7 +78,6 @@ _STEP_KEY = re.compile(r"^ (?:- | )([A-Za-z_][\w-]*):(.*)$")
|
||||
_ANY_KEY = re.compile(r"^(\s*)(?:- )?([A-Za-z_][\w-]*):(.*)$")
|
||||
_CI_CALL = re.compile(r"^(?:nix shell \S+ --command )?\./scripts/checkers\.sh --ci ([\w-]+)$")
|
||||
_DISQUALIFY = ("if", "continue-on-error", "shell")
|
||||
_JOB_DISQUALIFY = _DISQUALIFY + ("defaults",)
|
||||
|
||||
|
||||
_NAME = r"[A-Za-z_][\w-]*"
|
||||
@@ -154,7 +153,6 @@ def ci_steps(text):
|
||||
steps, stray, unread = [], [], []
|
||||
job, in_steps, job_flags, step = None, False, set(), None
|
||||
in_jobs, job_keyed = False, False
|
||||
workflow_flags = set()
|
||||
owner, owner_step = None, None
|
||||
needs, if_jobs, in_needs = {}, set(), False
|
||||
|
||||
@@ -187,8 +185,6 @@ def ci_steps(text):
|
||||
if _COMMENT.search(raw) or not shaped or not readable:
|
||||
unread.append((n, "comment" if _COMMENT.search(raw) else
|
||||
"plain" if not readable else "value"))
|
||||
if raw.startswith("defaults:"):
|
||||
workflow_flags.add("workflow defaults")
|
||||
m = _JOB.match(raw)
|
||||
if m:
|
||||
close_step()
|
||||
@@ -211,7 +207,7 @@ def ci_steps(text):
|
||||
needs[job] = {x.strip() for x in value.strip("[]").split(",") if x.strip()}
|
||||
if key == "if":
|
||||
if_jobs.add(job)
|
||||
if key in _JOB_DISQUALIFY:
|
||||
if key in _DISQUALIFY:
|
||||
job_flags.add(key)
|
||||
for s in steps:
|
||||
if s["job"] == job:
|
||||
@@ -238,7 +234,6 @@ def ci_steps(text):
|
||||
owner_step = step if m and m.group(1) == "run" else None
|
||||
close_step()
|
||||
for s in steps:
|
||||
s["flags"] |= workflow_flags
|
||||
s["flags"] |= {f"needs {j}, which has if" for j in _needed(s["job"], needs) & if_jobs}
|
||||
return steps, stray, unread
|
||||
|
||||
@@ -544,16 +539,10 @@ SUITE_SELF_TEST = [
|
||||
["job `kb`'s `checkers.sh --ci` step is conditional (job if), so it does not count"]
|
||||
+ _NO_PW),
|
||||
|
||||
("a job's defaults: can change the shell, so it does not count", _REQ,
|
||||
_wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci kb\n",
|
||||
head=" defaults:\n run:\n shell: python {0}\n")),
|
||||
["job `kb`'s `checkers.sh --ci` step is conditional (job defaults), "
|
||||
"so it does not count"] + _NO_PW),
|
||||
|
||||
("workflow-level defaults: disqualifies every step", ["assemble-stamp"],
|
||||
"defaults:\n run:\n shell: python {0}\n" + _wf(_BUILD),
|
||||
["job `build`'s `checkers.sh --ci` step is conditional (workflow defaults), "
|
||||
"so it does not count", _unrun("assemble-stamp")]),
|
||||
("a step under a defaults: run: shell: is counted (the user's ruling)", _REQ,
|
||||
"defaults:\n run:\n shell: nix develop .#ci --command bash -e {0}\n"
|
||||
+ _wf(_BUILD, _job("kb", _KB_RUN, head=" defaults:\n run:\n shell: bash\n")),
|
||||
[]),
|
||||
|
||||
("continue-on-error does not count", _REQ,
|
||||
_wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci kb\n"
|
||||
|
||||
Reference in New Issue
Block a user