From 0de9051820f4808a708c3343a5a8877024707673 Mon Sep 17 00:00:00 2001 From: ScottW514 Date: Sun, 18 Oct 2026 14:34:10 -0400 Subject: [PATCH] forgetest: a pull_request narrowed by types: is a filter too The coverage lint counts a HOST_TESTED row's workflow on a push or a pull_request with no filter under it, and it read a `types:` under the pull_request as nothing. A pull_request under `types:` runs on the activity types it names and no other, so a workflow on `pull_request: types: [closed]` never runs on a pull request's open or its new commits, and a row on it counted as running on every change, with the host test behind it keeping its paths covered. No workflow a row names writes a `types:`. workflow_filters reads a `types:` key at the pull_request's own level as a filter, by name, as it reads a `paths:`: the lint reads no type, so any `types:` is one, fail closed. A `types:` is read under the pull_request alone: a push has none, and the lint counts no other event. host_row_problems lists a workflow whose pull_request carries one and whose push carries a filter, or is absent, as "every push or pull_request in its own on: carries a filter (pull_request: types)"; a push with no filter beside it keeps the workflow counting. The HOST_TESTED comment and the docstrings name the types filter. 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. test_a_push_or_pull_request_under_a_filter_runs_on_not_every_change gains: workflow_filters reads a `types:` under the pull_request with a plain push beside it as the pull_request's filter alone; a `types:` block list with a `paths-ignore:` after it as both; and a `types:` under a push as none. Through host_row_problems, a plain push with a pull_request under `types: [closed]` passes, and the pull_request under `types: [closed]` alone is listed with "pull_request: types". Control: the parent, 9e7b065, with the new test file fails the case (failures=1) at the first `types:`, and against the parent's manifest.py, workflow_filters reads both `types:` fixtures as no filter and host_row_problems passes both rows. 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 nothing. 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 | 42 +++++++++++++++++++------------- forgetest/tests/test_manifest.py | 18 +++++++++++--- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/forgetest/forgetest/manifest.py b/forgetest/forgetest/manifest.py index 96c5633..1a5d701 100644 --- a/forgetest/forgetest/manifest.py +++ b/forgetest/forgetest/manifest.py @@ -119,7 +119,8 @@ BEHAVIORAL = [ # "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 a push or a pull_request with no branches, -# tags or paths filter under it (one of the two is enough), in a step +# tags or paths filter under it, and no types under the pull_request +# (one of the two events 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 @@ -509,23 +510,29 @@ def workflow_triggers(text): # 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. +# run: a ref or a path the event does not match, and under a pull_request +# alone, an activity type the pull request's event is not. _FILTER_KEYS = ("branches", "branches-ignore", "tags", "tags-ignore", "paths", "paths-ignore") +_PULL_REQUEST_FILTER_KEYS = _FILTER_KEYS + ("types",) 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.""" + `tags-ignore:`, `paths:` or `paths-ignore:` key under the event, and + under a pull_request a `types:` key too, 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 pull_request under `types:` runs on the activity types it + names and no other, so one under `types: [closed]` runs on no change + of a pull request; the lint reads no type, so any `types:` is a + filter. 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): @@ -555,7 +562,7 @@ def workflow_filters(text): 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: + elif km and km.group(2) in (_PULL_REQUEST_FILTER_KEYS if event == "pull_request" else _FILTER_KEYS): out.setdefault(event, []).append(km.group(2)) return out return out @@ -1161,10 +1168,11 @@ def host_row_problems(manifest, host=HOST_TESTED, root=REPO_ROOT, sources=SOURCE 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), + (workflow_filters: a branches, tags or paths filter, or a types + filter on the pull_request, 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 diff --git a/forgetest/tests/test_manifest.py b/forgetest/tests/test_manifest.py index 08300f6..091440f 100644 --- a/forgetest/tests/test_manifest.py +++ b/forgetest/tests/test_manifest.py @@ -482,6 +482,12 @@ class CoverageReportTests(unittest.TestCase): {"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 pull_request under types: runs on the activity types it names and no other; the lint + # reads no type, so any types: is a filter, on the pull_request alone + self.assertEqual(f("on:\n push:\n pull_request:\n types: [closed]\n"), {"pull_request": ["types"]}) + self.assertEqual(f("on:\n pull_request:\n types:\n - opened\n - synchronize\n paths-ignore:\n" + " - '**.md'\n"), {"pull_request": ["types", "paths-ignore"]}) + self.assertEqual(f("on:\n push:\n types: [x]\n"), {}) # 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"]}) @@ -495,20 +501,24 @@ class CoverageReportTests(unittest.TestCase): 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 + # does not, nor do a push and a pull request each under a filter; every push with a pull + # request under types: does, a pull request under types: alone does not 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}) + " paths:\n - 'src/**'\n" + job, + ".github/workflows/push.yml": "on:\n push:\n pull_request:\n types: [closed]\n" + job, + ".github/workflows/closed.yml": "on:\n pull_request:\n types: [closed]\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")] + "forgefirm:.github/workflows/%s.yml" % w) for w in ("main", "docs", "both", "push", "closed")] 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",)]) + rows[2][:3] + (rows[2][3] + filtered % "pull_request: paths; push: branches",), + rows[4][:3] + (rows[4][3] + filtered % "pull_request: types",)]) def test_the_lint_says_when_it_could_not_read_a_workflow(self): import contextlib