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

@@ -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
+# must run the host test: on a push or a pull_request with no branches,
-# every run of the workflow and whose failure fails it (no `if:` other than
+# tags or paths filter under it (one of the two is enough), in a step
-# `true` and no `continue-on-error:` other than `false` on the step or its
+# that runs on every run of the workflow and whose failure fails it (no
-# job, every job the job's `needs:` names one that runs on every run, and
+# `if:` other than `true` and no `continue-on-error:` other than `false`
-# no key with one stray quote where the lint reads the workflow's keys),
+# on the step or its job, every job the job's `needs:` names one that
-# under bash or sh, on a line of the step's run text whose failure
+# runs on every run, and no key with one stray quote where the lint
-# ends the step (the command alone on its line, or in a pipeline under
+# reads the workflow's keys), under bash or sh, on a line of the step's
-# pipefail, with errexit set and no line before it that ends the step or
+# run text whose failure ends the step (the command alone on its line,
-# opens a compound command: workflow_runs), and name it, or discover it (a
+# or in a pipeline under pipefail, with errexit set and no line before
-# glob, unittest discover, pytest on its directory). A host-tested path
+# it that ends the step or opens a compound command: workflow_runs), and
-# counts as covered in the coverage lint, and like every path no test
+# name it, or discover it (a glob, unittest discover, pytest on its
-# covers it is in no fingerprint: a change there makes no acceptance run
+# directory). A host-tested path counts as covered in the coverage lint,
-# necessary. A path a catalog test exercises on the hardware belongs in
+# and like every path no test covers it is in no fingerprint: a change
-# that test's covers, not here.
+# 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
+    its paths, a dispatch's inputs) is not an event (workflow_filters
-    (`on: {push: ...}`) is not read, and names no event. An event key
+    reads the filters). A flow mapping (`on: {push: ...}`) is not read,
-    with one stray quote (`push":`) is no event, and the workflow then
+    and names no event. An event key with one stray quote (`push":`) is
-    reads as one that runs on none."""
+    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])

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