From 4fa03f7e2b9923a64eaa905a844bcb2c91c4840d Mon Sep 17 00:00:00 2001 From: ScottW514 Date: Sun, 18 Oct 2026 13:44:59 -0400 Subject: [PATCH] forgetest: the lint counts a push or pull request only when no filter is under it A HOST_TESTED row's workflow has to run on push or pull_request, and the lint read the events of its `on:` and nothing under them. A `paths:` or `paths-ignore:` under the event keeps the workflow from the pushes that change the other files, a `branches:` or `branches-ignore:` from the pushes to the other branches, and a push with only a `tags:` or `tags-ignore:` runs on no branch push at all. So a workflow whose push and pull_request each carried such a filter counted as running on every change, and a host test behind one kept its row green and its paths covered. workflow_filters reads what is under each event of the `on:` block mapping: each `branches:`, `branches-ignore:`, `tags:`, `tags-ignore:`, `paths:` or `paths-ignore:` key at the event's own level, by name; a value on the event's line (`push: {branches: [main]}`); and a key with one stray quote under the event. Each is a filter the lint does not read: the simple rule, fail closed. The manifest carries no branch (a component's record is its srcrev, source, files and recipes), so a `branches:` naming the default branch is read as any other filter, and a `paths:` is not matched against the row's covered paths. host_row_problems counts a push or a pull_request only with no filter under it, and lists a workflow whose every push and pull_request carries one as "every push or pull_request in its own on: carries a filter (push: paths-ignore)", with each event's filters named. One push or pull_request with no filter is enough: the workflow then runs on every change, through that event. "Runs" for a row's workflow now means: on a push or a pull_request with no filter under it, in a step on every run of the workflow, in a job whose needs are met on every run, on a line of run text whose failure ends the step. The HOST_TESTED comment and the docstrings say so. A key deeper than the event's own level (a dispatch's inputs) is no filter, and an `on:` list or scalar carries none. The workflows the rows name, forgectrl's and forgeext's build.yml, run on `push: branches: [main]` with a plain `pull_request:`, as grblhal-glowforge's does: the pull request keeps each row counting, and the push to main is the push that moves a pin. Proof: forgetest-ci "Unit tests" (`python -B -m unittest discover -s tests -v` in forgetest): test_manifest.py runs 41 tests, OK, on Ubuntu 24.04 with python 3.12, in a private PID namespace. The new case test_a_push_or_pull_request_under_a_filter_runs_on_not_every_change: workflow_filters reads a `branches:` on the push with a plain pull_request beside it; a `branches-ignore:` and a `paths-ignore:` on the push with a `paths:` on the pull_request; `tags:`; `tags-ignore:`; a value on the event's line and a stray-quoted key under the event as filters the lint does not read; a dispatch's `inputs: paths:` as none; and an `on:` flow list, scalar and block list as none. Through host_row_problems, a push to main with every pull request passes, a push with `paths-ignore:` alone is listed, and a push under `branches:` with a pull_request under `paths:` is listed, each with its filters named. The CI_YML fixture of the two older cases moves its `paths:` from the push to the pull_request, so the workflow still runs on every push and still names the second harness in a filter alone. Controls: the parent, 9c97f05, with the new test file errors the new case at workflow_filters (errors=1); host_row_problems on the parent, and on e1d1c1b, passes a fixture whose push carries `paths-ignore:` alone, which the change lists with "push: paths-ignore". The coverage lint on the tree manifest from the recipe pins, with meta-openglow at 1e163bd on scarthgap, reports 0 uncovered paths, 0 empty entries, 0 host-tested row problems, 43 tests. pyflakes reports no undefined name. Acceptance: none. forgetest is dev-only and in no fingerprint; the lint runs in CI, not in a campaign, and tests/ are in no fingerprint. --- forgetest/forgetest/manifest.py | 103 +++++++++++++++++++++++++------ forgetest/tests/test_manifest.py | 41 +++++++++++- 2 files changed, 123 insertions(+), 21 deletions(-) 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