Skip to content

docs: the two test harnesses are parallel, not coupled - #932

Merged
jdatcmd merged 4 commits into
mainfrom
docs/harness-independence
Sep 10, 2026
Merged

docs: the two test harnesses are parallel, not coupled#932
jdatcmd merged 4 commits into
mainfrom
docs/harness-independence

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

The owner's rule, 2026-09-10: the shell tests and the python tests are parallel in functionality but must not call, import or reference each other. Documentation is the only exception — prose, comments and docstrings may name the other harness freely.

Why it belongs next to the twin rule

CONTEXT.md already requires every new test to be written twice, and gives the reason: "where they disagree, one of them is wrong, and that is worth finding at the time".

That reason only holds if the two are two measurements. A pytest test that drives test/lib.sh by subprocess is not a second measurement of the property — it is the first measurement wearing a Python wrapper. It agrees with the shell by construction, and it can never report the shell wrong. The coupling turns the twin from evidence into a mirror, which is the thing the twin rule exists to avoid.

So each harness asserts the property against the product, in its own terms, never against the other harness's implementation.

The line between a reference and a fixture

Building a throwaway tree that merely resembles the other harness is not a reference to it. test_build_refusal.py writes a fake lib.sh into a tmp_path and drives that; it never touches the real one, so it is not debt and the document says so. Sourcing the real test/lib.sh is a reference.

The debt this starts with, measured

Counted with comments and docstrings stripped, so these are executable references rather than mentions:

direction files sites
python drives shell 2 13
shell takes python as its subject 7 27

This PR fixes none of that, and says so in the text. It records which direction those files are expected to move, and makes the count falsifiable instead of leaving a vague sense that some coupling exists.

Two of those numbers were wrong in my first draft and are corrected here: test_suite_accounting.py is 12 executable sites on main, not the 24 a looser pattern reported, and test_check_results_are_machine_readable.py is not on main at all — it arrives with #923. The document notes the sweep trap that caused it: [a-z_]+\.sh matches sharedir, and reported two pg_config --sharedir calls as violations.

What this does not do

It adds no guard. A rule nothing enforces is a comment, and this repository says so in several places — but a sweep added today would fail on 40 existing sites, so the guard belongs with the work that pays the debt down, not with the sentence that names it.

Docs only: no code, no test, no CI change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

Owner's rule, 2026-09-10: the shell tests and the python tests are parallel in
functionality but must not call, import or reference each other. Documentation
is the only exception.

It follows from the twin rule already in this section. That rule's stated point
is that "where they disagree, one of them is wrong". Two harnesses can only
disagree if they are two measurements, and a pytest test that drives lib.sh by
subprocess is not a second measurement -- it is the first one wearing a Python
wrapper. It agrees with the shell by construction and can never report the shell
wrong, which turns the twin from evidence into a mirror.

The starting debt is measured rather than asserted, with comments and docstrings
stripped so the counts are of executable references: 13 sites in 2 python files
reach into shell, and 27 lines in 7 shell files take python as their subject.
A fixture that merely resembles the other harness is not a reference to it, so
test_build_refusal.py's fake lib.sh under tmp_path is excluded and said to be.

The paragraph fixes none of that. It records which direction those files are
expected to move, and makes the count falsifiable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rule is right and I have already acted on it#927 had a shell part whose whole subject was a python module's source text, and I deleted it rather than repairing it on the strength of this. "Each harness asserts the property against the product, in its own terms, never against the other harness's implementation" is a better statement of it than I managed, and the reasoning from the twin rule — that a coupled twin agrees by construction and can never report the other wrong — is the part that makes it more than a style preference.

Requesting changes on one thing: the file the document uses to define the line is on the wrong side of it.

test_build_refusal.py does touch the real lib.sh

The body says:

test_build_refusal.py writes a fake lib.sh into a tmp_path and drives that; it never touches the real one, so it is not debt and the document says so.

The first clause is true — _fake_tree at line 68 writes one. The second is not. Three helpers source the real tree, on main:

377:    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'     _sh
486:    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'     _sh_fp
538:    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'     _sh_fp_as

372: SRCDIR = pathlib.Path(__file__).resolve().parents[2]

SRCDIR is the source tree, not a fixture, so by the document's own line — "Sourcing the real test/lib.sh is a reference" — this is a reference, three times over, with 10 _sh( and 21 _sh_fp*( call sites behind them. So python → shell is at least 3 files rather than 2, and the example chosen to show what is not debt is the largest python→shell debt in the corpus.

That matters more than the arithmetic, because the sentence is what a reader will use to classify the next file.

A defect the check turned up, which is not this PR's doing

_sh takes a tree and discards it:

def _sh(srcdir, expr):
    """Evaluate one lib.sh expression against a tree, and return its stdout."""
    script = f'. "{SRCDIR}/test/lib.sh" || exit 1; {expr}'

srcdir is never read. Callers pass tmp_path_sh(tmp_path, 'pgc_write_source_stamp ...') at 396, 398, 412, 413 — so the call reads as "evaluate against this tree" and the docstring promises it, while the sourcing is hardcoded to the real one. Whatever those arms are measuring, it is not parameterised by the tree they hand it. Worth a separate issue rather than this PR.

What I could not fault, and where my own counts disagree

I tried to verify the inventory independently and my first pattern reproduced exactly the looseness you had already found and corrected: 24 sites in test_suite_accounting.py, which is the number you say a looser pattern reported. That is a good sign for the document — the trap it names is the trap I fell into.

With a tighter definition I get different totals from yours in both directions (python → shell 4 files / 8 sites; shell → python 4 files / 29 sites, with 17 of those in selftest/350 where a test_[a-z_]+\.py pattern matches the corpus file names in its data lists). I am not asserting your numbers are wrong — mine disagree with each other depending on where I draw "reference", which is the whole difficulty. What I would add is the counting rule itself, in the document, next to the numbers: the totals are only falsifiable if a reader can re-derive them, and three definitions give three answers.

CONTEXT.md +42/-0, no code, no CI change, and "a sweep added today would fail on 40 existing sites, so the guard belongs with the work that pays the debt down" is the right call — a rule with a guard that is immediately suppressed teaches people to suppress guards.

What I am not claiming

I did not check the selftest/040, selftest/030 or lib.sh entries in your shell→python list; my pattern found none of them, which probably means your definition catches something mine does not rather than that they are wrong.

@OffgridwithJD checked the inventory instead of believing it and found the
example chosen to show what is NOT debt is the biggest debt in the list.
test_build_refusal.py does both: it writes a fake test/lib.sh under a tmp_path
and drives that, and it also sources the REAL one in three helpers -- _sh at
377, _sh_fp at 486, _sh_fp_as at 538 -- behind 39 call sites. The first draft
read the fake tree, stopped there, and called the file clean.

So the section now states the counting rule beside the numbers, which is what
the reviewer asked for: a total nobody can re-derive is an assertion, not a
measurement. Three different patterns gave three different line totals while
this was being settled, including one of mine that reported 24 sites in
test_suite_accounting.py and another that reported 2. The unit is therefore the
FILE, and each file is named with the mechanism that makes it a reference --
which is also what has to change for it to stop being one.

The inventory is 4 python files and 7 shell files. test_build_refusal.py is kept
as the worked example, correctly this time, because one file doing both is
exactly where the fixture/reference line has to be drawn.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at b82dd6b8. The finding is fixed and the thing I actually asked for is in: the counting rule now sits beside the numbers, so the inventory can be re-derived instead of believed.

The fix is better than the correction I asked for

I asked for test_build_refusal.py to be moved to the right side of the line. It is kept as the worked example and correctly classified, with the reason stated — "it does both" — and that is the more useful outcome, because one file writing a fake tree and sourcing the real one is exactly where the line has to be drawn. "Judge a file by what it executes, not by the fixture it builds" is the sentence a reader needs.

What I re-derived, with your rule

The seven shell files check out exactly. Every one has executable lines naming the corpus, comments stripped:

lib.sh          5     selftest/350   17     selftest/370    2
selftest/030    1     selftest/360    3     selftest/380    8
selftest/040    3

"Count files, not lines" is the right unit and your reason for it is the one I hit myself: three patterns gave me three totals, and my first pattern reproduced the 24 you had already corrected to 12. A line total is a claim about a regex; a file list is a claim about the tree.

Two small things, neither blocking

The 39 call sites read as 36 to me. By AST, counting calls to the three helpers and excluding their definitions:

_sh 9 · _sh_fp 20 · _sh_fp_as 7  ->  36 calls, 3 definitions, 39 occurrences

So 39 is right if the definitions are included and 36 if they are not. Worth a word either way, since the document's whole argument is that a number should be re-derivable — and this is the one number left in it that needs a convention a reader cannot see. The file-level claim is unaffected.

"4 python files" includes one not on main. test_check_results_are_machine_readable.py arrives with #923, which you say plainly in the entry — so it is transparent rather than wrong, but the heading reads as a count of the tree on 2026-09-10 and the list is a count of the tree plus one open PR. Three on main today.

Acted on, not just agreed with

The rule already changed two of my PRs:

  • #927 had a shell part whose whole subject was a python module's source text, checking a scanner by grepping it. I deleted it rather than repairing it, after listing all 55 of its check names against the corpus's arms so the removal was not a silent loss. It merged at eb7d9783.
  • #921 was worse: its additions to selftest/350 invoked the decider, python3 "$_hd_decide" --disagree. Both blocks are gone, with the same coverage check — every property they tested already exists in the corpus. Net new cross-harness references against main: the ci.yml job-name grep, which belongs to neither harness, and three synthetic strings in the anchor fixtures, which are rule 2.

And #933 is filed for _sh(srcdir, expr) never reading srcdir, as you asked — with no suggested patch, because which of the two readings was intended decides the fix and whoever wrote those arms knows it.

One thing I would not change

"None of that is fixed by this section", and the argument that a sweep added today would fail on 40 sites so the guard belongs with the work that pays the debt down. A rule whose guard is immediately suppressed teaches people to suppress guards. The SHA-pinning note at the end is the same instinct and is worth as much as the rule.

jdatcmd and others added 2 commits September 10, 2026 13:13
Two corrections from @OffgridwithJD's review, and the first one is the
document's own rule 3 catching the document.

"39 call sites" was 36 calls plus the 3 definitions. The pattern `[^_a-z]_sh(`
matches `def _sh(` as readily as a call, which is the same class of error as
`[a-z_]+\.sh` matching `sharedir` -- already written three lines above as the
thing not to do. Counted with ast now, and the entry says how, because a number
in this section has to be re-derivable or it does not belong here.

The heading said 4 python files. Three are on main; the fourth arrives with
PR #923. The entry always said so, the heading did not, and a reader who stops
at the bold line gets a count that is wrong today.

Recounted against main at aa53c1b, after #927 and #931 landed: still 3 python
files and 7 shell files. test_raises_sqlstate.py, new on main, adds neither --
it drives pytester, not the shell.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 142cd9fc, because my earlier approval named b82dd6b8 and this repository does not dismiss stale reviews — which is the hazard I raised on #923 an hour ago, so it would be poor form to leave one of my own standing on a moved head.

Both notes are addressed, and the second is better than what I asked for.

36, with the reason it was 39. "Counted with ast, not grep: the pattern [^_a-z]_sh( also matches def _sh(, which is how the first draft said 39 — 36 calls plus the 3 definitions." I re-derived it independently and get the same split:

_sh 9 · _sh_fp 20 · _sh_fp_as 7  ->  36 calls, 3 definitions

Recording why the number moved is worth more than the corrected number, and naming it as an instance of the document's own rule 3 — a pattern matching something that merely looks like the thing — is the right place to put it. A reader who hits the same trap next week now has the answer in the paragraph where they will be standing.

"3 python files and 7 shell files, with a fourth arriving in PR #923" splits the tree from the queue, which is what the heading needed: the inventory is dated, and a dated count should describe the tree on that date.

The rest is unchanged from what I read at b82dd6b8, and main is merged in, so the diff against the previous head is mostly #927 and #931 arriving rather than anything new here: CONTEXT.md ±8 is the whole of this push's own content.

Nothing else from me. The rule, the counting rule, the file-level unit with its justification, the worked example that does both, and the deliberate absence of a guard until the debt is paid down — all stand as reviewed.

@jdatcmd
jdatcmd merged commit 110e676 into main Sep 10, 2026
12 checks passed
jdatcmd added a commit that referenced this pull request Sep 10, 2026
…new file

#921 and #932 landed. #921 brings the classifier that decides which files the
driver-free job runs, and it has an opinion about this branch's new file that
this branch could not have had when it was written.

TESTS.md: main's #921 inserted two file sections at 15 and 16, so the tail
sections moved again and this branch's section becomes 22. Same resolution as
last time and for the same reason -- main's convention is to append a new file
section after the tail -- so one section of this branch is renumbered rather
than seven of main's. Checked: 22 headings against 22 TOC entries, every TOC
text equal to its heading, every anchor equal to what GitHub derives, numbering
contiguous 1..22.

NO_CLUSTER gains test_check_results_are_machine_readable.py, because the
composed tree failed without it:

    NO_CLUSTER is exactly the database-free half of the corpus:
    got '[1: undeclared:test_check_results_are_machine_readable.py]' want '[]'

It drives test/lib.sh by subprocess, so what it needs is bash and the tree, not
psycopg or a cluster. Measured rather than assumed: 9 passed in a venv with no
driver, and the classifier agrees it is not driver-dependent. The derived job
goes from 9 files to 10, 161 passed to 170.

That failure is only visible where a cluster and the driver are both present,
and no CI job runs test_harness_deps.py's membership arms, so it would have
landed on main as a red nobody ran. Composing locally is what found it.

On the composed tree: selftest 350 53 checks 0 failed, selftest 400 64 checks
0 failed, selftest 080 15 checks 0 failed, the driver-free job 10 files 170
passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 10, 2026
…epipe sweep branch

# Conflicts:
#	CHANGELOG.md
jdatcmd added a commit that referenced this pull request Sep 10, 2026
#923 moved five times while this waited, and #921, #927, #930, #931 and #932
landed on main underneath it. Composing found two things that a clean merge
would not have.

TESTS.md: the base now carries 22 sections, and this branch had inserted the
ledger at 16. Auto-merge kept the base's tail sections AND this branch's copies
of them, so the file would have had two of each. Resolved by keeping only the
ledger section from this side, renumbered to 23, where the base's own 22 already
ends. Checked rather than eyeballed: 23 headings against 23 TOC entries, every
TOC title equal to its heading, every anchor equal to GitHub's derivation,
numbering contiguous 1..23, no duplicate heading.

NO_CLUSTER gains test_mutation_ledger.py. #921's classifier arrived on the base
and immediately named it:

    membership_report: [1: undeclared:test_mutation_ledger.py]

It drives test/pgc_ledger.py, a python tool rather than the shell harness, so it
needs neither a cluster nor psycopg. Measured rather than assumed: 9 passed in a
venv with no driver, and driver_dependent() agrees. The derived job goes to 11
files and 179 passed.

THE LEDGER IS REGENERATED, AND THAT IS THE POINT OF THIS MERGE RATHER THAN A
SIDE EFFECT. #923 added 17 checks to selftest 400 and converted 25 skip sites,
none of which the committed ledger had ever seen. The gate refuses a check it
has never seen, so the composed tree would have failed CI for a reason with
nothing to do with either change. Regenerating is the documented repair, and the
budget file says so.

From a real run of the composed tree, not a synthesised log:

    harness_selftest.sh: PASSED, rc=0
    checks run: 735 | accounting: 735 passed + 0 failed + 0 unrunnable + 0 skipped
    ledger: 701 rows -> 734 | never=734, ever red=0
    gate: new this run=0

Reconciled: 735 records == 732 distinct (suite, part, name) + 3 names that each
appear twice in one run, and 0 log triples are missing from the ledger, which is
exactly what the gate refuses. All 734 rows carry five fields and none ends in a
tab. The budget's asserted census follows to 734.

Two ledger rows do not appear in this log -- selftest 330's "all three runner
functions", which #923 changed to five. rename-scan reports appeared=0,
vanished=2. THEY ARE LEFT DELIBERATELY: this log is PG17 only, and pruning rows
that a single major did not produce would delete checks that legitimately run
elsewhere. The gate refuses unseen checks, not unused rows.

Gates on the composed tree: 350 53/53, 400 81/81, 410 96 checks 0 failed,
080 15/15, shellcheck rc=0 over the whole harness, driver-free job 11 files
179 passed, membership_report [].

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants