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

@@ -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 <manifest.json> [--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

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
 

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)

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):