mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-10-18 18:41:41 -07:00
forgetest: the lint's step parser keeps a step's real key column and reads a quoted key
workflow_steps rebuilt the first line of a step with its first key two columns past the dash, whatever followed the dash. A step written with more than one space after its dash (`- run: ./build/tray_test`, its `if: false` at the column of the run key) then had its first key at one column and the others at another: every key after the first was read as a child of the first and dropped, `if:`, `continue-on-error:`, `working-directory:` and `with:` among them. So such a step under `if: false` counted as running its host test, and such a checkout under a condition counted as a checkout. _KEY_RX did not read a quoted key, so a step's `"if": false` was dropped the same way. Both spellings are valid YAML that GitHub skips or tolerates. The parser now replaces the dash with a space and realigns nothing, so the key column of a step is the column its keys use. _KEY_RX reads a key in single or double quotes as the same key. The HOST_TESTED comment states the rule the lint applies: no `if:` other than `true` and no `continue-on-error:` other than `false` on the step or its job. Proof: forgetest-ci "Unit tests" (`python -B -m unittest discover -s tests -v` in forgetest): test_manifest.py runs 38 tests, OK, on Windows with python 3.14 and on Ubuntu 24.04 with python 3.12, with test_a_step_counts_only_when_every_run_of_the_workflow_runs_it_and_its_failure_fails_the_run extended by a step with three spaces after its dash under `if: false` and a step under a quoted `"if": false`, neither running the test. Negative controls on scratch copies: the first line rebuilt as before fails the three-space case; _KEY_RX as before fails the quoted case. The 17 workflows of the tree's repositories (205 steps) parse to the same steps and events 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:
@@ -119,13 +119,13 @@ BEHAVIORAL = [
|
|||||||
# "forgefirm" for this repository (the null-sink harnesses under
|
# "forgefirm" for this repository (the null-sink harnesses under
|
||||||
# scripts/bench/, which grblhal-glowforge's workflow runs). The workflow
|
# scripts/bench/, which grblhal-glowforge's workflow runs). The workflow
|
||||||
# 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:` and no
|
# every run of the workflow and whose failure fails it (no `if:` other than
|
||||||
# `continue-on-error:` on the step or its job), and name it, or discover it
|
# `true` and no `continue-on-error:` other than `false` on the step or its
|
||||||
# (a glob, unittest discover, pytest on its directory). A host-tested path
|
# job), and name it, or discover it (a glob, unittest discover, pytest on
|
||||||
# counts as covered in the coverage lint, and like every path no test
|
# its directory). A host-tested path counts as covered in the coverage
|
||||||
# covers it is in no fingerprint: a change there makes no acceptance run
|
# lint, and like every path no test covers it is in no fingerprint: a
|
||||||
# necessary. A path a catalog test exercises on the hardware belongs in
|
# change there makes no acceptance run necessary. A path a catalog test
|
||||||
# that test's covers, not here.
|
# exercises on the hardware belongs in that test's covers, not here.
|
||||||
HOST_TESTED = [
|
HOST_TESTED = [
|
||||||
# An extension package's holds as the engine reads them (forgeext's
|
# An extension package's holds as the engine reads them (forgeext's
|
||||||
# holdkeep_test and run_test prove the writer's side).
|
# holdkeep_test and run_test prove the writer's side).
|
||||||
@@ -418,7 +418,8 @@ 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))
|
||||||
_KEY_RX = re.compile(r"^(\s*)([\w-]+)\s*:\s*(.*)$")
|
# A mapping key, plain or quoted ("if": is the same key as if:).
|
||||||
|
_KEY_RX = re.compile(r"""^(\s*)["']?([\w-]+)["']?\s*:\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.
|
||||||
@@ -515,7 +516,9 @@ def workflow_steps(text):
|
|||||||
i += 1
|
i += 1
|
||||||
continue
|
continue
|
||||||
dash = len(m.group(1))
|
dash = len(m.group(1))
|
||||||
body = [" " * (dash + 2) + lines[i][dash + 1:].lstrip()]
|
# The dash becomes a space, nothing realigned: the first key stays at
|
||||||
|
# the column the step's other keys use, however many spaces follow it.
|
||||||
|
body = [" " * (dash + 1) + lines[i][dash + 1:]]
|
||||||
i += 1
|
i += 1
|
||||||
while i < len(lines) and (not lines[i].strip() or _indent(lines[i]) > dash):
|
while i < len(lines) and (not lines[i].strip() or _indent(lines[i]) > dash):
|
||||||
body.append(lines[i])
|
body.append(lines[i])
|
||||||
|
|||||||
@@ -326,6 +326,9 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
self.assertFalse(self._runs(step % " if: ${{ always() }}\n", test))
|
self.assertFalse(self._runs(step % " if: ${{ always() }}\n", test))
|
||||||
self.assertTrue(self._runs(step % " if: true\n", test))
|
self.assertTrue(self._runs(step % " if: true\n", test))
|
||||||
self.assertTrue(self._runs(step % " if: 'true'\n", test))
|
self.assertTrue(self._runs(step % " if: 'true'\n", test))
|
||||||
|
# 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(step % " \"if\": false\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))
|
||||||
|
|||||||
Reference in New Issue
Block a user