diff --git a/forgetest/forgetest/manifest.py b/forgetest/forgetest/manifest.py index 807c41f..76de921 100644 --- a/forgetest/forgetest/manifest.py +++ b/forgetest/forgetest/manifest.py @@ -118,20 +118,21 @@ BEHAVIORAL = [ # the manifest (its tests/ and .github/ files are listed there), or # "forgefirm" for this repository (the null-sink harnesses under # 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 -# 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 -# job, every job the job's `needs:` names one that runs on every run, and -# no key with one stray quote where the lint reads the workflow's keys), -# under bash or sh, on a line of the step's run text whose failure -# ends the step (the command alone on its line, or in a pipeline under -# pipefail, with errexit set and no line before it that ends the step or -# opens a compound command: workflow_runs), and name it, or discover it (a -# glob, unittest discover, pytest on 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 change there makes no acceptance run -# necessary. A path a catalog test exercises on the hardware belongs in -# that test's covers, not here. +# must run the host test: on a push or a pull_request with no branches, +# tags or paths filter under it (one of the two is enough), in a step +# that runs on 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 job, every job the job's `needs:` names one that +# runs on every run, and no key with one stray quote where the lint +# reads the workflow's keys), under bash or sh, on a line of the step's +# run text whose failure ends the step (the command alone on its line, +# or in a pipeline under pipefail, with errexit set and no line before +# it that ends the step or opens a compound command: workflow_runs), and +# name it, or discover it (a glob, unittest discover, pytest on 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 change +# there makes no acceptance run necessary. A path a catalog test +# exercises on the hardware belongs in that test's covers, not here. HOST_TESTED = [ # An extension package's holds as the engine reads them (forgeext's # holdkeep_test and run_test prove the writer's side). @@ -470,10 +471,10 @@ def workflow_triggers(text): """The events a workflow runs on: the keys of its `on:` mapping, the 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 paths, a dispatch's inputs) is not an event. A flow mapping - (`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.""" + its paths, a dispatch's inputs) is not an event (workflow_filters + reads the filters). A flow mapping (`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) for i, ln in enumerate(lines): m = _ON_RX.match(ln) if not _indent(ln) else None @@ -507,6 +508,59 @@ def workflow_triggers(text): return set() +# What under a push or a pull_request of `on:` keeps the workflow from a +# run: a ref or a path the event does not match. +_FILTER_KEYS = ("branches", "branches-ignore", "tags", "tags-ignore", "paths", "paths-ignore") + + +def workflow_filters(text): + """The events of a workflow's `on:` block mapping that carry a filter, + {event: [what]}: each `branches:`, `branches-ignore:`, `tags:`, + `tags-ignore:`, `paths:` or `paths-ignore:` key under the event, by + name; a value on the event's own line (`push: {branches: [main]}`), + which the lint does not read, as "a value the lint does not read"; + and a key with one stray quote under the event (`paths":`), as "a + key the lint does not read". Every filter counts: a push with only + a branches or a tags filter runs on no push of the other kind of + ref, and a branch it names is one the lint does not know, as the + manifest carries no branch. A key deeper than the event's own level + (a dispatch's inputs) is no filter. An event of a list or a scalar + `on:` carries none.""" + lines = _lines(text) + out = {} + for i, ln in enumerate(lines): + m = _ON_RX.match(ln) if not _indent(ln) else None + if not m: + continue + if m.group(1).strip(): + return out + at, event, sub = None, None, None + for child in lines[i + 1:]: + if not child.strip(): + continue + ind = _indent(child) + if ind == 0: + break + at = ind if at is None else at + km = _KEY_RX.match(child) + if ind == at: + event, sub = None, None + if km: + event = km.group(2) + if km.group(3).strip(): + out.setdefault(event, []).append("a value the lint does not read") + elif event is not None and ind > at: + sub = ind if sub is None else sub + if ind != sub: + continue + if _stray_key(child): + out.setdefault(event, []).append("a key the lint does not read") + elif km and km.group(2) in _FILTER_KEYS: + out.setdefault(event, []).append(km.group(2)) + return out + return out + + def _needs(lines, i): """The job names the `needs:` key at lines[i] names: a flow list (`[build, lint]`), one scalar, or a block list on the lines under @@ -1088,6 +1142,11 @@ def host_row_problems(manifest, host=HOST_TESTED, root=REPO_ROOT, sources=SOURCE push nor pull_request as the lint reads it (workflow_triggers: a dispatch, a call or a schedule alone, a call not followed to the workflow that calls it, and a flow mapping, which names no event), + one whose every push and pull_request carries a filter + (workflow_filters: a branches, tags or paths filter, which the lint + does not read, so the workflow runs on not every change of the + covered files, or on not every branch; one push or pull_request + with no filter makes the workflow run on every change), one that does not check out the test's repository, or one with no step that runs the test on every run (workflow_runs). Whether a workflow runs its test is not checked where it cannot be read (see @@ -1107,9 +1166,15 @@ def host_row_problems(manifest, host=HOST_TESTED, root=REPO_ROOT, sources=SOURCE continue if text is None: continue - if not workflow_triggers(text) & {"push", "pull_request"}: + events = workflow_triggers(text) & {"push", "pull_request"} + if not events: out.append((comp, pat, ref, "%s: the lint reads no push or pull_request in its own on:" % ci)) continue + filters = workflow_filters(text) + if all(e in filters for e in events): + out.append((comp, pat, ref, "%s: every push or pull_request in its own on: carries a filter (%s)" % ( + ci, "; ".join("%s: %s" % (e, ", ".join(filters[e])) for e in sorted(events))))) + continue steps = workflow_steps(text) test_repo, _sep, test_path = ref.partition(":") prefix = checkout_path(steps, repo_name(manifest, test_repo), test_repo == ci.partition(":")[0]) diff --git a/forgetest/tests/test_manifest.py b/forgetest/tests/test_manifest.py index 609f5f7..911e2df 100644 --- a/forgetest/tests/test_manifest.py +++ b/forgetest/tests/test_manifest.py @@ -197,9 +197,10 @@ class CoverageReportTests(unittest.TestCase): f.write(text) return root - # A workflow of this repository as the scratch checkouts carry it: it + # A workflow of this repository as the scratch checkouts carry it: on + # every push, and on the pull requests that touch one bench tool, it # checks itself out at the workspace root and runs the one harness. - CI_YML = ("on:\n push:\n paths:\n - 'scripts/bench/drill.py'\n" + CI_YML = ("on:\n push:\n pull_request:\n paths:\n - 'scripts/bench/drill.py'\n" "jobs:\n test:\n steps:\n" " - uses: actions/checkout@v4\n" " # drill.py is a bench tool\n" @@ -449,6 +450,42 @@ class CoverageReportTests(unittest.TestCase): 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_a_push_or_pull_request_under_a_filter_runs_on_not_every_change(self): + f = m.workflow_filters + self.assertEqual(f("on:\n push:\n branches: [main]\n pull_request:\njobs:\n t:\n"), {"push": ["branches"]}) + self.assertEqual(f("on:\n push:\n branches-ignore:\n - wip/*\n paths-ignore:\n - 'docs/**'\n" + " pull_request:\n paths:\n - 'src/**'\n"), + {"push": ["branches-ignore", "paths-ignore"], "pull_request": ["paths"]}) + self.assertEqual(f("on:\n push:\n tags: ['v*']\n"), {"push": ["tags"]}) + self.assertEqual(f("on:\n push:\n tags-ignore: ['v*']\n"), {"push": ["tags-ignore"]}) + # a value on the event's line, and a key with one stray quote under it, are filters the + # lint does not read + self.assertEqual(f("on:\n push: {branches: [main]}\n"), {"push": ["a value the lint does not read"]}) + self.assertEqual(f("on:\n push:\n paths\":\n - x\n"), {"push": ["a key the lint does not read"]}) + # a filter is read at the event's own level: a dispatch's inputs are none, and an on: list + # or scalar carries none + self.assertEqual(f("on:\n workflow_dispatch:\n inputs:\n paths:\n type: string\n push:\n"), + {}) + self.assertEqual(f("on: [push, pull_request]\n"), {}) + self.assertEqual(f("on: push\n"), {}) + self.assertEqual(f("on:\n - push\n"), {}) + # a row's workflow counts when its push or its pull_request carries no filter: a push to + # the default branch alone with every pull request does, a push with paths-ignore alone + # does not, and nor do a push and a pull request each under a filter + man = helpers.make_manifest() + job = "jobs:\n test:\n steps:\n - uses: actions/checkout@v4\n - run: python3 scripts/bench/stream_test.py\n" + root = self._root({"scripts/bench/stream_test.py": "", + ".github/workflows/main.yml": "on:\n push:\n branches: [main]\n pull_request:\n" + job, + ".github/workflows/docs.yml": "on:\n push:\n paths-ignore:\n - 'docs/**'\n" + job, + ".github/workflows/both.yml": "on:\n push:\n branches: [main]\n pull_request:\n" + " paths:\n - 'src/**'\n" + job}) + rows = [("grblhal-glowforge", "src/driver.c", "forgefirm:scripts/bench/stream_test.py", + "forgefirm:.github/workflows/%s.yml" % w) for w in ("main", "docs", "both")] + filtered = ": every push or pull_request in its own on: carries a filter (%s)" + self.assertEqual(m.host_row_problems(man, rows, root=root, sources=None), + [rows[1][:3] + (rows[1][3] + filtered % "push: paths-ignore",), + rows[2][:3] + (rows[2][3] + filtered % "pull_request: paths; push: branches",)]) + def test_the_lint_says_when_it_could_not_read_a_workflow(self): import contextlib import io