mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-10-19 02:51:42 -07:00
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.
This commit is contained in:
@@ -119,7 +119,8 @@ 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 a push or a pull_request with no branches,
|
# 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
|
# 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`
|
# `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
|
# 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
|
# 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")
|
_FILTER_KEYS = ("branches", "branches-ignore", "tags", "tags-ignore", "paths", "paths-ignore")
|
||||||
|
_PULL_REQUEST_FILTER_KEYS = _FILTER_KEYS + ("types",)
|
||||||
|
|
||||||
|
|
||||||
def workflow_filters(text):
|
def workflow_filters(text):
|
||||||
"""The events of a workflow's `on:` block mapping that carry a filter,
|
"""The events of a workflow's `on:` block mapping that carry a filter,
|
||||||
{event: [what]}: each `branches:`, `branches-ignore:`, `tags:`,
|
{event: [what]}: each `branches:`, `branches-ignore:`, `tags:`,
|
||||||
`tags-ignore:`, `paths:` or `paths-ignore:` key under the event, by
|
`tags-ignore:`, `paths:` or `paths-ignore:` key under the event, and
|
||||||
name; a value on the event's own line (`push: {branches: [main]}`),
|
under a pull_request a `types:` key too, by name; a value on the
|
||||||
which the lint does not read, as "a value the lint does not read";
|
event's own line (`push: {branches: [main]}`), which the lint does
|
||||||
and a key with one stray quote under the event (`paths":`), as "a
|
not read, as "a value the lint does not read"; and a key with one
|
||||||
key the lint does not read". Every filter counts: a push with only
|
stray quote under the event (`paths":`), as "a key the lint does not
|
||||||
a branches or a tags filter runs on no push of the other kind of
|
read". Every filter counts: a push with only a branches or a tags
|
||||||
ref, and a branch it names is one the lint does not know, as the
|
filter runs on no push of the other kind of ref, and a branch it
|
||||||
manifest carries no branch. A key deeper than the event's own level
|
names is one the lint does not know, as the manifest carries no
|
||||||
(a dispatch's inputs) is no filter. An event of a list or a scalar
|
branch. A pull_request under `types:` runs on the activity types it
|
||||||
`on:` carries none."""
|
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)
|
lines = _lines(text)
|
||||||
out = {}
|
out = {}
|
||||||
for i, ln in enumerate(lines):
|
for i, ln in enumerate(lines):
|
||||||
@@ -555,7 +562,7 @@ def workflow_filters(text):
|
|||||||
continue
|
continue
|
||||||
if _stray_key(child):
|
if _stray_key(child):
|
||||||
out.setdefault(event, []).append("a key the lint does not read")
|
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))
|
out.setdefault(event, []).append(km.group(2))
|
||||||
return out
|
return out
|
||||||
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
|
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),
|
workflow that calls it, and a flow mapping, which names no event),
|
||||||
one whose every push and pull_request carries a filter
|
one whose every push and pull_request carries a filter
|
||||||
(workflow_filters: a branches, tags or paths filter, which the lint
|
(workflow_filters: a branches, tags or paths filter, or a types
|
||||||
does not read, so the workflow runs on not every change of the
|
filter on the pull_request, which the lint does not read, so the
|
||||||
covered files, or on not every branch; one push or pull_request
|
workflow runs on not every change of the covered files, or on not
|
||||||
with no filter makes the workflow run on every change),
|
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
|
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
|
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
|
workflow runs its test is not checked where it cannot be read (see
|
||||||
|
|||||||
@@ -482,6 +482,12 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
{"push": ["branches-ignore", "paths-ignore"], "pull_request": ["paths"]})
|
{"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: ['v*']\n"), {"push": ["tags"]})
|
||||||
self.assertEqual(f("on:\n push:\n tags-ignore: ['v*']\n"), {"push": ["tags-ignore"]})
|
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
|
# a value on the event's line, and a key with one stray quote under it, are filters the
|
||||||
# lint does not read
|
# 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: {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"), {})
|
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
|
# 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
|
# 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()
|
man = helpers.make_manifest()
|
||||||
job = "jobs:\n test:\n steps:\n - uses: actions/checkout@v4\n - run: python3 scripts/bench/stream_test.py\n"
|
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": "",
|
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/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/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"
|
".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",
|
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)"
|
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),
|
self.assertEqual(m.host_row_problems(man, rows, root=root, sources=None),
|
||||||
[rows[1][:3] + (rows[1][3] + filtered % "push: paths-ignore",),
|
[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):
|
def test_the_lint_says_when_it_could_not_read_a_workflow(self):
|
||||||
import contextlib
|
import contextlib
|
||||||
|
|||||||
Reference in New Issue
Block a user