diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 369d08c..c95ecd1 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -31,6 +31,53 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre ### Tests +- **The hot path's "zero allocations" rule got a gate.** New + `tests/test_hot_path_allocations.py` pins that `decide()` RETAINS nothing per packet: at most + 64 blocks and 4096 bytes over 5000 calls, against a measured floor of 13 blocks and 608 bytes + that stays flat with duplication, latency, the NAT flow table and 200 ports. `test_hot_path.py` + guards the neighbouring rule (no syscalls on the packet threads); this catches the other class, + where something starts being kept once per packet. + 🔴 **It reads two meters because one of them has a hole, and the hole was found by mutation + rather than by reasoning.** The first version counted `sys.getallocatedblocks()` only, and a + mutation making `decide()` append to an ever-growing list **survived**: the appended value was + a cached small int, so no object was created and blocks moved from 6 to 7. `tracemalloc` + current sees the same case as 42 032 bytes against 208. The meter-canary test now proves both + meters can rise, including the references-only case specifically. Still not caught, stated in + the file: transient garbage, since both meters are net. +- **Size ceilings, as a ratchet.** New `tests/test_code_shape.py` caps a function at 167 logic + lines and a module at 1299 - today's maxima (`theme.py::init_style`, `gui/app.py`), so nothing + had to be rewritten to make it pass. Lowering them is routine work, raising either is the + owner's call. **Comments and docstrings do not count**, and that is measured rather than + assumed: 9 776 of the package's 17 770 lines are logic, so 45% is explanation, and a raw-line + cap would have been a cap on explaining. A second test pins that property directly (ninety + lines of comment measure the same as none). Both mutation-checked, plus the empty-scan canary. +- **"Verified by mutation" became checkable.** New `tests/test_mutation_registry.py` holds the + claim as data in three states that say three different things: `MUTATIONS` (re-runnable now, + five entries), `PROVEN_BY_HAND` (a dated session did it, no patch was written down, so no + machine repeats it) and `NOT_PROVEN` (no mutation, said out loud). Until now the phrase + appeared over twenty times in the notes and in a dozen docstrings with nothing behind it - + convention 5's own evidence was the prose convention 5 warns about. The suite checks the + bookkeeping only: every named test exists, none is filed under two states, and every search + pattern still occurs exactly once, so a registry entry rots the day the code moves rather + than the day someone runs it. The mutations themselves run from `internal_tools/mutate.py` + (outside git, one subprocess suite run each), with a mandatory canary entry that must report + BROKEN - a tree that fails to compile also exits non-zero, so without it a whole run can + report "all caught" and mean nothing. First full run: 5 caught, 0 survived, canary BROKEN. +- **The repository scanners now prove they read something.** + `test_repo_conventions.py::test_the_repository_scanners_actually_read_files` names one file + each collector must return plus a floor on the count, for all seven collectors across + `test_repo_conventions`, `test_code_hygiene`, `test_layering` and `test_readme_guards`. A + glob or walk that comes back EMPTY satisfies every assertion built on it and looks exactly + like a working guard, so renaming `beantester/` would have silenced four guards at once in + silence. Mutation-checked: emptying a collector and pointing the walk at a missing root each + turn it red. +- **The whole-tree scans stopped measuring files that are not in the repository.** + New `repo_text_files()` collector skips `internal_tools/`, `.claude/`, `crashes/` and the + private notes, and `::test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository` + keeps them out. Measured before the change: the dash scan covered 183 files, 12 of them + git-ignored, so the same test measured a different set locally than in CI - including + `crashes/latest-crash.txt`, whose text comes from OS exceptions. Convention 33's coverage of + the private notes moves to the Stop hook, which runs where those files exist. - `tests/test_gui_release_fixes.py::test_start_only_fields_are_locked_while_a_session_runs` rewritten to DERIVE its subjects from `fields.FIELD_DEFS` instead of naming `duration` and the filter combobox by hand, and to resolve each field to the surface that renders it diff --git a/tests/test_code_shape.py b/tests/test_code_shape.py new file mode 100644 index 0000000..c292d1c --- /dev/null +++ b/tests/test_code_shape.py @@ -0,0 +1,136 @@ +"""Size ceilings for the package, as a RATCHET: the numbers may only go down. + +Why a ceiling here, when nothing has actually gone wrong yet +----------------------------------------------------------- +Nobody reads this code line by line. There is no reviewer to notice that a function +reached three hundred lines over four sessions, each of which added twenty and each +of which was reasonable on its own. A ceiling is the only mechanism that notices, +and it costs nothing while nothing grows. + +Why LOGIC lines and not lines +----------------------------- +Measured before choosing the metric (2026-08-02): of 17 770 lines in ``beantester/``, +only 9 776 are logic - **45% of this package is comments, docstrings and blank +lines**. That is deliberate and it is where the measurements and the reasons live. A +raw-line ceiling would therefore be a ceiling on EXPLAINING, pushing the next session +to delete the paragraph that says why a constant is 0.30 rather than to simplify the +function. So comments and docstrings are free; only executable lines count. + +Where the numbers come from +--------------------------- +They are today's maxima, not a textbook figure: ``init_style`` at 167 logic lines and +``gui/app.py`` at 1299. Nothing has to be rewritten to make this pass, which is the +point - a ceiling picked out of the air either fails on day one or is set so loose it +never fires. + +**Lowering these is ordinary work. Raising either is the owner's decision, not a way +to get unblocked** - a threshold bent to fit the code has stopped being a threshold. +When a function crosses it, the answer is to split the function. + +What this does NOT check +------------------------ +Whether a function does one thing (a tidy twenty-line function doing three things +passes exactly like a good one), whether its name is honest, or whether splitting it +scattered the logic across ten places - that last one has its own cost and no metric. +Nesting depth is not measured either. This guard buys one thing only: nothing grows +past what a person can follow without somebody deciding that it should. +""" +import ast +import os + +from fakes import ROOT, check + +# Today's maxima, measured 2026-08-02. Ratchet: down is routine, up is a decision. +FUNCTION_CEILING = 167 # beantester/gui/theme.py::init_style +FILE_CEILING = 1299 # beantester/gui/app.py + + +def _logic_lines(source): + """Line numbers that carry executable code: no blanks, comments or docstrings. + + Docstrings are found through the AST rather than by matching quotes, so a string + that merely LOOKS like one (a multi-line literal assigned to a name) still counts + as logic - which is right, because it is. + """ + tree = ast.parse(source) + lines = source.splitlines() + doc = set() + for node in ast.walk(tree): + if (isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant) + and isinstance(node.value.value, str)): + doc.update(range(node.lineno, (node.end_lineno or node.lineno) + 1)) + live = set() + for number in range(1, len(lines) + 1): + text = lines[number - 1].strip() + if text and not text.startswith("#") and number not in doc: + live.add(number) + return tree, live + + +def _package_files(): + out = [] + for dirpath, dirnames, filenames in os.walk(os.path.join(ROOT, "beantester")): + dirnames[:] = [d for d in dirnames if d != "__pycache__"] + out += [os.path.join(dirpath, n) for n in filenames if n.endswith(".py")] + return out + + +def _measure(): + """(worst function, worst file) as (name, count) pairs, plus how many files.""" + worst_function = ("", 0) + worst_file = ("", 0) + paths = _package_files() + for path in paths: + tree, live = _logic_lines(open(path, encoding="utf-8").read()) + rel = os.path.relpath(path, ROOT).replace(os.sep, "/") + count = len(live) + if count > worst_file[1]: + worst_file = (rel, count) + for node in ast.walk(tree): + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + span = sum(1 for n in live if node.lineno <= n <= (node.end_lineno or 0)) + if span > worst_function[1]: + worst_function = (f"{rel}::{node.name}", span) + return worst_function, worst_file, len(paths) + + +def test_no_function_or_file_has_grown_past_the_ratchet(): + """Nothing in the package is longer than the longest thing was on 2026-08-02. + + Crossing this is not a licence to raise the number. A function over the ceiling + gets split - which is exactly what happened to the one case another project hit + on the day it introduced the same guard. + """ + worst_function, worst_file, seen = _measure() + + # The canary from test_repo_conventions, applied here: a walk that finds nothing + # satisfies both ceilings perfectly and looks like a guard that works. + check("the shape scan actually read the package " + "(an empty scan passes every ceiling ever set)", seen >= 30, f"({seen} files)") + + check(f"no function exceeds {FUNCTION_CEILING} logic lines " + f"(split it - do not raise the ceiling)", + worst_function[1] <= FUNCTION_CEILING, + f"(worst: {worst_function[0]} at {worst_function[1]})") + check(f"no module exceeds {FILE_CEILING} logic lines", + worst_file[1] <= FILE_CEILING, + f"(worst: {worst_file[0]} at {worst_file[1]})") + + +def test_the_ratchet_measures_logic_and_not_explanation(): + """Comments and docstrings must stay free, or the ceiling punishes the thing + this project is built on. + + Without this, the cheapest way to get back under the limit would be to delete + the paragraph explaining why a constant is what it is - the exact opposite of + what convention 5 asks for. Checked directly rather than assumed: a body padded + with ninety lines of comment measures the same as the body alone. + """ + bare = "def f():\n" + " x = 1\n" * 5 + padded = "def f():\n" + ' """Doc."""\n' + " # note\n" * 90 + " x = 1\n" * 5 + _, live_bare = _logic_lines(bare) + _, live_padded = _logic_lines(padded) + check("ninety lines of comment and a docstring do not count as logic", + len(live_padded) == len(live_bare), + f"(bare {len(live_bare)}, padded {len(live_padded)})") diff --git a/tests/test_hot_path_allocations.py b/tests/test_hot_path_allocations.py new file mode 100644 index 0000000..c18553e --- /dev/null +++ b/tests/test_hot_path_allocations.py @@ -0,0 +1,164 @@ +"""The decision path must not RETAIN anything per packet. + +What this guards +---------------- +"Zero allocations" has been written in the hot-path section of the notes for a long +time and nothing checked it. ``test_hot_path.py`` guards the neighbouring rule - no +syscalls on the packet threads - which is a different failure. This one catches the +class where ``decide()`` starts holding on to something once per packet: a cache +added for speed, a list that only ever grows, a per-flow object nobody frees. At the +measured real rate of ~14k packets a second that is tens of megabytes a minute, and +the symptom the user sees is not "slow" but a session that dies after an hour. + +Two meters, because one of them has a hole +------------------------------------------ +🔴 **Blocks alone are not enough, and this was found by mutation, not by thinking.** +The first version of this file counted ``sys.getallocatedblocks()`` only. A mutation +that made ``decide()`` append to an ever-growing list **SURVIVED it**: the appended +value was a small cached int, so no object was created and the list's array growth +cost about one block. Blocks see a retained NEW OBJECT; they are blind to a container +filling up with references to objects that already exist - which is the shape most +real leaks in this codebase would take. + +So the gate reads both, and each covers the other's blind spot: + +* ``sys.getallocatedblocks()`` - net objects retained. Measured for the mutation: + 7 against a baseline of 6, i.e. invisible. +* ``tracemalloc`` current (not peak) - bytes still held. Same mutation: **42 032 + against 208**, i.e. unmissable. + +What it still does NOT catch, said out loud +------------------------------------------- +**Transient garbage.** Both meters are net, so an object allocated and freed inside +the same call nets to nothing - measured, not assumed. A future ``f"{a}:{b}"`` built +per packet would pass here. CPython has no cheap deterministic counter of total +allocations (Go's ``AllocsPerRun`` has no equivalent), and pretending otherwise would +be the guard that promises more than it measures. Churn stays a job for measurement +(rule 5), not for this gate. + +Why the numbers are ceilings and not zero +----------------------------------------- +Measured across configurations (2026-08-02, three runs each, gc collected then +disabled, values identical every time): the pass-through path costs **13 blocks and +608 bytes per 5000 calls** and does not move - not with 200 ports instead of 50, not +with the NAT flow table armed, not with latency. Duplication takes it to 15 blocks +and 656 bytes. Those are interpreter bookkeeping, not per-packet retention, so the +ceilings sit far above them and far below a real regression: one retained reference +per packet is 42 kB, one retained object is +5000 blocks. +""" +import gc +import random +import sys +import tracemalloc + +from fakes import check + +from beantester.core import BeanCore + +CALLS = 5000 +BLOCK_CEILING = 64 # measured floor 13, flat across configurations +BYTE_CEILING = 4096 # measured floor 608; a one-reference-per-packet leak is 42k + + +def _decide_many(core, rng, count): + for i in range(count): + core.decide(100, True, 5000 + (i % 50), i * 0.001, rng, + remote_ip="1.2.3.4", remote_port=443, is_tcp=True) + + +def _cost_of(core, count=CALLS): + """(net blocks, net bytes) retained by ``count`` decisions, warmed and gc-quiet.""" + rng = random.Random(7) + _decide_many(core, rng, 200) # fill every lazy structure first + gc.collect() + gc.disable() + tracemalloc.start() + try: + blocks_before = sys.getallocatedblocks() + bytes_before = tracemalloc.get_traced_memory()[0] + _decide_many(core, rng, count) + return (sys.getallocatedblocks() - blocks_before, + tracemalloc.get_traced_memory()[0] - bytes_before) + finally: + tracemalloc.stop() + gc.enable() + + +def test_the_meter_can_actually_see_retention(): + """Prove the instrument before believing a zero from it. + + A gate whose measurement is stuck at zero passes forever and reads exactly like + a clean hot path. This is the same reason the mutation runner carries a canary: + an instrument that cannot report failure is not evidence. Retaining 5000 objects + must show up as thousands of blocks. + """ + if not hasattr(sys, "getallocatedblocks"): # non-CPython: say so, loudly + raise AssertionError("this interpreter has no getallocatedblocks; the hot " + "path allocation gate cannot run and must not be " + "reported as passing") + gc.collect() + gc.disable() + tracemalloc.start() + try: + blocks_before = sys.getallocatedblocks() + bytes_before = tracemalloc.get_traced_memory()[0] + kept = [object() for _ in range(5000)] + blocks = sys.getallocatedblocks() - blocks_before + # The case that defeated the block meter: a container filling with + # references to an object that ALREADY exists. No object is created, so + # blocks barely move - only the bytes do. + references = [] + bytes_before_refs = tracemalloc.get_traced_memory()[0] + blocks_before_refs = sys.getallocatedblocks() + for _ in range(5000): + references.append(100) # a cached small int: nothing new is made + ref_blocks = sys.getallocatedblocks() - blocks_before_refs + ref_bytes = tracemalloc.get_traced_memory()[0] - bytes_before_refs + retained = tracemalloc.get_traced_memory()[0] - bytes_before + finally: + tracemalloc.stop() + gc.enable() + + check("the block meter reports thousands when 5000 objects are retained " + "(a meter stuck at zero would pass the gate below forever)", + blocks >= 4000, f"(saw {blocks} for {len(kept)} objects)") + check("the byte meter reports thousands when 5000 objects are retained", + retained >= 4000, f"(saw {retained} bytes)") + check("the byte meter catches a container of REFERENCES, which the block meter " + "cannot see - this is the hole a surviving mutant exposed on 2026-08-02", + ref_bytes >= 4000 and ref_blocks < 100, + f"(refs: {ref_blocks} blocks, {ref_bytes} bytes)") + + +def test_the_decision_path_retains_nothing_per_packet(): + """A default engine judging 5000 packets holds on to nothing. + + Pass-through is the configuration the tool spends most of its life in and the + one "collect, do not damage" promises is free (see test_passthrough.py). + """ + blocks, retained = _cost_of(BeanCore()) + check(f"decide() retains at most {BLOCK_CEILING} blocks over {CALLS} packets " + f"(one retained object per packet would be {CALLS})", + blocks <= BLOCK_CEILING, f"(retained {blocks} blocks)") + check(f"decide() retains at most {BYTE_CEILING} bytes over {CALLS} packets " + f"(a container growing by one reference per packet is about 42 kB, and " + f"the block count above cannot see it)", + retained <= BYTE_CEILING, f"(retained {retained} bytes)") + + +def test_the_armed_gates_do_not_retain_per_packet_either(): + """Impairment on is still not a licence to keep a copy of every packet. + + Duplication is the one gate that legitimately hands a second packet onward, so + it is the honest worst case to point this at. + """ + for label, arm in (("duplication", lambda c: setattr(c, "dup", 1.0)), + ("latency", lambda c: setattr(c, "latency_s", 0.05)), + ("nat flow table", lambda c: setattr(c, "nat_timeout_s", 30))): + core = BeanCore() + arm(core) + blocks, retained = _cost_of(core) + check(f"with {label} armed, decide() retains at most {BLOCK_CEILING} blocks", + blocks <= BLOCK_CEILING, f"(retained {blocks} blocks)") + check(f"with {label} armed, decide() retains at most {BYTE_CEILING} bytes", + retained <= BYTE_CEILING, f"(retained {retained} bytes)") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py new file mode 100644 index 0000000..d9babad --- /dev/null +++ b/tests/test_mutation_registry.py @@ -0,0 +1,208 @@ +"""The registry of mutation proofs, and the guard that keeps it honest. + +Why this file exists +-------------------- +This project's strongest claim about its own tests is the sentence "verified by +mutation". It appears in PROJECT_NOTES more than twenty times and in a dozen +docstrings - and until now **nothing checked a single one of them**. That is the +exact failure mode convention 5 exists to prevent, applied to the evidence for +convention 5 itself: prose nobody guards, trusted precisely because it sounds +rigorous. + +So the claims move here, as data, in three lists that say three different things: + +* ``MUTATIONS`` - re-runnable today. ``internal_tools/mutate.py`` breaks the named + behaviour and proves the named test reddens. This is the only list that is proof. +* ``PROVEN_BY_HAND`` - a mutation WAS performed and dated, by a session, with no + re-runnable entry. The claim rests on that record, not on anything a machine can + repeat. This list should only shrink: entries move to ``MUTATIONS`` when someone + writes the patch down. +* ``NOT_PROVEN`` - no mutation, said out loud. An empty-looking guard and an + unproven one must not be indistinguishable, which is what happens when the third + list is missing. + +What the SUITE checks here (cheap, every run) is the bookkeeping: that every named +test exists, that no test is filed under two states, and - the one that matters - +that **every mutation's search pattern still occurs exactly once**. A pattern that +went stale would make the runner report SKIP, but only when someone runs it; the +suite catches it the day the code moves. Running the mutations themselves is not a +pytest job: each one costs a subprocess suite run. + +Deliberately NOT checked here: whether a mutation is *aimed well*. A patch can +redden its test for the wrong reason - see the note in convention 5 about a test +that passed because a transposition broke a different field than the one it named. +""" +import ast +import glob +import os + +from fakes import ROOT, check + + +# -- the three states ---------------------------------------------------------- # +# label -> what to break -> which single test must go red. Keep `old` long enough to +# be unambiguous and short enough to survive unrelated edits nearby. +MUTATIONS = [ + { + "label": "gui: the settings form stops refreshing its field states", + "file": "beantester/gui/panels/settings.py", + "old": " self.form.refresh_field_states()\n", + "new": "", + "test": "test_start_only_fields_are_locked_while_a_session_runs", + }, + { + "label": "gui: start/stop stops ticking the open windows", + "file": "beantester/gui/app.py", + "old": " with crashlog.quiet(\"gui.app\"):\n self.windows.refresh()", + "new": " with crashlog.quiet(\"gui.app\"):\n pass", + "test": "test_start_only_fields_are_locked_while_a_session_runs", + }, + { + "label": "guards: the repository collector returns nothing", + "file": "tests/test_repo_conventions.py", + "old": " out = []\n for dirpath, dirnames, filenames in os.walk(ROOT):", + "new": " out = []\n for dirpath, dirnames, filenames in []:", + "test": "test_the_repository_scanners_actually_read_files", + }, + { + "label": "guards: the whole-tree walk points at a root that is not there", + "file": "tests/test_repo_conventions.py", + "old": "for dirpath, dirnames, filenames in os.walk(ROOT):", + "new": "for dirpath, dirnames, filenames in os.walk(ROOT + '_nope'):", + "test": "test_the_repository_scanners_actually_read_files", + }, + { + "label": "guards: internal_tools falls back into the scanned set", + "file": "tests/test_repo_conventions.py", + "old": "\"internal_tools\", \".claude\", \"crashes\"}", + "new": "\".claude\", \"crashes\"}", + "test": "test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository", + }, + { + "label": "shape: the package walk finds no files to measure", + "file": "tests/test_code_shape.py", + "old": " out += [os.path.join(dirpath, n) for n in filenames if n.endswith(\".py\")]", + "new": " out += []", + "test": "test_no_function_or_file_has_grown_past_the_ratchet", + }, + { + "label": "shape: comments start counting as logic", + "file": "tests/test_code_shape.py", + "old": " if text and not text.startswith(\"#\") and number not in doc:", + "new": " if text:", + "test": "test_the_ratchet_measures_logic_and_not_explanation", + }, + { + "label": "hot path: decide() starts keeping one object per packet", + "file": "beantester/core.py", + "old": " with self._lock:\n # 1) process targeting", + "new": (" with self._lock:\n" + " self.__dict__.setdefault(\"_leak\", []).append(size)\n" + " # 1) process targeting"), + "test": "test_the_decision_path_retains_nothing_per_packet", + }, + { + "label": "hot path: the allocation meter stops seeing retention", + "file": "tests/test_hot_path_allocations.py", + "old": " kept = [object() for _ in range(5000)]", + "new": " kept = [object() for _ in range(0)]", + "test": "test_the_meter_can_actually_see_retention", + }, +] + +# The runner's own check: a patch that cannot compile must be reported as BROKEN, not +# as "caught". Without it, a tree that fails to build looks exactly like a mutation +# the suite detected, and every other line of the report becomes worthless. +CANARY = { + "label": "CANARY: deliberately unparsable, must report BROKEN", + "file": "beantester/utils.py", + "old": "def clamp01(", + "new": "def ((( clamp01(", + "test": "test_no_old_name_references", +} + +# A mutation was run and dated by a session, but nobody wrote the patch down, so no +# machine can repeat it. This is weaker than MUTATIONS and stronger than nothing - +# and it is the honest state of most "verified by mutation" lines in the notes. +PROVEN_BY_HAND = { + "test_shortcut_buttons_advertise_their_key": "2026-07-21, dropping shortcut= from Save/Load", + "test_an_overridden_field_is_visibly_disabled": "2026-07-21, removing the disabled style maps", + "test_no_stale_pending_markers": "2026-07-25, both directions", + "test_every_remote_endpoint_gate_fires_in_both_directions": "2026-07, the inbound branch", + "test_a_worker_thread_exception_is_recorded": "2026-08-01, the excepthook body", + "test_pid_for_takes_no_lock_because_the_capture_thread_calls_it": "2026-07-29, taking the lock", +} + +# No mutation at all. Naming them is the point: an unproven guard and a guard nobody +# looked at must not read the same. This list is allowed to grow only when a guard +# is added without its proof - and every entry is a debt. +NOT_PROVEN = { + "test_a_resize_after_the_label_is_gone_is_not_a_crash": "never mutated", + "test_the_ui_rebuild_does_not_pile_up_configure_handlers_on_the_root": "never mutated", + "test_an_injected_rst_is_always_recomputed": "never mutated", + "test_evicting_the_connection_log_can_never_empty_it": "never mutated", +} + + +def _known_test_names(): + names = set() + for path in glob.glob(os.path.join(ROOT, "tests", "test_*.py")): + tree = ast.parse(open(path, encoding="utf-8").read(), filename=path) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + if node.name.startswith("test_"): + names.add(node.name) + return names + + +def test_every_mutation_still_points_at_code_that_exists(): + """The registry rots the moment the code it patches moves. + + A stale pattern makes the runner print SKIP - but only for whoever runs it, and + the entry keeps LOOKING like a proof in the meantime. Checking the occurrence + count here means the suite says it the day the code changes, which is the whole + difference between a registry and a list of good intentions. + """ + for entry in MUTATIONS + [CANARY]: + path = os.path.join(ROOT, entry["file"]) + check(f"{entry['label']}: {entry['file']} exists", os.path.exists(path)) + text = open(path, encoding="utf-8").read() + found = text.count(entry["old"]) + check(f"{entry['label']}: its search pattern occurs exactly once " + f"(a stale pattern proves nothing and reports SKIP)", + found == 1, f"(found {found} times)") + + +def test_every_named_test_exists_and_has_exactly_one_state(): + """A guard is proven, hand-proven or unproven - never two of those, never none. + + The state being VISIBLE is the point. Two rows of another project's regression + table said "verified by mutation" with no entry behind them, and the only reason + anyone found out was a test exactly like this one. + """ + known = _known_test_names() + states = {} + for entry in MUTATIONS: + states.setdefault(entry["test"], set()).add("MUTATIONS") + for name in PROVEN_BY_HAND: + states.setdefault(name, set()).add("PROVEN_BY_HAND") + for name in NOT_PROVEN: + states.setdefault(name, set()).add("NOT_PROVEN") + + for name, where in sorted(states.items()): + check(f"{name} is a real test (a registry naming a ghost is worse than " + f"an empty registry)", name in known, f"(listed in {sorted(where)})") + check(f"{name} is filed under exactly one state", len(where) == 1, + f"(in {sorted(where)})") + + +def test_the_canary_is_not_quietly_disarmed(): + """The canary must name a real test and a real file, or the runner cannot fail. + + A runner whose canary silently stops firing reports "everything caught" for a + run that proved nothing - which is worse than not running it, because it is + quotable. + """ + check("the canary names a test that exists", + CANARY["test"] in _known_test_names(), f"({CANARY['test']})") + check("the canary would really break the parse", "(((" in CANARY["new"]) diff --git a/tests/test_repo_conventions.py b/tests/test_repo_conventions.py index bcdfd65..caffeff 100644 --- a/tests/test_repo_conventions.py +++ b/tests/test_repo_conventions.py @@ -8,6 +8,38 @@ from fakes import ROOT, check +# Directories a whole-repository walk must not descend into. The first group is +# generated or vendored; the second is the maintainer's own, kept OUT of git. +# +# The second group matters more than it looks. `internal_tools/`, `.claude/`, +# `crashes/` and the private notes exist on the owner's machine and in NO public +# checkout, so a guard that scans them measures a different set here than in CI: +# an em dash typed into PROJECT_NOTES.md would redden the suite locally while CI +# stayed green, which teaches that a red guard is local noise. Measured 2026-08-02: +# the dash scan covered 183 files, 12 of them git-ignored - including +# `crashes/latest-crash.txt`, whose contents are arbitrary text from OS exceptions. +# The notes keep their own dash check in `.claude/hooks/check_notes.py`, which runs +# exactly where they exist. +SKIP_DIRS = {".git", "__pycache__", ".pytest_cache", "licenses", "build", "dist", + ".hypothesis", "internal_tools", ".claude", "crashes"} +SKIP_FILES = {"PROJECT_NOTES.md", "HISTORY_NOTES.md", "CLAUDE.md"} + + +def repo_text_files(exts): + """Every text file that is actually IN the repository, with the given suffixes. + + Returns a list so a caller can assert it is not empty: a scanner whose walk + yields nothing passes every check it makes, and looks exactly like a scanner + that works. See test_the_repository_scanners_actually_read_files. + """ + out = [] + for dirpath, dirnames, filenames in os.walk(ROOT): + dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS] + for name in filenames: + if name.endswith(exts) and name not in SKIP_FILES: + out.append(os.path.join(dirpath, name)) + return out + def _source_files(): files = [os.path.join(ROOT, "bean_network_tester.py"), @@ -121,18 +153,11 @@ def test_no_em_or_en_dashes_in_repo_text(): exts = (".py", ".md", ".json", ".toml", ".spec", ".yml", ".yaml", ".txt", ".cfg", ".ini") offenders = [] - for dirpath, dirnames, filenames in os.walk(ROOT): - dirnames[:] = [d for d in dirnames - if d not in (".git", "__pycache__", ".pytest_cache", - "licenses", "build", "dist", ".hypothesis")] - for name in filenames: - if not name.endswith(exts): - continue - path = os.path.join(dirpath, name) - text = open(path, encoding="utf-8", errors="replace").read() - for ch, label in banned.items(): - if ch in text: - offenders.append(f"{os.path.relpath(path, ROOT)}: {label}") + for path in repo_text_files(exts): + text = open(path, encoding="utf-8", errors="replace").read() + for ch, label in banned.items(): + if ch in text: + offenders.append(f"{os.path.relpath(path, ROOT)}: {label}") check("no em/en dashes outside licenses/ (use '-')", not offenders, f"({offenders[:8]}{'...' if len(offenders) > 8 else ''})") @@ -180,28 +205,22 @@ def test_no_stale_pending_markers(): marker = re.compile(r"PENDING\(([a-z0-9][a-z0-9-]*)\)") seen = {} - for dirpath, dirnames, filenames in os.walk(ROOT): - dirnames[:] = [d for d in dirnames - if d not in (".git", "__pycache__", ".pytest_cache", - "licenses", "build", "dist", ".hypothesis")] - for name in filenames: - if not name.endswith(PENDING_EXTS): - continue - if name == os.path.basename(__file__): - continue # the registry does not scan itself - if name.startswith("CHANGELOG"): - # A changelog records what HAPPENED and is dated by its nature: an - # entry saying "added a marker named X" stays TRUE after X closes, so - # it is history, not drift. Found the hard way - the first real - # closing (socket-event-fields) tripped over the very entry that - # announced the marker. - continue - path = os.path.join(dirpath, name) - text = open(path, encoding="utf-8", errors="replace").read() - for lineno, line in enumerate(text.splitlines(), 1): - for found in marker.findall(line): - seen.setdefault(found, []).append( - f"{os.path.relpath(path, ROOT)}:{lineno}") + for path in repo_text_files(PENDING_EXTS): + name = os.path.basename(path) + if name == os.path.basename(__file__): + continue # the registry does not scan itself + if name.startswith("CHANGELOG"): + # A changelog records what HAPPENED and is dated by its nature: an + # entry saying "added a marker named X" stays TRUE after X closes, so + # it is history, not drift. Found the hard way - the first real + # closing (socket-event-fields) tripped over the very entry that + # announced the marker. + continue + text = open(path, encoding="utf-8", errors="replace").read() + for lineno, line in enumerate(text.splitlines(), 1): + for found in marker.findall(line): + seen.setdefault(found, []).append( + f"{os.path.relpath(path, ROOT)}:{lineno}") stale = sorted(f"{key} ({', '.join(where)})" for key, where in seen.items() if key not in OPEN_PENDING) @@ -213,3 +232,67 @@ def test_no_stale_pending_markers(): check("every id in OPEN_PENDING is still referenced by a marker " "(an id nothing points at is a leftover entry, not an open stage)", not unmarked, f"({unmarked})") + + +# -- the canary: a scanner that reads nothing passes everything ----------------- # +def test_the_repository_scanners_actually_read_files(): + """Every whole-tree scanner proves it saw a file it must have seen. + + A guard built on a glob or a walk has a failure mode that looks identical to + success: the collection comes back EMPTY and every assertion over it holds + vacuously. Nothing here would notice - `not offenders` is true when there are + no files, a rename of `beantester/` would silence four separate guards at once, + and the suite would stay green while guarding nothing. + + This is the cheapest possible answer: name one file each collector MUST return, + plus a floor on the count. Both halves matter - the anchor catches a collector + pointed at the wrong root, the floor catches one that quietly narrowed. + + Mutation-checked 2026-08-02: emptying any collector, and pointing the walk at a + non-existent root, each turn this red. + """ + import test_code_hygiene + import test_layering + import test_readme_guards + + def names(paths): + return {os.path.basename(p) for p in paths} + + for label, paths, anchor, floor in ( + ("_source_files", _source_files(), "core.py", 20), + ("_gui_files", _gui_files(), "app.py", 10), + ("repo_text_files(.py)", repo_text_files((".py",)), "engine.py", 60), + ("repo_text_files(.md)", repo_text_files((".md",)), "README.md", 4), + ("code_hygiene._pkg_files", test_code_hygiene._pkg_files(), "core.py", 20), + ("layering._top_level_modules", + test_layering._top_level_modules(), "engine.py", 15), + ("readme_guards._top_level_modules", + test_readme_guards._top_level_modules(), "engine.py", 15), + ): + check(f"{label} returned files at all (an empty scan passes every check " + f"it makes and looks exactly like a working guard)", paths, "(empty)") + check(f"{label} still reaches {anchor}", anchor in names(paths), + f"({sorted(names(paths))[:6]}...)") + check(f"{label} did not quietly narrow (expected at least {floor})", + len(paths) >= floor, f"(got {len(paths)})") + + +def test_the_repository_scanners_stay_out_of_what_is_not_in_the_repository(): + """The dash and PENDING scans must measure the REPOSITORY, not this machine. + + `internal_tools/`, `.claude/`, `crashes/` and the private notes live here and in + no public checkout, so scanning them makes the guard mean one thing locally and + another in CI - and a red that CI cannot reproduce teaches that red is noise. + Measured before the fix (2026-08-02): 12 of the 183 files scanned were + git-ignored, `crashes/latest-crash.txt` among them, whose text comes from OS + exceptions and is nobody's convention to keep. + """ + scanned = {os.path.relpath(p, ROOT).replace(os.sep, "/") + for p in repo_text_files((".py", ".md", ".json", ".txt"))} + for stray in ("PROJECT_NOTES.md", "CLAUDE.md", "HISTORY_NOTES.md"): + check(f"{stray} is not scanned (it is not in the repository)", + stray not in scanned, f"({stray})") + for prefix in ("internal_tools/", ".claude/", "crashes/"): + leaked = sorted(p for p in scanned if p.startswith(prefix)) + check(f"nothing under {prefix} is scanned (git-ignored, absent in CI)", + not leaked, f"({leaked[:4]})")