From 335c6dea9da5d4317e200c0b35ab29e4fe7382fb Mon Sep 17 00:00:00 2001 From: ScottW514 Date: Sun, 23 Aug 2026 12:54:55 -0400 Subject: [PATCH] forgetest: non-behavioral paths outside every fingerprint; hollow covers fail the lint The coverage lint already allowed docs, CI, unit tests and licenses to go uncovered; the same list now keeps them out of every fingerprint, so a README edit in any component re-requires nothing. The list moves to the manifest module as NON_BEHAVIORAL, the one place both uses read it. And a coverage entry that selects no file of its component (a glob without the recipe's subdirectory, a component the manifest lacks, a glob naming docs only) fails the lint: such an entry covers nothing and the test's fingerprint ignores the file it meant. The contract says both. Every test whose maps reached a doc or a test file gets a new fingerprint once. --- docs/ACCEPTANCE.md | 26 +++++++++++++--- forgetest/forgetest/coverage.py | 44 ++++++++++++--------------- forgetest/forgetest/manifest.py | 52 +++++++++++++++++++++++++++++++- forgetest/tests/test_manifest.py | 25 +++++++++++++++ 4 files changed, 118 insertions(+), 29 deletions(-) diff --git a/docs/ACCEPTANCE.md b/docs/ACCEPTANCE.md index 30dbeaa..a4c30b1 100644 --- a/docs/ACCEPTANCE.md +++ b/docs/ACCEPTANCE.md @@ -34,7 +34,12 @@ Every test declares, in code (`forgetest/forgetest/suite/*.py`): mode it finds, or manages the mode itself (the `cloud.*` job tests, through `enter_cloud`, which also waits for the service session); - **covers** - the source paths whose content the test stands for, as - `(component, glob)` pairs; + `(component, glob)` pairs. Globs anchor at the component's repository + root (`forgefirm-app/gfcloud.py`, not `gfcloud.py`), and a glob that + selects nothing is a lint failure. Non-behavioral paths (docs, CI, the + components' own unit tests, licenses; the list is `NON_BEHAVIORAL` in + `forgetest/forgetest/manifest.py`) are outside every fingerprint, so a + README edit re-requires nothing; - **requires** - tests that must be satisfied first (the emission tests require the motion and readback tests). This orders the runs; it is not a release condition of its own (the release needs every test satisfied @@ -171,6 +176,19 @@ bench, or one whose `/data` has been wiped, starts from a full campaign. latch, so they stay `live`. The offline client is left running; the next test that needs the service restarts it (`enter_cloud` does), as does a mode switch or a controller restart. + **The coverage maps follow the split.** The protocol test stands for + the web session, the emulator and its fixtures; the offline tests for + the run loop, the hardware it drives, the offline dispatch and the + pulse path; `cloud.mode-switch` for the homing path (the session, the + whole hardware library, `gfhome`); and every one of them for the + client's common ground (`gfcloud.py`, `ffmachine.py`, the config, the + identity, the cooling reporter, gfutilities' core and its transport + helpers). The one real print keeps the coarse maps, all three cloud + components whole: it is the integration, and the floor the lint needs, + so whatever the finer maps leave out still re-requires it. A sign-in + change therefore re-requires the protocol test and the print; a feeder + change the offline tests and the print; a camera change the mode + switch and the print. **The service's connect-time hunt is paid only where it is the subject.** A cloud client the tool starts for anything else (the real client back after the emulator, a mode the runner switches to or hands @@ -341,9 +359,9 @@ The mechanical floor is the coverage lint, python3 -m forgetest.coverage --manifest [--enforce] -which lists every manifest path no test covers, minus the allowlist of -non-behavioral paths in `forgetest/forgetest/coverage.py` (docs, CI, tests, -licenses). CI (`forgetest-ci.yml`) runs it on a manifest generated from the +which lists every manifest path no test covers, minus the non-behavioral +paths (docs, CI, tests, licenses), and every coverage entry that selects +nothing. CI (`forgetest-ci.yml`) runs it on a manifest generated from the recipe pins with `scripts/manifest-from-tree.py` (no Yocto build needed) and fails the job on any uncovered path. On the board, run it against `/etc/forgefirm-manifest.json`. The lint proves a diff --git a/forgetest/forgetest/coverage.py b/forgetest/forgetest/coverage.py index c7f34c9..f51917e 100644 --- a/forgetest/forgetest/coverage.py +++ b/forgetest/forgetest/coverage.py @@ -1,11 +1,14 @@ """Coverage lint: every source path in the manifest must be selected by -some test's coverage globs, except the allowlisted non-behavioral paths. +some test's coverage globs, except the allowlisted non-behavioral paths, +and every coverage glob must select at least one path of its component. Why a hard rule: under the domain model an uncovered file is worse than an untested one - a change there leaves every inherited PASS valid when it should have invalidated them. The lint is the floor (the file is fingerprinted by at least one test); whether that test exercises the -change stays with the change author. +change stays with the change author. An entry that selects nothing is +the same defect from the other side: the test's fingerprint ignores the +file it named. python3 -m forgetest.coverage [--manifest PATH] [--enforce] [--json] @@ -19,30 +22,19 @@ import sys from . import catalog as _catalog from . import manifest as _manifest -# Non-behavioral paths that need no acceptance coverage. Reviewed with the -# catalog: widening this list is a change like any other. -ALLOW = [ - ("*", ".github/**"), - ("*", ".gitignore"), - ("*", ".gitmodules"), - ("*", "**/*.md"), - ("*", "LICENSE*"), - ("*", "COPYING*"), - ("*", "docs/**"), - ("*", "tests/**"), - ("*", "graphify-out/**"), - ("*", "**/.gitkeep"), - ("*", ".devcontainer/**"), # editor/dev-environment setup, no target - ("*", ".vscode/**"), # behavior - ("*", ".env.example"), - ("forgectrl", "tools/**"), # host-side dev tools (panel dev server) -] +# Non-behavioral paths need no acceptance coverage, and no fingerprint +# carries them: the one list, in the manifest module. +ALLOW = _manifest.NON_BEHAVIORAL def run(manifest, tests, allow=ALLOW): return _manifest.coverage_report(manifest, tests, allow) +def empty(manifest, tests): + return _manifest.empty_covers(manifest, tests) + + def main(argv=None): ap = argparse.ArgumentParser(description="forgetest coverage lint") ap.add_argument("--manifest", default=None, help="manifest JSON (default: the running image's)") @@ -58,16 +50,20 @@ def main(argv=None): tests = _catalog.all_tests(registry) report = run(manifest, tests) total = sum(len(v) for v in report.values()) + hollow = empty(manifest, tests) if args.json: - print(json.dumps({"uncovered": report, "total": total, "tests": len(tests)}, indent=1, sort_keys=True)) + print(json.dumps({"uncovered": report, "total": total, "tests": len(tests), + "empty": [list(e) for e in hollow]}, indent=1, sort_keys=True)) else: for comp in sorted(report): print("%s: %d uncovered path(s)" % (comp, len(report[comp]))) for p in report[comp]: print(" %s" % p) - print("coverage: %d uncovered path(s) across %d component(s), %d tests" - % (total, len(report), len(tests))) - if args.enforce and total: + for tid, comp, pat in hollow: + print("%s: covers (%s, %s) selects nothing" % (tid, comp, pat)) + print("coverage: %d uncovered path(s) across %d component(s), %d empty entr%s, %d tests" + % (total, len(report), len(hollow), "y" if len(hollow) == 1 else "ies", len(tests))) + if args.enforce and (total or hollow): return 1 return 0 diff --git a/forgetest/forgetest/manifest.py b/forgetest/forgetest/manifest.py index 3930a43..6c50ec0 100644 --- a/forgetest/forgetest/manifest.py +++ b/forgetest/forgetest/manifest.py @@ -67,6 +67,37 @@ def match_files(files, pattern): return [(p, b) for p, b in files if rx.match(p)] +# Paths that carry no target behavior: docs, CI, the components' own unit +# tests, licenses, editor setup. Outside every fingerprint (a README edit +# re-requires nothing) and outside the coverage lint (no test has to name +# them). Reviewed with the catalog: widening this list is a change like +# any other. "*" applies to every component. +NON_BEHAVIORAL = [ + ("*", ".github/**"), + ("*", ".gitignore"), + ("*", ".gitattributes"), + ("*", ".gitmodules"), + ("*", "**/*.md"), + ("*", "LICENSE*"), + ("*", "COPYING*"), + ("*", "docs/**"), + ("*", "tests/**"), + ("*", "graphify-out/**"), + ("*", "**/.gitkeep"), + ("*", ".devcontainer/**"), + ("*", ".vscode/**"), + ("*", ".env.example"), + ("forgectrl", "tools/**"), # host-side dev tools (panel dev server) +] + + +def non_behavioral(comp, path, allow=NON_BEHAVIORAL): + for c, pat in allow: + if c in ("*", comp) and glob_to_regex(pat).match(path): + return True + return False + + class Manifest: def __init__(self, data): self.data = data @@ -133,6 +164,8 @@ def fingerprint(manifest, covers, extra=()): parts.add((comp, "@missing", "")) continue for p, b in match_files(files, pat): + if non_behavioral(comp, p): + continue parts.add((comp, p, b)) h = hashlib.sha256() h.update(canonical(sorted(parts)).encode("utf-8")) @@ -144,7 +177,24 @@ def fingerprint(manifest, covers, extra=()): return h.hexdigest() -def coverage_report(manifest, tests, allow=()): +def empty_covers(manifest, tests): + """Coverage entries that select no file of their component: a glob + that never matched (paths anchor at the repository root, so a file + under a recipe's subdirectory needs that directory in the glob), or + a component not in the manifest. Such an entry covers nothing, and + the test's fingerprint would not move with the file it meant. + Returns [(test id, component, glob)]. A glob selecting only + non-behavioral paths is hollow too: nothing it names is fingerprinted.""" + out = [] + for t in tests: + for comp, pat in t.covers: + files = manifest.files(comp) + if files is None or not [p for p, _b in match_files(files, pat) if not non_behavioral(comp, p)]: + out.append((t.id, comp, pat)) + return out + + +def coverage_report(manifest, tests, allow=NON_BEHAVIORAL): """Which manifest paths no test covers. tests: iterable with .covers. allow: iterable of (component, glob) diff --git a/forgetest/tests/test_manifest.py b/forgetest/tests/test_manifest.py index c6955f5..6e1b655 100644 --- a/forgetest/tests/test_manifest.py +++ b/forgetest/tests/test_manifest.py @@ -115,6 +115,31 @@ class CoverageReportTests(unittest.TestCase): self.assertNotIn("forgetest", m.coverage_report(man, [], allow=[]), "dev-only components are outside the report") + def test_non_behavioral_paths_are_outside_every_fingerprint(self): + man = helpers.make_manifest() + t = helpers.make_test("a.one", [("forgectrl", "**")]) + a = m.fingerprint(man, t.covers) + for comp, path in (("forgectrl", "README.md"), ("forgectrl", "docs/SERVICES.md"), + ("forgectrl", "tests/test_x.py"), ("forgectrl", ".github/workflows/ci.yml")): + self.assertEqual(a, m.fingerprint(helpers.with_file(man, comp, path, "changed"), t.covers), + path) + self.assertNotEqual(a, m.fingerprint(helpers.with_file(man, "forgectrl", "src/ui.c", "changed"), + t.covers)) + self.assertTrue(m.non_behavioral("forgectrl", "tools/devserver.py")) + self.assertFalse(m.non_behavioral("grblhal-glowforge", "tools/devserver.py")) + + def test_an_entry_that_selects_nothing_is_reported(self): + man = helpers.make_manifest() + good = helpers.make_test("a.one", [("forgectrl", "src/ui.c")]) + # the recipe's subdirectory left out of the glob, and an unknown component + bad = helpers.make_test("a.two", [("forgectrl", "ui.c"), ("no-such", "**")]) + self.assertEqual(m.empty_covers(man, [good]), []) + self.assertEqual(m.empty_covers(man, [good, bad]), + [("a.two", "forgectrl", "ui.c"), ("a.two", "no-such", "**")]) + # a glob that selects docs only names nothing any fingerprint carries + docs = helpers.make_test("a.three", [("forgectrl", "**/*.md")]) + self.assertEqual(m.empty_covers(man, [docs]), [("a.three", "forgectrl", "**/*.md")]) + class CatalogTests(unittest.TestCase): def test_catalog_hash_is_definition_only(self):