Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_<something>`, 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).

Expand Down
45 changes: 45 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_<something>`, **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 |
Expand Down Expand Up @@ -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 |
48 changes: 46 additions & 2 deletions test/pytest/compare_to_bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 `{}`.

Expand All @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
101 changes: 101 additions & 0 deletions test/pytest/test_compare_to_bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import ast
import pathlib
import re
import sys

HERE = pathlib.Path(__file__).resolve().parent
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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_<something>`. 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/<stem>.sh` against a
`test/pytest/test_<stem>.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_<something>`, 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.

Expand Down
18 changes: 18 additions & 0 deletions test/pytest/test_harness_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
Loading