test/pytest: stop driving lib.sh for properties selftest/340 already holds (#432)

CHANGELOG.md

@@ -496,6 +496,61 @@ true until the next version shipped.
   is the title and colliding with the real entry 5. The fixture reported 3 entries in a
   two-entry document, which is how it was found.
 
+- The pytest corpus no longer drives the shell harness for properties the shell
+  harness already holds, and the inventory of what remains is down to four calls,
+  none of them debt (#432).
+
+  CONTEXT.md's rule: the two harnesses are parallel in FUNCTIONALITY and independent
+  in CALL. `test_build_refusal.py` drove `test/lib.sh` for seven calls over
+  `pgc_write_source_stamp`, `pgc_source_stamp_path`, `pgc_freshness_report` and
+  `pgc_freshness_verdict` -- four PURE SHELL functions, so the python arms were a
+  second measurement of someone else's subject, agreeing with it by construction.
+
+  **Five arms removed and not one needed porting.** `test/selftest/340` already held
+  every property they asserted, and more of it in each case:
+
+  | the python arm | what 340 already had |
+  | --- | --- |
+  | the stamp writer reports failure | the same, plus a premise that the stamp really was not written |
+  | two installations of one major do not share a stamp | the same, plus pkglibdir keying and two unreadable pg_configs |
+  | the report names each file and states how many | the same two arms |
+  | an empty manifest says so rather than printing nothing | the same arm |
+  | a failed digest is unknown, never a false stale | the same, over TWO unreadable files, plus the premise below |
+
+  340's version of the last one carries a premise the pytest twin did not state: that
+  the unprivileged read AGREES with the privileged one while nothing is denied.
+  Without it the arms measure the user switch rather than the permission denial.
+
+  **The first measurement of 340 was wrong and nearly cost a duplicate.** Enumerating
+  its checks with `grep -cE '^check "'` gave 81. The real number is **89**: 340 has
+  indented `check` calls inside an `if` and a `for`, and the eight the sweep missed
+  are exactly the unreadable-source block. On that bad count one python arm looked
+  like a genuine gap, and a duplicate of it was written -- and proven to discriminate
+  against a mutation -- before the duplication was noticed.
+
+  **The tell was a duplicate check name.** `pgc_ledger.py` reported "duplicate check
+  name in one run, so one ledger row covers 2", and the first response was to rename
+  the new check. The right response to a name that already exists is to ask WHY it
+  exists. Renaming it hid the only evidence that the work was unnecessary. A check
+  sweep has to be anchored at `^[[:space:]]*`, not at column 0.
+
+  Two mutations were run against the duplicate before it was discarded, and the first
+  was a no-op for a reason worth keeping: `pgc_source_fingerprint` returns empty
+  because the MODULE prints nothing and exits 0, not because the wrapper's `rc != 0`
+  branch fires. Mutating that branch changes nothing on this path. The property lives
+  in `test/pgc_fingerprint.py`, and the wrapper's contribution is only that it does
+  not substitute a value for the module's empty answer.
+
+  What remains in `test_build_refusal.py` is four calls in two arms, both named:
+  `test_the_two_fingerprint_implementations_cover_the_same_inputs`, which is the one
+  permitted cross-reference because the property IS the relationship between the two
+  implementations, and a historical-parity arm whose fixture is its own. `_sh_fp_as`
+  is deleted with its last caller.
+
+  TESTS.md loses the rows naming the deleted arms, and three prose passages that named
+  them are rewritten to say where the property lives now rather than left pointing at
+  arms that do not exist. A backticked name is a claim that it exists.
+
 - A count `grep` never produced no longer reads as "present" (#929).
 
   #922 replaced roughly 28 `producer | grep -q PAT` tests with

CONTEXT.md

@@ -241,13 +241,25 @@ Python that reaches into shell:
   `chmod 000`, one varies the locale -- and both use the module's own CLI, which is
   the entry point lib.sh uses with lib.sh taken out of the path.
 
-  **13 calls remain, in three groups, and only the first is debt.** Seven drive
-  `pgc_write_source_stamp`, `pgc_source_stamp_path`, `pgc_freshness_report` and
-  `pgc_freshness_verdict`, which are PURE SHELL rather than wrappers over shared
-  code: those properties belong to the shell harness and moving them is the next
-  step. Two are `test_the_two_fingerprint_implementations_cover_the_same_inputs`,
-  which reaches across on purpose -- see below. The last two are a historical-parity
-  arm whose fixture is its own, bar one call for a directory list.
+  **FOUR calls remain and none of them is debt.** Two are
+  `test_the_two_fingerprint_implementations_cover_the_same_inputs`, which reaches
+  across on purpose -- see below. Two are a historical-parity arm whose fixture is
+  its own.
+
+  The seven that drove `pgc_write_source_stamp`, `pgc_source_stamp_path`,
+  `pgc_freshness_report` and `pgc_freshness_verdict` are **gone**, and not one of
+  them needed porting: `test/selftest/340` already held every property they
+  asserted, with more arms in each case. Measured arm by arm before anything was
+  deleted.
+
+  **THE FIRST MEASUREMENT OF 340 WAS WRONG AND NEARLY COST A DUPLICATE.** Enumerating
+  its checks with `grep -cE '^check "'` gave 81; the real number is 89, because 340
+  has INDENTED `check` calls inside an `if` and a `for`, and the eight it missed are
+  exactly the unreadable-source block. On that bad count one python arm looked like a
+  genuine gap, and a duplicate of it was written and proven to discriminate before
+  the duplication was noticed. The tell was a duplicate check name -- the ledger tool
+  reported "one ledger row covers 2" -- and the right response to a name that already
+  exists is to ask why, not to rename it. Anchor a check sweep at `^[[:space:]]*`.
 
 - **The one permitted cross-reference, named as the rule asks.**
   `test_the_two_fingerprint_implementations_cover_the_same_inputs` asserts that the

test/pytest/TESTS.md

@@ -498,8 +498,6 @@ each leaving the module well-formed, restored byte-for-byte afterwards:
 The two that redden both are the two whose effect reaches the filesystem. The three
 that redden only the source arm are exactly the properties the behavioural arm cannot
 see, which is why they are written down separately rather than folded into it.
-| `test_the_stamp_writer_reports_failure` | `\|\| true` made both controllers' warnings unreachable |
-| `test_two_installations_of_one_major_do_not_share_a_stamp` | the key names the installation, not just the major |
 | `test_moving_bytes_between_files_moves_the_shell_fingerprint` | the digest sees a repartition |
 | `test_the_two_fingerprint_implementations_cover_the_same_inputs` | **the two implementations move on the same edits** |
 
@@ -604,9 +602,8 @@ itself. The subject is the instrument every other arm in this section depends on
 if the fingerprint can be wrong, `never report on source you did not build` reports
 on nothing.
 
-`test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one` and
-`test_a_failed_digest_gives_unverified_and_never_a_false_stale` are the two arms
-here, and THE MECHANISM CHANGED WITH THE IMPLEMENTATION. They used to drive the real
+`test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one` is the arm
+here, and THE MECHANISM CHANGED WITH THE IMPLEMENTATION. It used to drive the real
 shell function with a **stub `md5sum`** on `PATH`, because the shell forked one
 per file. The digest now lives in `test/pgc_fingerprint.py` and uses `hashlib`,
 which no `PATH` can reach, so the stub would have left both arms green while
@@ -621,13 +618,22 @@ rewritten:
 
 So the tree is built outside any mode-0700 directory and read by a second user,
 and where no such user exists the arm records `expect.cannot_run` rather than
-passing. `test_one_tree_hashes_one_way_however_the_locale_is_set` pins the defect
+passing.
+
+**The verdict that follows is the shell harness's property, and it is asserted
+there (#432).** `pgc_freshness_verdict` is pure shell, so an arm here could only
+reach it by driving `lib.sh` — which is the coupling the two-harness rule removes.
+`test/selftest/340` holds it, and holds more of it than this corpus did: it loops
+over two unreadable files rather than one, and it carries the premise this side
+lacked, that the unprivileged read AGREES with the privileged one while nothing is
+denied. Without that premise the arms measure the user switch rather than the
+permission denial. `test_one_tree_hashes_one_way_however_the_locale_is_set` pins the defect
 the single implementation removed on the way: `sort -z` used locale collation and
 nothing pinned a locale, so one tree hashed two ways —
 `LC_ALL=C` gave `6d122a7158d5` and `LC_ALL=en_US.UTF-8` gave `0b59bd75fa4f`.
 
-`test_a_failed_digest_gives_unverified_and_never_a_false_stale` is the property
-that matters. `stale` is the FATAL; `unknown` prints `freshness UNVERIFIED` and
+The verdict is the property that matters, and the reason it is worth an arm at all
+is the asymmetry. `stale` is the FATAL; `unknown` prints `freshness UNVERIFIED` and
 runs the suites. The asymmetry is the whole argument for the change: a false
 UNVERIFIED costs a line of output, a false FATAL costs a matrix **and** teaches
 people to re-run past a freshness check, which is the failure this controller
@@ -701,14 +707,17 @@ requires the diff to name the file rather than report that something changed.
 — a tree-relative path and a 32-character digest, never an absolute path, because
 an absolute path in the digest is the spelling defect returning by another route.
 `test_the_fingerprint_is_the_hash_of_the_manifest` is the arm that keeps the two
-from drifting, and `test_an_empty_manifest_is_reported_as_empty_not_as_silence`
-covers the case the report exists for.
-
-`test_the_fatal_report_can_be_run_rather_than_grepped_for` exists because the
-alternative was asserting that the source calls the function, which is the shape
-this suite refuses everywhere else. The report is a function so an arm can drive
-it, and the empty case says `(empty -- nothing under ...)` rather than printing
-nothing, because a silent empty dump reads as *the manifest was fine*.
+from drifting.
+
+**`pgc_freshness_report` is the shell harness's, and `test/selftest/340` asserts it
+(#432).** Two arms here used to: one that the report names each hashed file and
+states how many, and one that an empty manifest says `(empty -- nothing under ...)`
+rather than printing nothing, because a silent empty dump reads as *the manifest
+was fine*. Both drove `lib.sh` to reach a pure-shell function, which is the
+coupling the two-harness rule removes, and 340 already held both properties. The
+reason they exist is worth keeping even though the arms moved: the alternative was
+asserting that the source CALLS the function, which is the shape this suite refuses
+everywhere else, so the report is a function precisely so that an arm can drive it.
 
 ### The suite that wrote into the tree the other suites were reading
 

test/pytest/test_build_refusal.py

@@ -570,31 +570,6 @@ def _tree_with_module(tmp_path, name):
     return t
 
 
-def test_the_stamp_writer_reports_failure(tmp_path, expect):
-    """`|| true` made both controllers' warning branches unreachable."""
-    out, rc = _sh('pgc_write_source_stamp "/proc/pgc-twin" "deadbeef"')
-    expect.num(rc, 1, "the writer reports failure on an unwritable target")
-    ok, rc2 = _sh(f'pgc_write_source_stamp "{tmp_path}/s" "cafebabe"')
-    expect.num(rc2, 0, "control: and succeeds on a writable one")
-
-
-def test_two_installations_of_one_major_do_not_share_a_stamp(tmp_path, expect):
-    """The stamp key must name the installation, not only the major."""
-    cfgs = []
-    for n in ("a", "b"):
-        c = tmp_path / f"pg_config.{n}"
-        c.write_text('#!/bin/sh\ncase "$1" in\n'
-                     '  --version) echo "PostgreSQL 18.4" ;;\n'
-                     f'  --pkglibdir) echo "/usr/local/pg18{n}/lib" ;;\nesac\n')
-        c.chmod(0o755)
-        cfgs.append(c)
-    a, _ = _sh(f'pgc_source_stamp_path /tree "{cfgs[0]}"')
-    b, _ = _sh(f'pgc_source_stamp_path /tree "{cfgs[1]}"')
-    expect.text(str(a != b), "True", "two prefixes of one major get different stamps")
-    a2, _ = _sh(f'pgc_source_stamp_path /tree "{cfgs[0]}"')
-    expect.text(a2, a, "control: the same pg_config twice gives the same path")
-
-
 def test_moving_bytes_between_files_moves_the_shell_fingerprint(tmp_path, expect):
     """`xargs -0 cat | md5sum` could not see a repartition."""
     t = _tree_with_module(tmp_path, "rp")
@@ -712,15 +687,6 @@ def _unprivileged_user():
     return None
 
 
-def _sh_fp_as(user, expr):
-    """Evaluate a lib.sh expression as USER ("" means this process)."""
-    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'
-    argv = ["bash", "-c", script] if not user else \
-           ["runuser", "-u", user, "--", "bash", "-c", script]
-    p = subprocess.run(argv, capture_output=True, text=True)
-    return p.stdout.strip(), p.returncode
-
-
 def _readable_tree(name):
     """A fingerprintable tree an unprivileged user can traverse.
 
@@ -780,34 +746,6 @@ def test_a_failed_digest_yields_no_fingerprint_rather_than_a_wrong_one(expect):
         shutil.rmtree(root, ignore_errors=True)
 
 
-def test_a_failed_digest_gives_unverified_and_never_a_false_stale(expect):
-    """The property that matters. `stale` is the FATAL; `unknown` is UNVERIFIED.
-
-    A false UNVERIFIED costs a line of output. A false FATAL costs a matrix and
-    teaches people to re-run past a freshness check, which is the failure this
-    controller exists to prevent.
-    """
-    user = _unprivileged_user()
-    if user is None:
-        expect.cannot_run("MISSING_DEPENDENCY",
-                          "no non-root user to read as; root ignores chmod 000")
-        return
-    root, t = _readable_tree("v")
-    try:
-        base, _ = _fp_cli(t, user=user)
-        expect.at_least(len(base), 12, "premise: the tree fingerprints at all")
-        (t / "src" / "b.c").chmod(0o000)
-        verdict, _ = _sh_fp_as(
-            user, f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"')
-        expect.text(verdict, "unknown", "a failed digest reads as unknown, not stale")
-        (t / "src" / "b.c").chmod(0o644)
-        healthy, _ = _sh_fp_as(
-            user, f'pgc_freshness_verdict "{base}" "$(pgc_source_fingerprint "{t}")"')
-        expect.text(healthy, "fresh", "control: a readable run still reads fresh")
-    finally:
-        shutil.rmtree(root, ignore_errors=True)
-
-
 def test_one_tree_hashes_one_way_however_the_path_is_spelled(tmp_path, expect):
     """`${f#"$dir"/}` strips a prefix that must match character for character."""
     t = _fp_tree(tmp_path, "s")
@@ -957,26 +895,6 @@ def test_an_added_file_is_named_rather_than_merely_changing_the_hash(tmp_path, e
                 "and it names the file that appeared")
 
 
-def test_the_fatal_report_can_be_run_rather_than_grepped_for(tmp_path, expect):
-    """A dump nobody can run is a dump nobody knows is empty."""
-    t = _mf_tree(tmp_path, "rep")
-    out, _ = _sh_fp(f'pgc_freshness_report "{t}"')
-    expect.num(len([l for l in out.splitlines() if l.strip().startswith("| src/a.c ")]), 1,
-               "the report names each hashed file")
-    expect.num(len([l for l in out.splitlines() if "(6 files, under" in l]), 1,
-               "the report states how many files it hashed")
-
-
-def test_an_empty_manifest_is_reported_as_empty_not_as_silence(tmp_path, expect):
-    """A silent empty dump reads as "the manifest was fine", which is the failure
-    this report exists to end."""
-    hollow = tmp_path / "hollow_report"
-    hollow.mkdir()
-    out, _ = _sh_fp(f'pgc_freshness_report "{hollow}"')
-    expect.num(len([l for l in out.splitlines() if "empty -- nothing under" in l]), 1,
-               "an empty manifest says so")
-
-
 def test_no_selftest_part_writes_into_the_live_source_tree(expect):
     """A suite that runs beside others must not write into the tree they read.
 

test/pytest/test_harness_deps.py

@@ -944,8 +944,9 @@ def test_the_job_installs_no_database_driver(expect):
         "sources the shell harness's helper library to call its build-and-install "
         "function, so the build refusal has one implementation rather than two",
     "test_build_refusal.py":
-        "sources that library for the pure-shell stamp and freshness helpers, and "
-        "for the one permitted cross-implementation arm; see its module docstring",
+        "sources that library twice and for two reasons only: the one permitted "
+        "cross-implementation arm, and a historical-parity arm whose fixture is its "
+        "own. The pure-shell stamp and freshness drivers are gone; see CONTEXT.md",
     "test_suite_accounting.py":
         "reads the matrix runner's text and executes the real runner",
     "test_mutation_ledger.py":