mirror of
https://github.com/openglow-org/forgefirm.git
synced 2026-10-19 02:51:42 -07:00
forgetest: the lint counts a step as run only when every job its job needs runs
The coverage lint judges a HOST_TESTED row by a step of its workflow
that runs on every run of the workflow. It read the `if:` and the
`continue-on-error:` of a step and of its job, and not the job's
`needs:`. GitHub skips a job whose needed job did not run (an `if:`
that held it back, or a failure), unless the job says `always()`, so
a test step in a job that needs a conditional job runs on not every
push, and counted all the same: a host test behind a conditional
build keeps its row green and its paths covered. No workflow a row
names uses `needs:`.
workflow_steps now reads each job's `needs:` (_needs: a flow list,
one scalar, or a block list), keeps the jobs by name, and resolves
them when the workflow is read (_needs_run): a job's needs are met
on every run when every job it names runs on every run, that is,
carries no `if:` other than the literal true and has its own needs
met, transitively. A job the workflow does not have, a cycle, and a
`needs:` in a form the lint does not read each fail closed. A needed
job's `continue-on-error:` does not bear: its failure fails neither
the run nor the jobs after it. The job's dict carries the result as
"needs_run", and _on_every_run counts no step of a job whose needs
are not met, so workflow_runs and checkout_path refuse it as they
refuse a conditional step. The reason stays "does not run it". The
lint evaluates no condition, so a dependent job under `always()`
counts as no job, as a step under `always()` counts as no step. The
HOST_TESTED comment and the docstrings say what the lint reads.
Proof: forgetest-ci "Unit tests" (`python -B -m unittest discover -s
tests -v` in forgetest): test_manifest.py runs 40 tests, OK, on
Ubuntu 24.04 with python 3.12, in a private PID namespace. The new
case test_a_job_runs_only_when_every_job_it_needs_runs: a job that
needs an unconditional job runs its test, with the `needs:` as a
scalar, a flow list, a quoted flow list and a block list; the same
four with the needed job under `if: false` or `if: github.event_name
== 'push'` do not; a needed job under `continue-on-error: true`
runs it; a chain of three runs it, and does not with the first job
under `if: false`; a needed job the workflow does not have, a cycle
of two, a self-need and a `needs:` holding a mapping each do not;
`if: always()` on the dependent job, and on the middle job of the
chain, does not. test_the_workflow_has_to_run_the_host_test gains a
workflow whose test job needs a job under `if: false`, listed as
"does not run it". Control: the parent, e1d1c1b, with the new test
file fails both cases (failures=2). 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.
This commit is contained in:
@@ -121,8 +121,9 @@ BEHAVIORAL = [
|
|||||||
# must run the host test: on push or pull request, in a step that runs on
|
# 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
|
# 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
|
# `true` and no `continue-on-error:` other than `false` on the step or its
|
||||||
# job, and no key with one stray quote where the lint reads the workflow's
|
# job, every job the job's `needs:` names one that runs on every run, and
|
||||||
# keys), under bash or sh, on a line of the step's run text whose failure
|
# 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
|
# 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
|
# 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
|
# opens a compound command: workflow_runs), and name it, or discover it (a
|
||||||
@@ -506,6 +507,56 @@ def workflow_triggers(text):
|
|||||||
return set()
|
return set()
|
||||||
|
|
||||||
|
|
||||||
|
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
|
||||||
|
it. None for a form the lint does not read (a line under the key
|
||||||
|
that is no list item): a need no job meets."""
|
||||||
|
val = _KEY_RX.match(lines[i]).group(3).strip()
|
||||||
|
if val.startswith("["):
|
||||||
|
return [_scalar(x) for x in val.strip("[]").split(",") if x.strip()]
|
||||||
|
if val:
|
||||||
|
return [_scalar(val)]
|
||||||
|
out = []
|
||||||
|
for ln in lines[i + 1:]:
|
||||||
|
if not ln.strip():
|
||||||
|
continue
|
||||||
|
if _indent(ln) <= _indent(lines[i]):
|
||||||
|
break
|
||||||
|
body = ln.strip()
|
||||||
|
if not body.startswith("- "):
|
||||||
|
return None
|
||||||
|
out.append(_scalar(body[2:]))
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def _needs_run(jobs):
|
||||||
|
"""Whether each job's `needs:` are met on every run of the workflow,
|
||||||
|
as "needs_run" on the job's dict: True when every job it needs runs
|
||||||
|
on every run (no `if:` other than the literal true, and its own
|
||||||
|
needs met, transitively), False when one does not, names no job of
|
||||||
|
the workflow, or is in a cycle with it, or when the `needs:` is a
|
||||||
|
form the lint does not read. The lint evaluates no condition, so a
|
||||||
|
job under `always()` counts as no job, as its steps do. A needed
|
||||||
|
job's `continue-on-error:` does not bear: its failure fails neither
|
||||||
|
the run nor the jobs after it."""
|
||||||
|
state = {}
|
||||||
|
|
||||||
|
def runs(name, trail):
|
||||||
|
if name in state:
|
||||||
|
return state[name]
|
||||||
|
job = jobs.get(name)
|
||||||
|
if job is None or name in trail:
|
||||||
|
return False
|
||||||
|
needs = job.get("needs", [])
|
||||||
|
met = needs is not None and all(runs(n, trail | {name}) for n in needs)
|
||||||
|
job["needs_run"] = met
|
||||||
|
state[name] = met and job.get("if", "true").lower() == "true"
|
||||||
|
return state[name]
|
||||||
|
for name in jobs:
|
||||||
|
runs(name, frozenset())
|
||||||
|
|
||||||
|
|
||||||
def _defaults(lines, i):
|
def _defaults(lines, i):
|
||||||
"""{"shell": its value} for the `defaults:` key at lines[i] when it
|
"""{"shell": its value} for the `defaults:` key at lines[i] when it
|
||||||
gives the run steps a shell:, {} when it gives none. A flow mapping
|
gives the run steps a shell:, {} when it gives none. A flow mapping
|
||||||
@@ -534,7 +585,9 @@ def workflow_steps(text):
|
|||||||
"working-directory", "with" (a dict), "shell", "if" and
|
"working-directory", "with" (a dict), "shell", "if" and
|
||||||
"continue-on-error" as written; "job": the "if" and
|
"continue-on-error" as written; "job": the "if" and
|
||||||
"continue-on-error" of the job the step is in, when it is in one,
|
"continue-on-error" of the job the step is in, when it is in one,
|
||||||
and the "shell" its `defaults:` give; and "workflow": the "shell" the
|
its "needs" (_needs) and whether they are met on every run
|
||||||
|
("needs_run", _needs_run), and the "shell" its `defaults:` give; and
|
||||||
|
"workflow": the "shell" the
|
||||||
workflow's own `defaults:` give, when they give one (_defaults). YAML
|
workflow's own `defaults:` give, when they give one (_defaults). YAML
|
||||||
read by its indentation, as far as these workflows use it; comments,
|
read by its indentation, as far as these workflows use it; comments,
|
||||||
whole lines and from ' #' to the end of a line, are not read.
|
whole lines and from ' #' to the end of a line, are not read.
|
||||||
@@ -547,7 +600,7 @@ def workflow_steps(text):
|
|||||||
then carries "unread" and counts as no step."""
|
then carries "unread" and counts as no step."""
|
||||||
lines = _lines(text)
|
lines = _lines(text)
|
||||||
steps, i = [], 0
|
steps, i = [], 0
|
||||||
in_jobs, job_at, key_at, job = False, None, None, None
|
in_jobs, job_at, key_at, job, jobs = False, None, None, None, {}
|
||||||
stray, workflow = False, {}
|
stray, workflow = False, {}
|
||||||
while i < len(lines):
|
while i < len(lines):
|
||||||
km = _KEY_RX.match(lines[i])
|
km = _KEY_RX.match(lines[i])
|
||||||
@@ -559,10 +612,13 @@ def workflow_steps(text):
|
|||||||
workflow.update(_defaults(lines, i))
|
workflow.update(_defaults(lines, i))
|
||||||
elif in_jobs and (job_at is None or ind == job_at):
|
elif in_jobs and (job_at is None or ind == job_at):
|
||||||
job_at, key_at, job = ind, None, {}
|
job_at, key_at, job = ind, None, {}
|
||||||
|
jobs[key] = job
|
||||||
elif in_jobs and job_at is not None and ind > job_at and (key_at is None or ind == key_at):
|
elif in_jobs and job_at is not None and ind > job_at and (key_at is None or ind == key_at):
|
||||||
key_at = ind
|
key_at = ind
|
||||||
if key in _CONDITION_KEYS:
|
if key in _CONDITION_KEYS:
|
||||||
job[key] = _scalar(km.group(3))
|
job[key] = _scalar(km.group(3))
|
||||||
|
elif key == "needs":
|
||||||
|
job["needs"] = _needs(lines, i)
|
||||||
elif key == "defaults":
|
elif key == "defaults":
|
||||||
job.update(_defaults(lines, i))
|
job.update(_defaults(lines, i))
|
||||||
elif _stray_key(lines[i]):
|
elif _stray_key(lines[i]):
|
||||||
@@ -620,6 +676,7 @@ def workflow_steps(text):
|
|||||||
if job is not None:
|
if job is not None:
|
||||||
step["job"] = job # the job's own dict: a job key written after its steps counts too
|
step["job"] = job # the job's own dict: a job key written after its steps counts too
|
||||||
steps.append(step)
|
steps.append(step)
|
||||||
|
_needs_run(jobs)
|
||||||
for s in steps:
|
for s in steps:
|
||||||
if stray:
|
if stray:
|
||||||
s["unread"] = True
|
s["unread"] = True
|
||||||
@@ -632,13 +689,18 @@ def _on_every_run(step):
|
|||||||
"""Whether the lint counts a step as one that runs on every run of
|
"""Whether the lint counts a step as one that runs on every run of
|
||||||
its workflow and fails the run when it fails: neither the step nor
|
its workflow and fails the run when it fails: neither the step nor
|
||||||
its job carries an `if:` other than the literal `true` or a
|
its job carries an `if:` other than the literal `true` or a
|
||||||
`continue-on-error:` other than the literal `false`, and the
|
`continue-on-error:` other than the literal `false`, the job's
|
||||||
workflow has no key the lint cannot read ("unread"). The lint
|
`needs:` are met on every run (_needs_run: GitHub skips a job whose
|
||||||
evaluates no condition, so a step under `always()` or `success()`
|
needed job did not run, unless the job says `always()`, which the
|
||||||
counts as no step too."""
|
lint does not evaluate), and the workflow has no key the lint
|
||||||
|
cannot read ("unread"). The lint evaluates no condition, so a step
|
||||||
|
under `always()` or `success()` counts as no step too."""
|
||||||
if step.get("unread"):
|
if step.get("unread"):
|
||||||
return False
|
return False
|
||||||
for d in (step, step.get("job") or {}):
|
job = step.get("job") or {}
|
||||||
|
if not job.get("needs_run", True):
|
||||||
|
return False
|
||||||
|
for d in (step, job):
|
||||||
if d.get("if", "true").lower() != "true" or d.get("continue-on-error", "false").lower() != "false":
|
if d.get("if", "true").lower() != "true" or d.get("continue-on-error", "false").lower() != "false":
|
||||||
return False
|
return False
|
||||||
return True
|
return True
|
||||||
|
|||||||
@@ -254,6 +254,16 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
self.assertEqual(m.host_row_problems(man, [manual, soft], root=root, sources=None),
|
self.assertEqual(m.host_row_problems(man, [manual, soft], root=root, sources=None),
|
||||||
[manual[:3] + (manual[3] + ": the lint reads no push or pull_request in its own on:",),
|
[manual[:3] + (manual[3] + ": the lint reads no push or pull_request in its own on:",),
|
||||||
soft[:3] + (soft[3] + " does not run it",)])
|
soft[:3] + (soft[3] + " does not run it",)])
|
||||||
|
# a step in a job that needs a conditional job does not run it (the checkout in a job of
|
||||||
|
# its own, as the lint reads a checkout for the whole workflow)
|
||||||
|
root = self._root({"scripts/bench/stream_test.py": "",
|
||||||
|
".github/workflows/needs.yml": "on: [push]\njobs:\n lint:\n steps:\n"
|
||||||
|
" - uses: actions/checkout@v4\n build:\n if: false\n steps:\n"
|
||||||
|
" - run: make\n test:\n needs: build\n steps:\n"
|
||||||
|
" - run: python3 scripts/bench/stream_test.py\n"})
|
||||||
|
needs = runs[:3] + ("forgefirm:.github/workflows/needs.yml",)
|
||||||
|
self.assertEqual(m.host_row_problems(man, [needs], root=root, sources=None),
|
||||||
|
[needs[:3] + (needs[3] + " does not run it",)])
|
||||||
|
|
||||||
def _runs(self, yml, path):
|
def _runs(self, yml, path):
|
||||||
return m.workflow_runs(m.workflow_steps(yml), path)
|
return m.workflow_runs(m.workflow_steps(yml), path)
|
||||||
@@ -396,6 +406,35 @@ class CoverageReportTests(unittest.TestCase):
|
|||||||
self.assertEqual(m.checkout_path(m.workflow_steps(yml % ""), "x", True), "own")
|
self.assertEqual(m.checkout_path(m.workflow_steps(yml % ""), "x", True), "own")
|
||||||
self.assertIsNone(m.checkout_path(m.workflow_steps(yml % " if: false\n"), "x", True))
|
self.assertIsNone(m.checkout_path(m.workflow_steps(yml % " if: false\n"), "x", True))
|
||||||
|
|
||||||
|
def test_a_job_runs_only_when_every_job_it_needs_runs(self):
|
||||||
|
test = "tests/tray_test.c"
|
||||||
|
# GitHub skips a job whose needed job did not run: a job that needs a conditional job runs
|
||||||
|
# on not every run, however the needs: is written
|
||||||
|
yml = "jobs:\n build:\n%s steps:\n - run: make\n test:\n%s steps:\n - run: ./build/tray_test\n"
|
||||||
|
for needs in (" needs: build\n", " needs: [build]\n", " needs: ['build']\n",
|
||||||
|
" needs:\n - build\n"):
|
||||||
|
self.assertTrue(self._runs(yml % ("", needs), test), needs)
|
||||||
|
self.assertFalse(self._runs(yml % (" if: false\n", needs), test), needs)
|
||||||
|
self.assertFalse(self._runs(yml % (" if: github.event_name == 'push'\n", needs), test), needs)
|
||||||
|
# the needed job's continue-on-error: does not bear: its failure fails neither the run nor
|
||||||
|
# the jobs after it
|
||||||
|
self.assertTrue(self._runs(yml % (" continue-on-error: true\n", " needs: build\n"), test))
|
||||||
|
# transitively; a job the workflow does not have, a cycle, and a needs: the lint does not
|
||||||
|
# read each fail closed
|
||||||
|
chain = "jobs:\n a:\n%s steps:\n - run: make\n b:\n needs: a\n steps:\n - run: make\n" \
|
||||||
|
" c:\n needs: [b]\n steps:\n - run: ./build/tray_test\n"
|
||||||
|
self.assertTrue(self._runs(chain % "", test))
|
||||||
|
self.assertFalse(self._runs(chain % " if: false\n", test))
|
||||||
|
self.assertFalse(self._runs(yml % ("", " needs: lint\n"), test))
|
||||||
|
self.assertFalse(self._runs(yml % (" needs: test\n", " needs: build\n"), test))
|
||||||
|
self.assertFalse(self._runs(yml % ("", " needs: test\n"), test))
|
||||||
|
self.assertFalse(self._runs(yml % ("", " needs:\n build: true\n"), test))
|
||||||
|
# a job under always() runs on every run, and the lint evaluates no condition: it counts as
|
||||||
|
# no job for its own steps and for the jobs after it
|
||||||
|
self.assertFalse(self._runs(yml % (" if: false\n", " needs: build\n if: always()\n"), test))
|
||||||
|
self.assertFalse(self._runs((chain % " if: false\n").replace(
|
||||||
|
" b:\n needs: a\n", " b:\n needs: a\n if: ${{ always() }}\n"), test))
|
||||||
|
|
||||||
def test_the_events_a_workflow_runs_on(self):
|
def test_the_events_a_workflow_runs_on(self):
|
||||||
self.assertEqual(m.workflow_triggers("on:\n push:\n branches: [main]\n pull_request:\njobs:\n t:\n"),
|
self.assertEqual(m.workflow_triggers("on:\n push:\n branches: [main]\n pull_request:\njobs:\n t:\n"),
|
||||||
{"push", "pull_request"})
|
{"push", "pull_request"})
|
||||||
|
|||||||
Reference in New Issue
Block a user