From f239e98c2924be1a63cf9d7e0213cc82e7de5e5d Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Mon, 14 Sep 2026 03:26:16 +0000 Subject: [PATCH] test: read a suite's own recorders, and refuse the one it cannot (#1053) #1051 taught the extractor the recorders `lib.sh` shares. A suite may also define its own, and there are two shapes, only one of which is a gap: COMPOSE check "non-owner refused: ${1%%(*}" the definition states a TEMPLATE naming the property FORWARD check_text "$label" "$got" "$want" the definition states nothing; the NAME is at the call sites A composing wrapper is already read correctly: `non-owner refused: {}` covers all nine of `native_ownership`'s call sites, which is why that pair grades one-for-one today. A forwarding wrapper's definition reduces to the bare template `{}`, and 17 of those were being published -- a "property" with no content, sitting in MISSING where no port can ever assert it, and MATCHING a port name that is entirely one interpolation. A wrong name is worse than an absent one, which is the argument #1051 turned on. The grader now derives each suite's own recorders by the rule that already works for `lib.sh` -- a function forwarding a bare positional into a known recorder's name slot, transitively, seeded from `pgc_record` -- reads the call sites of the forwarding ones, drops the bare `{}`, and leaves composers alone. names the reader gains 145 across 14 suites every graded pair unchanged refused corpus-wide 1 of 264, and it has no twin `sorted_pathkeys` gains 18, and they are not a random 18: that suite pairs every "plans no Sort" with an "and still answers correctly", so the grader could see every claim about the PLAN and none about the ANSWER. AND IT REFUSES WHAT IT CANNOT READ. `hilbert_curve.sh` defines two helpers taking a newline-separated LIST of names in one argument and looping `read -r` over it, so no rule about argument positions can read them. `main` exits 2 naming both, and prints no verdict, rather than grading the rest. MEASURED BEFORE BUILDING, AND IT CHANGED THE DESIGN. Refusing on "the name position is not a bare positional" also refuses every COMPOSING wrapper: 32 suites, including `hilbert_cluster`, `hilbert_locality` and `native_ownership` -- three pairs that are COMPLETE -- to fix nothing. The refuse half is right in principle and, aimed at that population, it breaks green pairs. Removal proof, `__pycache__` cleared before every arm and the RUNTIME asserted: CONTROL rc=0, 0 failed (110 names, ans+ansp) M1 do not read the call sites rc=1, 3 failed runtime 110 -> 92 M2 composers treated as unreadable rc=1, 3 failed M3 do not refuse, just skip rc=1, 1 failed M4 publish the bare {} rc=1, 2 failed runtime 110 -> 113 M5 seed without the primitive rc=1, 1 failed M3 AND M5 REDDENED NOTHING AT FIRST. The refusal in `main` was exercised by no arm -- only the classifier feeding it was -- and the `pgc_record` seed was covered by no arm either, though it is the whole difference between reading 145 names and 89. Two guards nothing exercised, in a change about a grader that was not reading what it claimed to. Both now have an arm and both reddens are isolated to it. The refused SET is pinned by name rather than counted. `checks_never_observed_red` is this repo's worked example of the other shape: a census every legitimate addition broke, so the only way to land one was to raise a number the design said may only fall. A count tells a reviewer something moved; a set tells them what. guard leg 329 passed, 830 checks, 0 fail cluster leg 320 passed, 885 checks, 0 fail (pg17a) `guard_tests` 322 -> 329 by collection. #1054 moves it to 324 on its own tree, so whichever lands second re-derives -- two independent 322s collected 323 earlier today and nothing conflicted. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs --- CHANGELOG.md | 36 +++ test/pytest/TESTS.md | 7 + test/pytest/compare_to_bash.py | 222 ++++++++++++++++++- test/pytest/expected_tests.txt | 10 +- test/pytest/test_compare_to_bash.py | 325 ++++++++++++++++++---------- 5 files changed, 488 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ddcd002..2c40cb48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,42 @@ true until the next version shipped. ### Added +- `compare_to_bash.py` could not read a name a suite passed through its OWN wrapper, + and published a bare `{}` in its place (#1053). + + #1051 taught the extractor the recorders `lib.sh` shares. A suite may also define + its own, and there are two shapes, only one of which is a gap: + + COMPOSE check "non-owner refused: ${1%%(*}" the definition states a + TEMPLATE naming the property + FORWARD check_text "$label" "$got" "$want" the definition states nothing + + A composing wrapper is already read correctly -- `non-owner refused: {}` covers all + nine of `native_ownership`'s call sites, which is why that pair grades one-for-one. + A forwarding wrapper's definition yields the bare template `{}`, and 17 of those + were being published: a "property" with no content, sitting in MISSING where no port + can ever assert it, and MATCHING a port name that is entirely one interpolation. + + The grader now derives each suite's own recorders by the rule that already works for + `lib.sh` -- a function forwarding a bare positional into a known recorder's name + slot, transitively, seeded from `pgc_record` -- reads the call sites of the + forwarding ones, drops the bare `{}`, and leaves composers alone. 145 names across + 14 suites become readable. `sorted_pathkeys` alone gains 18, and they are not a + random 18: that suite pairs every "plans no Sort" with an "and still answers + correctly", so the grader could see every claim about the PLAN and none about the + ANSWER. + + AND IT REFUSES what it cannot read. `hilbert_curve.sh` defines two helpers taking a + newline-separated LIST of names in one argument, so no rule about argument positions + can read them; the grader now exits 2 naming both rather than grading the rest. One + suite of 264, a true positive, with no pytest twin. + + MEASURED BEFORE BUILDING, and it changed the design: refusing on "the name position + is not a bare positional" also refuses every COMPOSING wrapper -- 32 suites, + including `hilbert_cluster`, `hilbert_locality` and `native_ownership`, three pairs + that are COMPLETE today -- to fix nothing. Every graded pair is unchanged by what + shipped. + - `iceberg_fdw.sh` is ported to pytest: the FDW's partition and metrics pruning (#388, #432). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 881d23f6..ab4a6f50 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -3781,6 +3781,13 @@ the tool grades THIS tree. | `test_the_derivation_finds_a_wrapper_planted_in_a_fixture` | the derivation on a fixture where the answer is known: four forwarding shapes found, and a function owning its own literal name rejected | | `test_the_comment_stripper_keeps_a_parameter_expansion` | `${shape#*|}` and `$#` are not comments; `#` opens one only at a word boundary | | `test_an_empty_helper_group_fabricates_names_rather_than_reading_none` | an empty alternation matches everywhere, so a position with no helper would invent `$PGC_DB` as a check name rather than read none | +| `test_a_suites_own_forwarding_wrapper_is_read` | a suite's own wrapper that forwards a bare positional has its names read from the CALL SITES | +| `test_a_composing_wrapper_is_left_alone` | a wrapper that COMPOSES its name already states a template; refusing it would break three COMPLETE pairs | +| `test_a_helper_whose_name_cannot_be_resolved_is_refused` | a helper taking a LIST of names in one argument is refused by name, not skipped | +| `test_the_refusal_names_exactly_the_suites_it_refuses` | the refused SET is pinned by name, not counted; both directions, so an entry cannot outlive its cause | +| `test_a_bare_interpolation_is_not_published_as_a_name` | a forwarding wrapper's `{}` is dropped: it names nothing and can match a wholly-interpolated port name | +| `test_the_grader_itself_refuses_the_suite_it_cannot_read` | `main` exits 2 and prints no verdict, with a readable suite as the control | +| `test_a_helper_reaching_only_the_primitive_is_found` | the closure is seeded from `pgc_record`, not the `check` family; four suites turn on it | | `test_a_longer_helper_name_is_not_shadowed_by_a_shorter_one` | `check_ratio` must not eat `check_ratio_needs_quiet_machine` | | `test_the_suite_local_helpers_are_known_and_excluded` | the four suite-local helpers, and that none of their suites is graded | | `test_every_pair_in_the_tree_is_declared` | the declaration is asserted BOTH ways, so a new pair cannot be silently ungraded | diff --git a/test/pytest/compare_to_bash.py b/test/pytest/compare_to_bash.py index 39b51304..691bbf40 100755 --- a/test/pytest/compare_to_bash.py +++ b/test/pytest/compare_to_bash.py @@ -250,6 +250,12 @@ def _as_names(node): # Hand-written so the tool stays standalone, and pinned like `_NAME_ARG`: the drift # guard in `test_compare_to_bash.py` DERIVES this table from `lib.sh` -- membership # and position both -- and fails with the helper named when the two disagree. +# The primitive every recorder reaches, and the argument IT names its check in. The +# seed of the closure below; named rather than inlined so an arm can assert that no +# suite calls it directly. +_RECORD_PRIMITIVE = "pgc_record" +_RECORD_NAME_ARG = 2 + _BASH_NAME_ARG = { "check_ratio_needs_quiet_machine": 1, "check_unrunnable": 1, @@ -271,6 +277,115 @@ def _as_names(node): _BASH_HELPERS = tuple(_BASH_NAME_ARG) +def _strip_comments(text): + r"""-> the text with shell comments removed, and NOTHING else removed. + + `#` starts a comment only at a word boundary. `${shape#*|}` and `$#` are not + comments, and cutting at the first `#` truncates the line to something that + parses as a different program. That exact slip has produced two wrong counts in + this repo, so the fixtures for it are in the arm below rather than in a comment. + """ + out = [] + for line in text.splitlines(): + res, i, quote = [], 0, None + while i < len(line): + ch = line[i] + if quote: + if ch == quote: + quote = None + res.append(ch) + elif ch in "\"'": + quote = ch + res.append(ch) + elif ch == "#" and (i == 0 or line[i - 1] in " \t;&|()"): + break + else: + res.append(ch) + i += 1 + out.append("".join(res)) + return "\n".join(out) + + +def _bodies(text): + """-> [(function name, body)] with each body ended by ITS OWN closing brace. + + Per-line brace depth, not `find("\n}")`: 199 definitions in this tree are written + on one line (`q() { psql ...; }`), and a scan for a brace in the first column + swallows every following definition into the first one's body. + """ + out, lines = [], text.splitlines() + for i, line in enumerate(lines): + m = re.match(r"[ \t]*([A-Za-z_][A-Za-z0-9_]*)[ \t]*\(\)[ \t]*\{", line) + if not m: + continue + depth = line.count("{") - line.count("}") + body, j = [line[m.end():]], i + 1 + while j < len(lines) and depth > 0: + depth += lines[j].count("{") - lines[j].count("}") + body.append(lines[j]) + j += 1 + out.append((m.group(1), "\n".join(body))) + return out + + +def _words(text): + """-> the shell words of a call's argument list, quotes kept.""" + return re.findall(r'"[^"]*"|\S+', text) + + +def _derive_recorders(lib, seed=("pgc_record", 2)): + """-> {helper: which argument holds the check name}, derived from what lib.sh DOES. + + THE POPULATION IS THE POINT (#1045). The #1040 guard derived its population by + SPELLING -- every `lib.sh` function whose name begins `check`. It was green for + weeks while `diff_query` went unread, and correctly so: `diff_query` was never in + its population. The guard was not broken; the definition of the thing it guards + was. 225 names across 59 suites were outside it. + + So: start from `pgc_record`, the primitive that actually records, and take the + closure. A function is a recorder at position N when it passes its own `$N` -- + directly, or renamed once through a `local` -- into the name slot of a helper + already known to be one. `diff_query` calls `check`, which calls `pgc_record`. + One level of indirection was the entire gap. + + The seed is not returned. It is `lib.sh`'s own primitive and no suite calls it, + which the arm below asserts rather than assumes: the day a suite calls it, the + extractor has to learn it and this stops being true quietly. + """ + lib = _strip_comments(lib) + defs = _bodies(lib) + known = {seed[0]: seed[1]} + + changed = True + while changed: + changed = False + for fn, body in defs: + if fn in known: + continue + aliases = {m.group(1): int(m.group(2)) for m in + re.finditer(r'\b([A-Za-z_][A-Za-z0-9_]*)="\$\{?(\d+)\}?"', body)} + for rec, pos in sorted(known.items()): + for m in re.finditer(r'\b' + rec + r'([ \t]+.*)$', body, re.M): + args = _words(m.group(1)) + if len(args) < pos: + continue + slot = args[pos - 1] + inner = re.fullmatch(r'"\$\{?([A-Za-z_0-9]+)\}?"', slot) + if not inner: + continue # a literal, or something not a bare $x + tok = inner.group(1) + n = int(tok) if tok.isdigit() else aliases.get(tok) + if n is None: + continue + known[fn] = n + changed = True + break + if fn in known: + break + + del known[seed[0]] + return known + def _template(name): """-> the name with every interpolation reduced to `{}`. @@ -341,9 +456,114 @@ def _bash_names(text): return out +def _suite_recorders(text): + """-> ({helper: which argument holds the name}, [helpers whose name is unreadable]). + + A SUITE'S OWN RECORDERS, derived from its own definitions by the rule that already + works for `lib.sh`: seed from the shared table, and any function forwarding a bare + positional into a known recorder's name slot is itself a recorder (#1053). + + TWO SHAPES, AND ONLY ONE IS A GAP. The distinction is the whole of this function: + + COMPOSE check "non-owner refused: ${1%%(*}" the definition states a + TEMPLATE naming the property, + and it covers every call site + FORWARD check_text "$label" ... the definition states nothing; + the NAME is at the call sites + + A composing wrapper is already read, correctly, out of the suite file -- which is + why `native_ownership` grades one-for-one today. Treating it as unreadable and + refusing it would have broken three COMPLETE pairs to fix nothing; measured, at 32 + suites refused including `hilbert_cluster`, `hilbert_locality` and + `native_ownership`. So only FORWARDING wrappers are returned here, and the call + sites are where their names are read. + + The unreadable list is the refuse half: a helper that reaches a recorder with a + name slot this cannot resolve at all. Skipping it silently is how 147 names in 14 + suites came to be ungraded. + """ + body_text = _strip_comments(text) + known = dict(_BASH_NAME_ARG) + known[_RECORD_PRIMITIVE] = _RECORD_NAME_ARG + forwarding, unreadable = {}, [] + + changed = True + while changed: + changed = False + for fn, body in _bodies(body_text): + if fn in known or fn in unreadable: + continue + aliases = {m.group(1): int(m.group(2)) for m in + re.finditer(r'\b([A-Za-z_][A-Za-z0-9_]*)="\$\{?(\d+)\}?"', body)} + for rec, pos in sorted(known.items()): + m = re.search(r"\b" + rec + r"([ \t]+.*)$", body, re.M) + if not m: + continue + args = _words(m.group(1)) + if len(args) < pos: + continue + slot = args[pos - 1] + bare = re.fullmatch(r'"\$\{?([A-Za-z_0-9]+)\}?"', slot) + if bare: + token = bare.group(1) + n = int(token) if token.isdigit() else aliases.get(token) + if n is None: + # Reaches a recorder, and which argument carries the name + # cannot be decided. REFUSE rather than skip. + unreadable.append(fn) + else: + known[fn] = n + forwarding[fn] = n + changed = True + break + # A literal or a composed name: the definition states the property and + # `_bash_names` already reads it. Not a forwarder, not a refusal. + break + return forwarding, sorted(unreadable) + + +def _names_in(text): + """-> every check name the suite states, including through its OWN wrappers. + + A BARE `{}` IS DROPPED. A forwarding wrapper's definition reads as `"$label"`, + which reduces to the template `{}` -- a property with no content. Published, it + sits in MISSING naming nothing a port could assert, and it MATCHES a port name + that is entirely one interpolation, which is a spurious pass. 17 of them were + being published. A wrong name is worse than an absent one, which is the argument + #1051 turned on. + """ + forwarding, _ = _suite_recorders(text) + names = list(_bash_names(text)) + for helper, pos in sorted(forwarding.items()): + names += re.findall(_pattern_for(pos, (helper,)), _strip_comments(text)) + # THE FILTER IS APPLIED ONCE, AT THE END, AND TO BOTH SOURCES. A forwarder calling + # another forwarder -- `ans() { ansp "$1" h c "$2"; }` -- is a call site like any + # other to the pattern, and it yields `$1`. Filtering only the definitions left + # that one through, which the fixture below caught. + return [n for n in names if _template(n) != "{}"] + + def main(bash_file, py_file): """-> the exit status: 1 when a bash property has no counterpart.""" - bash_names = _bash_names(open(bash_file).read()) + bash_src = open(bash_file).read() + + # THE REFUSE HALF (#1053). A helper that reaches a recorder whose name argument + # cannot be resolved makes every name it carries invisible, and grading the rest + # would report a verdict about a suite the tool has only partly read. That is the + # shape this whole issue is about, so it is a refusal rather than a silent skip. + _forwarding, unreadable = _suite_recorders(bash_src) + if unreadable: + print(f"REFUSED: {bash_file} defines {len(unreadable)} helper(s) that record a " + f"check under a name this cannot resolve:") + for helper in unreadable: + print(f" unreadable {helper}") + print() + print("Every check they carry is invisible, so any verdict here would be about " + "the part of the suite that happens to be readable. Give the helper a " + "name argument in a position the extractor can see, or record directly.") + return 2 + + bash_names = _names_in(bash_src) py_names = _py_names(open(py_file).read()) bset, pset = set(bash_names), set(py_names) diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 3e280e7f..e2365d49 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -96,7 +96,15 @@ # `$(dirname ` as check names out of a suite containing no position-2 helper. Found # when a reviewer's stale `.pyc` left the table mid-mutation. Re-derived by # collection: `322 tests collected`. -guard_tests 322 +# 322 -> 329 when the grader learned a suite's OWN recorders (#1053): the forwarding +# shapes, a composing wrapper left alone, the refusal of a helper whose name position +# cannot be resolved, the corpus budget pinned at one suite, the bare-{} drop, the +# grader's refusal exercised through `main`, and a helper reaching only `pgc_record`. +# +# #1054 moves this to 324 on ITS tree, from a different arm. Whichever lands second +# re-derives by collection -- 322 and 322 collected 323 earlier today, and nothing +# conflicted. +guard_tests 329 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index a1ba585d..fd5adb84 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -41,7 +41,10 @@ HERE = pathlib.Path(__file__).resolve().parent sys.path.insert(0, str(HERE)) -from compare_to_bash import _as_names, _parametrized_names, _py_names, _template # noqa: E402 +from compare_to_bash import (_as_names, _bash_names, _bodies, # noqa: E402 + _derive_recorders, _names_in, _suite_recorders, + _parametrized_names, _py_names, _strip_comments, + _template, _words) # THE PAIRS THIS TREE HOLDS, DECLARED IN BOTH DIRECTIONS (#1046). @@ -113,115 +116,6 @@ } -def _strip_comments(text): - r"""-> the text with shell comments removed, and NOTHING else removed. - - `#` starts a comment only at a word boundary. `${shape#*|}` and `$#` are not - comments, and cutting at the first `#` truncates the line to something that - parses as a different program. That exact slip has produced two wrong counts in - this repo, so the fixtures for it are in the arm below rather than in a comment. - """ - out = [] - for line in text.splitlines(): - res, i, quote = [], 0, None - while i < len(line): - ch = line[i] - if quote: - if ch == quote: - quote = None - res.append(ch) - elif ch in "\"'": - quote = ch - res.append(ch) - elif ch == "#" and (i == 0 or line[i - 1] in " \t;&|()"): - break - else: - res.append(ch) - i += 1 - out.append("".join(res)) - return "\n".join(out) - - -def _bodies(text): - """-> [(function name, body)] with each body ended by ITS OWN closing brace. - - Per-line brace depth, not `find("\n}")`: 199 definitions in this tree are written - on one line (`q() { psql ...; }`), and a scan for a brace in the first column - swallows every following definition into the first one's body. - """ - out, lines = [], text.splitlines() - for i, line in enumerate(lines): - m = re.match(r"[ \t]*([A-Za-z_][A-Za-z0-9_]*)[ \t]*\(\)[ \t]*\{", line) - if not m: - continue - depth = line.count("{") - line.count("}") - body, j = [line[m.end():]], i + 1 - while j < len(lines) and depth > 0: - depth += lines[j].count("{") - lines[j].count("}") - body.append(lines[j]) - j += 1 - out.append((m.group(1), "\n".join(body))) - return out - - -def _words(text): - """-> the shell words of a call's argument list, quotes kept.""" - return re.findall(r'"[^"]*"|\S+', text) - - -def _derive_recorders(lib, seed=("pgc_record", 2)): - """-> {helper: which argument holds the check name}, derived from what lib.sh DOES. - - THE POPULATION IS THE POINT (#1045). The #1040 guard derived its population by - SPELLING -- every `lib.sh` function whose name begins `check`. It was green for - weeks while `diff_query` went unread, and correctly so: `diff_query` was never in - its population. The guard was not broken; the definition of the thing it guards - was. 225 names across 59 suites were outside it. - - So: start from `pgc_record`, the primitive that actually records, and take the - closure. A function is a recorder at position N when it passes its own `$N` -- - directly, or renamed once through a `local` -- into the name slot of a helper - already known to be one. `diff_query` calls `check`, which calls `pgc_record`. - One level of indirection was the entire gap. - - The seed is not returned. It is `lib.sh`'s own primitive and no suite calls it, - which the arm below asserts rather than assumes: the day a suite calls it, the - extractor has to learn it and this stops being true quietly. - """ - lib = _strip_comments(lib) - defs = _bodies(lib) - known = {seed[0]: seed[1]} - - changed = True - while changed: - changed = False - for fn, body in defs: - if fn in known: - continue - aliases = {m.group(1): int(m.group(2)) for m in - re.finditer(r'\b([A-Za-z_][A-Za-z0-9_]*)="\$\{?(\d+)\}?"', body)} - for rec, pos in sorted(known.items()): - for m in re.finditer(r'\b' + rec + r'([ \t]+.*)$', body, re.M): - args = _words(m.group(1)) - if len(args) < pos: - continue - slot = args[pos - 1] - inner = re.fullmatch(r'"\$\{?([A-Za-z_0-9]+)\}?"', slot) - if not inner: - continue # a literal, or something not a bare $x - tok = inner.group(1) - n = int(tok) if tok.isdigit() else aliases.get(tok) - if n is None: - continue - known[fn] = n - changed = True - break - if fn in known: - break - - del known[seed[0]] - return known - def _names(src): return _py_names(src) @@ -863,6 +757,217 @@ def test_the_suite_local_helpers_are_known_and_excluded(expect): "and none of their suites has a pytest twin, so none is graded today") +def test_a_suites_own_forwarding_wrapper_is_read(expect): + """THE SUITE-LOCAL GAP (#1053). `lib.sh`'s wrappers were #1051; these are the ones + a suite defines for itself. + + sorted_pathkeys.sh ans() { ansp "$1" h c "$2"; } + ansp() { local label="$1"; check_text "$label" ...; } + + 19 of that suite's 113 names go through those two, and they are not a random 19: + the suite pairs every `check "... plans no Sort"` with an `ans "and ... still + answers correctly"`, because losing the Sort is only correct if the rows still + come back in that order. So the grader saw every claim about the PLAN and none + about the ANSWER, and a port dropping all 18 answer arms would have graded + one-for-one. + """ + src = ('check_text() { pgc_record "$1" "$2"; }\n' + 'ansp() {\n\tlocal label="$1"\n\tcheck_text "$label" "$2" "$3"\n}\n' + 'ans() { ansp "$1" h c "$2"; }\n' + 'ans "and returns the same rows in the same order as heap" \'SELECT 1\'\n' + 'ansp "and it answers in j order" h c \'SELECT 2\'\n') + forwarding, unreadable = _suite_recorders(src) + expect.text(", ".join(f"{k}:${v}" for k, v in sorted(forwarding.items())), + "ans:$1, ansp:$1", + "both forwarding shapes are found -- one through a local, one a " + "one-line body") + expect.text(", ".join(unreadable) or "none", "none", + "and neither is refused, because both name positions resolve") + expect.text(", ".join(sorted(_names_in(src))), + "and it answers in j order, and returns the same rows in the same " + "order as heap", + "the names are read from the CALL SITES, which is where a forwarding " + "wrapper's names are") + + +def test_a_composing_wrapper_is_left_alone(expect): + """COMPOSE IS NOT A GAP, and treating it as one was the expensive mistake. + + native_ownership.sh refused() { check "non-owner refused: ${1%%(*}" ...; } + + `_BASH_INTERP` reduces `${1%%(*}` to `{}`, so the definition already states + `non-owner refused: {}` -- a real template naming a real property, covering all + nine call sites. That is why `native_ownership` grades one-for-one today. + + MEASURED BEFORE THIS ARM EXISTED: refusing on "the name slot is not a bare + positional" refuses 32 suites, including `hilbert_cluster`, `hilbert_locality` and + `native_ownership` -- three pairs that are COMPLETE -- to fix nothing. The refuse + half is right in principle and, aimed at this population, it breaks green pairs. + """ + src = ('check() { pgc_record "$1" "$2"; }\n' + 'refused() {\n\tcheck "non-owner refused: ${1%%(*}" "$2" "$3"\n}\n' + 'refused "read_projection(x)" a b\n') + forwarding, unreadable = _suite_recorders(src) + expect.text(", ".join(forwarding) or "none", "none", + "a composing wrapper is not a forwarder, so its call sites are not " + "re-read") + expect.text(", ".join(unreadable) or "none", "none", + "and it is NOT refused: the definition states the property") + expect.text(", ".join(_names_in(src)), "non-owner refused: ${1%%(*}", + "the template is read from the definition, as it always was") + + +def test_a_helper_whose_name_cannot_be_resolved_is_refused(expect): + """THE REFUSE HALF. `hilbert_curve.sh` is the whole population, and it is real: + + arms_failed() { # arms_failed "LIST" DETAIL + while IFS= read -r _a; do + [ -n "$_a" ] && pgc_fail "$_a" "$2" + done <<< "$1" + } + + The names are newline-separated INSIDE `$1` and reach `pgc_fail` through a loop + variable, so no rule about argument positions can read them. Silently skipping is + how 147 names in 14 suites came to be ungraded, so this names the helper and + refuses the suite instead. + """ + src = ('pgc_fail() { pgc_record FAIL "$1" "$1"; }\n' + 'arms_failed() {\n\tlocal _a\n\twhile IFS= read -r _a; do\n' + '\t\t[ -n "$_a" ] && pgc_fail "$_a" "$2"\n\tdone <<< "$1"\n}\n') + forwarding, unreadable = _suite_recorders(src) + expect.text(", ".join(unreadable), "arms_failed", + "a helper that reaches a recorder under an unresolvable name is " + "refused by name") + expect.text(", ".join(forwarding) or "none", "none", + "and is not silently treated as a forwarder") + + +def test_the_refusal_names_exactly_the_suites_it_refuses(expect): + """A STATIC GUARD NEEDS A FALSE-POSITIVE BUDGET, measured over the tree before it + ships rather than discovered by it. One suite of 264 is refused and it is a true + positive; every graded pair is unchanged. + + THE SET IS PINNED, NOT THE COUNT, and that is not a style choice. + `checks_never_observed_red` is this repo's worked example of the other shape: a + census over the tree that every legitimate addition broke, so the only way to land + one was to raise a number the design said may only fall, which retires the guard + the first time it is inconvenient. A count tells a reviewer that something moved. + A set tells them WHAT, which is the difference between a diff they can judge and a + number they can only bump. + + Asserted in both directions, so an entry cannot outlive its cause: a suite that + starts being refused reddens with its name, and one that stops reddens too. + """ + refused = sorted(sh.name[:-3] for sh in HERE.parent.glob("*.sh") + if sh.name != "lib.sh" and _suite_recorders(sh.read_text())[1]) + expect.text(", ".join(refused), "hilbert_curve", + "exactly these suites are refused -- `hilbert_curve`, whose helpers " + "take a newline-separated LIST of names in one argument") + twinned = [r for r in refused if (HERE / f"test_{r}.py").exists()] + expect.text(", ".join(twinned) or "none", "none", + "and none of them has a pytest twin, so the refusal grades nothing " + "today") + + +def test_a_bare_interpolation_is_not_published_as_a_name(expect): + """A FORWARDING WRAPPER'S DEFINITION STATES NO PROPERTY, and publishing `{}` for it + is worse than publishing nothing. + + `check_text "$label"` reduces to the template `{}`. Published, it sits in MISSING + naming nothing a port could assert -- and it MATCHES a port name that is entirely + one interpolation, which is a pass for a property neither side named. 17 were + being published across the corpus. A wrong name is worse than an absent one, which + is the argument #1051 turned on. + """ + src = ('check_text() { pgc_record "$1" "$2"; }\n' + 'ansp() {\n\tlocal label="$1"\n\tcheck_text "$label" "$2" "$3"\n}\n' + 'ansp "a real property" a b\n') + expect.num(sum(1 for n in _bash_names(src) if _template(n) == "{}"), 1, + "premise: the raw extractor does publish a bare {} for this shape") + expect.text(", ".join(_names_in(src)), "a real property", + "and the reader drops it, keeping only names that state something") + + +def test_the_grader_itself_refuses_the_suite_it_cannot_read(expect, tmp_path): + """THE REFUSAL RUNS, not just the classifier that feeds it. + + `_suite_recorders` returning an unreadable helper is asserted above. That is not + the same as `main` acting on it, and the difference is the whole value of the + refuse half: a classifier nobody consults is a list. MEASURED -- disabling the + refusal in `main` reddened no arm at all until this one existed, which is the same + defect the refusal exists to prevent, one level up. + + THE PORT SIDE IS A FIXTURE, not one of this tree's real ports. Naming a real + `test_*.py` here made `test_harness_deps.py` classify THIS file as cluster-bound, + because a file that drives a cluster-bound file needs whatever that file needs -- + and it was right to. The arm is about the grader's refusal, not about any port, so + it supplies its own. + """ + import io, contextlib + from compare_to_bash import main + + port = tmp_path / "test_stub.py" + port.write_text("def test_x(expect):\n expect.num(1, 1, 'a name')\n") + + out = io.StringIO() + with contextlib.redirect_stdout(out): + rc = main(str(HERE.parent / "hilbert_curve.sh"), str(port)) + text = out.getvalue() + expect.num(rc, 2, "the grader exits 2 -- neither a pass nor an ordinary MISSING -- " + "on a suite whose names it cannot fully read") + expect.num(1 if "REFUSED" in text else 0, 1, "and says so") + expect.num(1 if "arms_failed" in text and "arms_unrunnable" in text else 0, 1, + "naming both helpers, so the reader knows what to change") + expect.num(1 if "missing:" in text else 0, 0, + "and prints NO verdict, because a verdict about a partly-read suite is " + "the thing this refuses to produce") + + # THE CONTROL: a suite it CAN read still grades, so the refusal is about the + # unreadable helper and not about every foreign suite. + readable = tmp_path / "readable.sh" + readable.write_text('check "a property the port also asserts" "$a" "$b"\n') + port.write_text("def test_x(expect):\n" + " expect.num(1, 1, 'a property the port also asserts')\n") + out = io.StringIO() + with contextlib.redirect_stdout(out): + rc = main(str(readable), str(port)) + expect.num(rc, 0, "control: a readable suite still grades, and grades clean") + expect.num(1 if "missing:" in out.getvalue() else 0, 1, + "and DOES print a verdict, so the refusal above is the difference") + + +def test_a_helper_reaching_only_the_primitive_is_found(expect): + """THE SEED IS `pgc_record`, NOT THE `check` FAMILY, and four suites turn on it. + + phase4.sh::assert_plan reaches: pgc_record + phase5.sh::assert_plan reaches: pgc_record + audit.sh::expect_error reaches: pgc_record + + None of them ever calls a `check_*` helper. A closure seeded from the check family + cannot see them, and seeding from the primitive is the difference between 145 + names read and 89 -- measured against a second implementation that seeded from + `check*` and came up 56 short across exactly these four suites. + + This is the direct-call-versus-closure error one level up: closing over `check*` + without closing over the thing `check*` itself closes over. + """ + src = ('pgc_record() { PGC_CHECKS=$((PGC_CHECKS + 1)); }\n' + 'assert_plan() {\n\tlocal name="$1"\n' + '\tpgc_record PASS "$name" "PASS $name"\n}\n' + 'assert_plan "the plan has no Sort" "$(plan)"\n') + forwarding, unreadable = _suite_recorders(src) + expect.text(", ".join(f"{k}:${v}" for k, v in sorted(forwarding.items())), + "assert_plan:$1", + "a helper that reaches ONLY the primitive is still a recorder") + expect.text(", ".join(_names_in(src)), "the plan has no Sort", + "and its call sites are read") + + # AND ON THE REAL TREE, because the fixture proves the rule and not the corpus. + real = _suite_recorders((HERE.parent / "phase4.sh").read_text())[0] + expect.num(1 if "assert_plan" in real else 0, 1, + "phase4.sh's assert_plan is found in the tree, not only in a fixture") + + def test_every_pair_in_the_tree_is_declared(expect): """THE DECLARATION IS ASSERTED IN BOTH DIRECTIONS (#1046).