diff --git a/CHANGELOG.md b/CHANGELOG.md index bfc30042..51513131 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1297,6 +1297,42 @@ true until the next version shipped. ### Fixed +- `compare_to_bash.py` read five of the eight check helpers `lib.sh` defines + (#432, #1040). + + The bash side of the parity tool matched + `check(?:_num|_ratio|_text|_timing)?`, so `check_unrunnable`, `check_skip` and + `check_ratio_needs_quiet_machine` were invisible. A property asserted through one of + the three was never reported MISSING and could not move `rc`, which means **a pair + could grade one-for-one on the strength of the grader's blind spot.** + `hilbert_locality` was exactly that, and #1041 closed the two properties it was + hiding. + + It never drifted out of date: `0cbf574` introduced that pattern, and + `check_unrunnable` already had 21 call sites that day. Same defect shape as the + python side in #1036 and #1038 -- a rule true of most of a class taken for a + property of the class -- sitting on the other side of the same tool for five days. + + The helper list is hand-written, so it is pinned the way `_NAME_ARG` is: an arm + re-derives it from `lib.sh`'s DEFINITIONS and fails with the helper named. Two more + arms state what the tool does not cover -- the four suite-local helpers + (`check_structure`, `check_reconstruct`, `check_split_happened`, `check_float`), + none of whose suites has a pytest twin, and the pattern shape that used to make a + prefix unreadable. + + **The longest-first ordering is NOT what makes that work, and the code said + otherwise until it was measured.** Python's `re` backtracks across alternatives, so + a pure reorder reads both names identically. What the old pattern could not do was + read `check_ratio_needs_quiet_machine` at all: it matches `check_ratio`, wants + whitespace, finds `_needs`, backtracks to the empty option, wants whitespace after + `check`, and fails. Measured on a fixture holding both, the old form reads + `['short']` and this one reads `['short', 'long']`. The arm pins the pattern shape; + the ordering is readability. + + The population is `check` or `check_`, not `check[a-z_]*`: the loose form + also matches `checks_in` in `decode_interrupts.sh`, a counting utility that returns a + number and records nothing. + - `compare_to_bash.py` read the wrong argument for the four helpers whose name is not last (#432, #1036). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index bd565109..ef8d47db 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -3657,6 +3657,48 @@ standalone (`ast`, `re`, `sys`) and cannot import `Expect` to ask where each nam out of `pgc_vacuity.py`, recomputes every entry, and fails with the helper named when the two disagree. +### The BASH side had the same blind spot, and it shipped that way (#1040) + +Everything above is about the python side. The bash side read five of the eight check +helpers `lib.sh` defines: + + check check_num check_text check_ratio check_timing READ + check_unrunnable check_skip check_ratio_needs_quiet_machine INVISIBLE + +A property asserted through one of the three was never reported MISSING and could not +move `rc`, so **a pair could grade one-for-one on the strength of the grader's blind +spot.** `hilbert_locality` was exactly that: two of the four properties its unrunnable +branch records had no counterpart in the port, and #1041 closed them. + +It never drifted out of date. `0cbf574` introduced the pattern, and `check_unrunnable` +already had 21 call sites that day. + +All eight take the check NAME as `$1`, so one pattern serves them all. That is a +property of these helpers rather than of bash, which is why the drift guard re-reads it +from `lib.sh` instead of trusting it. + +`check_ratio` is a prefix of `check_ratio_needs_quiet_machine`, and **the old pattern +shape could not read the longer one at all**: `check(?:_num|_ratio|_text|_timing)?\s+"` +matches `check_ratio`, wants whitespace, finds `_needs...`, backtracks to the empty +option, wants whitespace after `check`, and fails. Measured on a fixture holding both, +the old form reads `['short']` and the current one reads `['short', 'long']`. + +**The entries are listed longest-first for readability, and that ordering is NOT what +makes it work.** Python's `re` backtracks across alternatives, so a pure reorder reads +both names identically -- measured, and the arm stays green under it. Said explicitly +because the list LOOKS as though its order is load-bearing, and the arm pins the pattern +shape rather than the order. + +**Suite-local helpers are out of scope, asserted rather than assumed.** Four suites +define one of their own (`check_structure`, `check_reconstruct`, `check_split_happened` +in `parallel_copy.sh`, `check_float` in `parquet_export_stats.sh`) and none has a pytest +twin, so none is graded. An arm holds both halves, so the day one is ported the grader's +limit is stated rather than discovered. + +The population is `check` or `check_`, **not** `check[a-z_]*`: the loose form +also matches `checks_in` in `decode_interrupts.sh`, a counting utility that returns a +number and records nothing. + ### Removal proof | mutation | red | @@ -3694,4 +3736,7 @@ the tool grades THIS tree. | `test_a_helper_whose_name_is_optional_takes_it_only_from_the_keyword` | `plan_marker` and `plan_node` carry no name positionally; absent beats a key | | `test_the_tools_table_agrees_with_the_signatures_it_describes` | the drift guard: every entry re-derived from the real signatures | | `test_no_later_argument_can_overtake_the_name` | nothing after the name may be passed positionally, so `-1` is true of every CALL and not just every signature | +| `test_the_extractor_reads_every_check_helper_lib_sh_defines` | the BASH-side drift guard: the helper list re-derived from `lib.sh`'s definitions | +| `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_the_ported_suites_in_this_tree_are_graded_one_for_one` | the standing arm: every pair in the tree, graded | diff --git a/test/pytest/compare_to_bash.py b/test/pytest/compare_to_bash.py index 69683c56..6dde85c3 100755 --- a/test/pytest/compare_to_bash.py +++ b/test/pytest/compare_to_bash.py @@ -207,6 +207,51 @@ def _as_names(node): r'\$\{[^}]*\}|\$\([^)]*\)|\$[A-Za-z_][A-Za-z0-9_]*|\$[0-9]+|\$[@*#?]') +# EVERY CHECK HELPER `lib.sh` DEFINES, and the pattern built from it (#1040). +# +# This read five of the eight. `check_unrunnable`, `check_skip` and +# `check_ratio_needs_quiet_machine` matched no branch, so a bash property asserted +# through any of them was INVISIBLE: never reported MISSING, never able to move `rc`, +# and therefore a pair could grade one-for-one because the grader could not see the +# gap. `hilbert_locality` was exactly that -- two of the four properties its +# unrunnable branch records had no counterpart in the port. +# +# All eight take the check NAME as `$1`, so one pattern serves them all; that is a +# property of these helpers rather than of bash, and the drift guard re-reads it. +# +# `check_ratio` is a prefix of `check_ratio_needs_quiet_machine`, and the OLD pattern +# shape could not read the longer one at all: `check(?:_num|_ratio|_text|_timing)?\s+"` +# matches `check_ratio`, needs whitespace, finds `_needs...`, backtracks to the empty +# option, needs whitespace after `check`, and fails. Measured on a fixture holding both: +# the old form reads ['short'], this one reads ['short', 'long']. +# +# The entries are written longest-first for readability. **That ordering is NOT what +# makes it work** -- Python's `re` backtracks across alternatives, so a pure reorder +# reads both names identically (measured). An arm pins the BEHAVIOUR rather than the +# order, because the order is the thing that looks load-bearing and is not. +# +# Hand-written so the tool stays standalone, and pinned like `_NAME_ARG`: +# `test_compare_to_bash.py` reads the DEFINITIONS out of `lib.sh` and fails with the +# helper named when the two part company. +# +# SUITE-LOCAL HELPERS ARE OUT OF SCOPE, deliberately. Four suites define one of their +# own (`check_structure`, `check_reconstruct`, `check_split_happened` in +# `parallel_copy.sh`, `check_float` in `parquet_export_stats.sh`) and none of the four +# has a pytest twin, so none is graded. An arm asserts both halves of that. +_BASH_HELPERS = ( + "check_ratio_needs_quiet_machine", + "check_unrunnable", + "check_timing", + "check_ratio", + "check_text", + "check_skip", + "check_num", + "check", +) + +_BASH_PATTERN = (r'\b(?:' + "|".join(_BASH_HELPERS) + r')\s+"([^"]+)"') + + def _template(name): """-> the name with every interpolation reduced to `{}`. @@ -218,8 +263,7 @@ def _template(name): def main(bash_file, py_file): """-> the exit status: 1 when a bash property has no counterpart.""" - bash_names = re.findall( - r'\bcheck(?:_num|_ratio|_text|_timing)?\s+"([^"]+)"', open(bash_file).read()) + bash_names = re.findall(_BASH_PATTERN, open(bash_file).read()) 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 9d4b63a7..cd0599fd 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -75,7 +75,12 @@ # and this branch's REASON check are complementary, and pgc_vacuity.py kept both. # Re-derived by collection on the merged tree: the two 307s and the 310 that # preceded this are each right for a DIFFERENT tree and none of them is this one. -guard_tests 313 +# 313 -> 316 when the bash-side extractor was widened to every check helper lib.sh +# defines (#1040): a drift guard re-deriving that list from lib.sh's DEFINITIONS, a +# shadowing arm (check_ratio is a prefix of check_ratio_needs_quiet_machine), and one +# pinning the four suite-local helpers as knowingly out of scope. +# Re-derived by collection against main 0d17a87: `316 tests collected`. +guard_tests 316 # 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 dd8fd653..56d8d74c 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -35,6 +35,7 @@ import ast import pathlib +import re import sys HERE = pathlib.Path(__file__).resolve().parent @@ -294,6 +295,7 @@ def test_the_tools_table_agrees_with_the_signatures_it_describes(expect): # The one hand-written semantic claim above, pinned against the body it describes: # read cannot_run's own `_record(...)` call and check which parameter it names. + # fn = [f for f in helpers if f.name == "cannot_run"] expect.num(len(fn), 1, "premise: cannot_run is among the helpers read") recorded = [kw.value.id for call in ast.walk(fn[0]) @@ -364,6 +366,105 @@ def test_no_later_argument_can_overtake_the_name(expect): "no positional argument can be written after the name and be read as it") +def test_the_extractor_reads_every_check_helper_lib_sh_defines(expect): + """THE BASH-SIDE DRIFT GUARD (#1040), and the mirror of the table guard above. + + The extractor read five of the eight check helpers `lib.sh` defines. The other + three -- `check_unrunnable`, `check_skip`, `check_ratio_needs_quiet_machine` -- + matched no branch of its pattern, so a bash property asserted through any of + them was invisible, was never reported MISSING, and could not move `rc`. + + **A pair could therefore be declared one-for-one on the strength of the grader's + blind spot**, which is what `hilbert_locality` was: two of the four properties + its unrunnable branch records had no counterpart in the port at all. + + The helper list is hand-written, for the same reason `_NAME_ARG` is: the tool + stays standalone. So it is pinned the same way -- this reads the DEFINITIONS out + of `lib.sh` and fails with the helper named when the two part company. Add a + `check_whatever()` to `lib.sh` and this goes red before a suite using it is + silently ungraded. + + Suite-LOCAL helpers are deliberately not in scope here; that is asserted, with + its reason, in the arm below. + """ + from compare_to_bash import _BASH_HELPERS + + lib = (HERE.parent / "lib.sh").read_text() + # `check` or `check_`. NOT `check[a-z_]*`, which also matches + # `checks_in` -- a COUNTING utility in decode_interrupts.sh that returns a + # number and records nothing. Define the population before counting it. + defined = set(re.findall(r'^(check(?:_[a-z_]+)?)\(\)\s*\{', lib, re.M)) + expect.at_least(len(defined), 8, + "premise: lib.sh's check helpers were found, not an empty set") + + missing = sorted(defined - set(_BASH_HELPERS)) + extra = sorted(set(_BASH_HELPERS) - defined) + expect.text(", ".join(missing) or "none", "none", + "every check helper lib.sh defines is one the extractor reads") + expect.text(", ".join(extra) or "none", "none", + "and the extractor claims no helper lib.sh does not define") + expect.num(len(_BASH_HELPERS), len(defined), + "inputs == sum(buckets): the two lists are the same size") + + +def test_a_longer_helper_name_is_not_shadowed_by_a_shorter_one(expect): + r"""`check_ratio` is a PREFIX of `check_ratio_needs_quiet_machine`. + + THE PRE-#1040 PATTERN COULD NOT READ THE LONGER ONE AT ALL, and that is what this + holds. `check(?:_num|_ratio|_text|_timing)?\s+"` matches `check_ratio`, wants + whitespace, finds `_needs...`, backtracks to the empty option, wants whitespace + after `check`, and fails. Measured on the fixture below: the old form reads + `['short']`, the current one reads `['short', 'long']`. + + WHAT THIS ARM DOES NOT HOLD, said out loud because the code reads as though it + does: the entries are listed longest-first, and that ordering is NOT load-bearing. + Python's `re` backtracks across alternatives, so a PURE REORDER putting + `check_ratio` first reads both names identically -- measured, and this arm stays + green under it. The order is the thing that looks decisive and is not; the pattern + SHAPE is the thing that is. + """ + import re as _re + from compare_to_bash import _BASH_PATTERN + src = ('\tcheck_ratio "the short one" "$a" "$b" 2\n' + '\tcheck_ratio_needs_quiet_machine "the long one" "$a" "$b" 2\n') + got = _re.findall(_BASH_PATTERN, src) + expect.text(", ".join(sorted(got)), "the long one, the short one", + "both are read; the longer name is not eaten by the shorter") + + +def test_the_suite_local_helpers_are_known_and_excluded(expect): + """Four helpers are defined by ONE suite each, and the extractor does not read + them. That is a scope decision and it is asserted rather than left implicit. + + `compare_to_bash.py` grades a `test/.sh` against a + `test/pytest/test_.py`. None of the four suites defining its own helper + has a pytest twin, so none is graded and the exclusion costs nothing TODAY. + The day one of them is ported, this arm is what says the grader cannot see it. + + The population is `check_`, which is not the same as "starts with + check": `checks_in` in `decode_interrupts.sh` is a COUNTING utility returning a + number of interrupt checks in a function body, and records nothing. It was in + this list until the arm printed it and the definition was read. + """ + root = HERE.parent + local = {} + for sh in sorted(root.glob("*.sh")): + if sh.name == "lib.sh": + continue + for h in re.findall(r'^(check_[a-z_]+)\(\)\s*\{', sh.read_text(), re.M): + local.setdefault(h, sh.name) + expect.text(", ".join(f"{h} ({f})" for h, f in sorted(local.items())), + "check_float (parquet_export_stats.sh), " + "check_reconstruct (parallel_copy.sh), " + "check_split_happened (parallel_copy.sh), " + "check_structure (parallel_copy.sh)", + "the suite-local helpers are exactly these four") + twinned = [f for h, f in local.items() + if (HERE / f"test_{f[:-3]}.py").exists()] + expect.num(len(twinned), 0, + "and none of their suites has a pytest twin, so none is graded today") + + def test_the_ported_suites_in_this_tree_are_graded_one_for_one(expect): """THE STANDING ARM, and the reason this file is not only about fixtures. diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index 4ef25dd3..bee165dc 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -1127,6 +1127,18 @@ def test_the_job_installs_no_database_driver(expect): # rather than by pattern, which is the only way to tell the first from a string the # code actually passes to bash. +# THE SCAN IS THIS DIRECTORY, AND THAT IS DELIBERATE (jd, 2026-09-13). The inventory is +# the PYTEST CORPUS'S SELF-GUARD. Tooling under `.github/scripts/` belongs to neither +# harness -- it is CI's, and a tool there reading both sides is what it is for -- so it +# is out of scope rather than exempt, and two such tools do read `test/*.sh` today. +# +# Recorded because the distinction is invisible from the output and was inferred wrongly +# once: absent-from-the-report and outside-the-scan produce identical evidence, and a +# reader who meets the second and concludes the first will also conclude that MOVING a +# file to `.github/scripts/` deletes its crossing. Under this reading a move there is +# defensible on design grounds -- that is where cross-harness tooling lives -- but never +# because it makes this arm stop reporting the file. +# # THE DESCRIPTIONS NAME NO FILE, and that is not squeamishness: the first version # spelled the helper library's path in them, and the detector flagged THIS file for its # own inventory -- four files where the tree has three. The mechanism is what the entry @@ -1144,6 +1156,12 @@ def test_the_job_installs_no_database_driver(expect): "test_mutation_ledger.py": "executes the matrix runner with its list flag to get the registered suite " "list, which is the same mechanism the entry above uses", + "test_compare_to_bash.py": + "reads the shell harness's helper library to re-derive the parity tool's " + "check-helper list from the definitions themselves, so the list the grader " + "matches on cannot become a hand-maintained copy that rots apart from it " + "(#1040, where five of eight helpers had been unreadable since the pattern " + "shipped)", } _SHELL_NAMES = re.compile(