mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-10-18 18:41:41 -07:00
forgetest: the lint reads a quoted key only in a matched pair, and a key with one stray quote runs nothing
_KEY_RX took an opening quote and a closing quote each on its own, so a key with one stray quote read as the bare key: `push":` and `pull_request':` counted as push and pull_request, where YAML reads the keys `push"` and `pull_request'`, which are no events; a step's `run": ...` counted as its run command, and `uses": actions/checkout@v4` as a checkout. A HOST_TESTED row whose workflow was spelled so passed the lint, while that workflow runs on no push and runs no such step. _KEY_RX now reads a quote only as a matched pair. A key with one stray quote is no key the lint reads, and where the lint reads keys it counts as running nothing: an event key spelled so makes the `on:` name no event, and one at the top level, at a job's name or key, or at a step's key below its dash line makes every step of the workflow count as no step (each carries "unread", which _on_every_run refuses). So a step's `if": false` still keeps the step from counting, as it did under the loose reading, and under a `jobs":` top level, where a job's `if: false` goes unread, no step counts either. The HOST_TESTED comment says so. The lint only lists more: a workflow with no such key reads as before. Proof: forgetest-ci "Unit tests" (`python -B -m unittest discover -s tests -v` in forgetest): test_manifest.py runs 39 tests, OK, on Windows with python 3.14 and on Ubuntu 24.04 with python 3.12. test_the_events_a_workflow_runs_on gains an `on:` with `push":`, and one with `push:` and `'pull_request:`, each naming no event, and test_a_step_counts_only_when_every_run_of_the_workflow_runs_it_and_its_failure_fails_the_run gains a step with `if": false`, a step with `run": ./build/tray_test`, a job with `if": false` and a `jobs":` top level, none of them running the test. Negative controls on scratch copies: _KEY_RX as it was fails the `run":` step and the `push":` event; the matched pairs with no stray-quote rule fail the `if": false` step and the second `on:`; the rule at a step's keys alone fails the job's `if": false`. The 17 workflows of the tree's repositories (211 steps) read to the same events and steps before and after the change. The coverage lint on the tree manifest from the recipe pins reports 0 uncovered paths, 0 host-tested row problems, 43 tests. pyflakes reports no undefined name. Acceptance: none. forgetest is dev-only and in no fingerprint (the source_sha of 43 of 43 tests is unchanged); the lint runs in CI, not in a campaign, and tests/ are in no fingerprint.
This commit is contained in:
@@ -121,7 +121,8 @@ BEHAVIORAL = [
|
|||||||
# must run the host test: on push or pull request, in a step that runs on
|
# must run the host test: on push or pull request, in a step that runs on
|
||||||
# every run of the workflow and whose failure fails it (no `if:` other than
|
# every run of the workflow and whose failure fails it (no `if:` other than
|
||||||
# `true` and no `continue-on-error:` other than `false` on the step or its
|
# `true` and no `continue-on-error:` other than `false` on the step or its
|
||||||
# job), and name it, or discover it (a glob, unittest discover, pytest on
|
# job, and no key with one stray quote where the lint reads the workflow's
|
||||||
|
# keys), and name it, or discover it (a glob, unittest discover, pytest on
|
||||||
# its directory). A host-tested path counts as covered in the coverage
|
# its directory). A host-tested path counts as covered in the coverage
|
||||||
# lint, and like every path no test covers it is in no fingerprint: a
|
# lint, and like every path no test covers it is in no fingerprint: a
|
||||||
# change there makes no acceptance run necessary. A path a catalog test
|
# change there makes no acceptance run necessary. A path a catalog test
|
||||||
@@ -418,8 +419,13 @@ def repo_name(manifest, repo):
|
|||||||
_STEP_KEYS = ("name", "uses", "run", "id", "if", "with", "env", "working-directory", "shell",
|
_STEP_KEYS = ("name", "uses", "run", "id", "if", "with", "env", "working-directory", "shell",
|
||||||
"continue-on-error", "timeout-minutes")
|
"continue-on-error", "timeout-minutes")
|
||||||
_STEP_RX = re.compile(r"^(\s*)-\s+(%s)\s*:" % "|".join(re.escape(k) for k in _STEP_KEYS))
|
_STEP_RX = re.compile(r"^(\s*)-\s+(%s)\s*:" % "|".join(re.escape(k) for k in _STEP_KEYS))
|
||||||
# A mapping key, plain or quoted ("if": is the same key as if:).
|
# A mapping key, plain or in a matched pair of quotes ("if": is the same
|
||||||
_KEY_RX = re.compile(r"""^(\s*)["']?([\w-]+)["']?\s*:\s*(.*)$""")
|
# key as if:).
|
||||||
|
_KEY_RX = re.compile(r"""^(\s*)(?=(?:"[\w-]+"|'[\w-]+'|[\w-]+)\s*:)["']?([\w-]+)["']?\s*:\s*(.*)$""")
|
||||||
|
# A key with one stray quote (if": or "if:), which _KEY_RX does not read:
|
||||||
|
# YAML reads a key with the quote in its name, which no workflow key and
|
||||||
|
# no event has, or refuses the file.
|
||||||
|
_STRAY_RX = re.compile(r"""^\s*["']?[\w-]+["']?\s*:""")
|
||||||
_ON_RX = re.compile(r"""^(?:on|"on"|'on')\s*:\s*(.*)$""")
|
_ON_RX = re.compile(r"""^(?:on|"on"|'on')\s*:\s*(.*)$""")
|
||||||
# What a job or a step says about whether it runs on every run of the
|
# What a job or a step says about whether it runs on every run of the
|
||||||
# workflow, and whether its failure fails the run.
|
# workflow, and whether its failure fails the run.
|
||||||
@@ -430,6 +436,11 @@ def _indent(line):
|
|||||||
return len(line) - len(line.lstrip(" "))
|
return len(line) - len(line.lstrip(" "))
|
||||||
|
|
||||||
|
|
||||||
|
def _stray_key(line):
|
||||||
|
"""Whether a line is a mapping key with one stray quote."""
|
||||||
|
return _KEY_RX.match(line) is None and _STRAY_RX.match(line) is not None
|
||||||
|
|
||||||
|
|
||||||
def _lines(text):
|
def _lines(text):
|
||||||
"""A workflow's lines with the comments dropped: whole comment lines,
|
"""A workflow's lines with the comments dropped: whole comment lines,
|
||||||
and from ' #' to the end of a line."""
|
and from ' #' to the end of a line."""
|
||||||
@@ -455,7 +466,9 @@ def workflow_triggers(text):
|
|||||||
items of its list (`on: [push, pull_request]`, or one per line), or
|
items of its list (`on: [push, pull_request]`, or one per line), or
|
||||||
its one scalar (`on: push`). What is under an event (its branches,
|
its one scalar (`on: push`). What is under an event (its branches,
|
||||||
its paths, a dispatch's inputs) is not an event. A flow mapping
|
its paths, a dispatch's inputs) is not an event. A flow mapping
|
||||||
(`on: {push: ...}`) is not read, and names no event."""
|
(`on: {push: ...}`) is not read, and names no event. An event key
|
||||||
|
with one stray quote (`push":`) is no event, and the workflow then
|
||||||
|
reads as one that runs on none."""
|
||||||
lines = _lines(text)
|
lines = _lines(text)
|
||||||
for i, ln in enumerate(lines):
|
for i, ln in enumerate(lines):
|
||||||
m = _ON_RX.match(ln) if not _indent(ln) else None
|
m = _ON_RX.match(ln) if not _indent(ln) else None
|
||||||
@@ -479,6 +492,8 @@ def workflow_triggers(text):
|
|||||||
body = child.strip()
|
body = child.strip()
|
||||||
if body.startswith("- "):
|
if body.startswith("- "):
|
||||||
out.add(_scalar(body[2:]))
|
out.add(_scalar(body[2:]))
|
||||||
|
elif _stray_key(child):
|
||||||
|
return set()
|
||||||
else:
|
else:
|
||||||
km = _KEY_RX.match(child)
|
km = _KEY_RX.match(child)
|
||||||
if km:
|
if km:
|
||||||
@@ -495,10 +510,14 @@ def workflow_steps(text):
|
|||||||
in one. YAML read by its indentation, as far as these workflows use
|
in one. YAML read by its indentation, as far as these workflows use
|
||||||
it; comments, whole lines and from ' #' to the end of a line, are not
|
it; comments, whole lines and from ' #' to the end of a line, are not
|
||||||
read. Anything outside a step (a trigger's paths, a job's name) is not
|
read. Anything outside a step (a trigger's paths, a job's name) is not
|
||||||
a step."""
|
a step. A key with one stray quote (`if":`, `"run:`) at the top level,
|
||||||
|
at a job's name or a job's key, or at a step's key is no key the lint
|
||||||
|
reads: every step of the workflow then carries "unread" and counts as
|
||||||
|
no step."""
|
||||||
lines = _lines(text)
|
lines = _lines(text)
|
||||||
steps, i = [], 0
|
steps, i = [], 0
|
||||||
in_jobs, job_at, key_at, job = False, None, None, None
|
in_jobs, job_at, key_at, job = False, None, None, None
|
||||||
|
stray = False
|
||||||
while i < len(lines):
|
while i < len(lines):
|
||||||
km = _KEY_RX.match(lines[i])
|
km = _KEY_RX.match(lines[i])
|
||||||
if km: # a mapping key: which job this is, and what the job itself says
|
if km: # a mapping key: which job this is, and what the job itself says
|
||||||
@@ -511,6 +530,10 @@ def workflow_steps(text):
|
|||||||
key_at = ind
|
key_at = ind
|
||||||
if key in _CONDITION_KEYS:
|
if key in _CONDITION_KEYS:
|
||||||
job[key] = _scalar(km.group(3))
|
job[key] = _scalar(km.group(3))
|
||||||
|
elif _stray_key(lines[i]):
|
||||||
|
ind = _indent(lines[i])
|
||||||
|
stray = stray or ind == 0 or (in_jobs and (job_at is None or ind == job_at or (
|
||||||
|
ind > job_at and (key_at is None or ind == key_at))))
|
||||||
m = _STEP_RX.match(lines[i])
|
m = _STEP_RX.match(lines[i])
|
||||||
if not m:
|
if not m:
|
||||||
i += 1
|
i += 1
|
||||||
@@ -528,6 +551,7 @@ def workflow_steps(text):
|
|||||||
while k < len(body):
|
while k < len(body):
|
||||||
km = _KEY_RX.match(body[k])
|
km = _KEY_RX.match(body[k])
|
||||||
if not km or _indent(body[k]) != key_col:
|
if not km or _indent(body[k]) != key_col:
|
||||||
|
stray = stray or (_indent(body[k]) == key_col and _stray_key(body[k]))
|
||||||
k += 1
|
k += 1
|
||||||
continue
|
continue
|
||||||
key, val = km.group(2), _scalar(km.group(3))
|
key, val = km.group(2), _scalar(km.group(3))
|
||||||
@@ -546,6 +570,9 @@ def workflow_steps(text):
|
|||||||
if job is not None:
|
if job is not None:
|
||||||
step["job"] = job # the job's own dict: a job key written after its steps counts too
|
step["job"] = job # the job's own dict: a job key written after its steps counts too
|
||||||
steps.append(step)
|
steps.append(step)
|
||||||
|
if stray:
|
||||||
|
for s in steps:
|
||||||
|
s["unread"] = True
|
||||||
return steps
|
return steps
|
||||||
|
|
||||||
|
|
||||||
@@ -554,7 +581,10 @@ def _on_every_run(step):
|
|||||||
fails the run: neither it nor its job carries an `if:` other than
|
fails the run: neither it nor its job carries an `if:` other than
|
||||||
`true`, or a `continue-on-error:` other than `false`. A step that
|
`true`, or a `continue-on-error:` other than `false`. A step that
|
||||||
runs only under a condition, or whose failure the run tolerates,
|
runs only under a condition, or whose failure the run tolerates,
|
||||||
proves nothing about what it runs."""
|
proves nothing about what it runs, and neither does a step of a
|
||||||
|
workflow with a key the lint cannot read ("unread")."""
|
||||||
|
if step.get("unread"):
|
||||||
|
return False
|
||||||
for d in (step, step.get("job") or {}):
|
for d in (step, step.get("job") or {}):
|
||||||
if d.get("if", "true").lower() != "true" or d.get("continue-on-error", "false").lower() != "false":
|
if d.get("if", "true").lower() != "true" or d.get("continue-on-error", "false").lower() != "false":
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -329,6 +329,14 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
# however the step is spelled: more than one space after its dash, or a quoted key
|
# however the step is spelled: more than one space after its dash, or a quoted key
|
||||||
self.assertFalse(self._runs("steps:\n - run: ./build/tray_test\n if: false\n", test))
|
self.assertFalse(self._runs("steps:\n - run: ./build/tray_test\n if: false\n", test))
|
||||||
self.assertFalse(self._runs(step % " \"if\": false\n", test))
|
self.assertFalse(self._runs(step % " \"if\": false\n", test))
|
||||||
|
# a key with one stray quote is no key the lint reads: at a step's key, a job's key or
|
||||||
|
# the top level, no step of the workflow counts
|
||||||
|
self.assertFalse(self._runs(step % " if\": false\n", test))
|
||||||
|
self.assertFalse(self._runs("steps:\n - name: x\n run\": ./build/tray_test\n", test))
|
||||||
|
self.assertFalse(self._runs("jobs:\n t:\n if\": false\n steps:\n - run: ./build/tray_test\n",
|
||||||
|
test))
|
||||||
|
self.assertFalse(self._runs("jobs\":\n t:\n if: false\n steps:\n - run: ./build/tray_test\n",
|
||||||
|
test))
|
||||||
# a step whose failure the run tolerates proves nothing; literally false is no tolerance
|
# a step whose failure the run tolerates proves nothing; literally false is no tolerance
|
||||||
self.assertFalse(self._runs(step % " continue-on-error: true\n", test))
|
self.assertFalse(self._runs(step % " continue-on-error: true\n", test))
|
||||||
self.assertFalse(self._runs(step % " continue-on-error: ${{ inputs.soft }}\n", test))
|
self.assertFalse(self._runs(step % " continue-on-error: ${{ inputs.soft }}\n", test))
|
||||||
@@ -361,6 +369,9 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
" type: string\n"), {"workflow_dispatch"})
|
" type: string\n"), {"workflow_dispatch"})
|
||||||
self.assertEqual(m.workflow_triggers("on:\n workflow_call:\n"), {"workflow_call"})
|
self.assertEqual(m.workflow_triggers("on:\n workflow_call:\n"), {"workflow_call"})
|
||||||
self.assertEqual(m.workflow_triggers("name: x\njobs:\n t:\n steps: []\n"), set())
|
self.assertEqual(m.workflow_triggers("name: x\njobs:\n t:\n steps: []\n"), set())
|
||||||
|
# an event key with one stray quote is no event, and the workflow then names none
|
||||||
|
self.assertEqual(m.workflow_triggers("on:\n push\":\n"), set())
|
||||||
|
self.assertEqual(m.workflow_triggers("on:\n push:\n 'pull_request:\n"), set())
|
||||||
|
|
||||||
def test_the_lint_says_when_it_could_not_read_a_workflow(self):
|
def test_the_lint_says_when_it_could_not_read_a_workflow(self):
|
||||||
import contextlib
|
import contextlib
|
||||||
|
|||||||
Reference in New Issue
Block a user