test: an absence claim needs a premise, and a refusal needs its SQLSTATE (#1033) - #1034
Conversation
…ATE (commandprompt#1033) Two checks could not fail when their probe broke, because the probe SYNTHESIZED the value the check expected. A UNIQUE CONSTRAINT DECIDED BY AN EXIT CODE. native_recluster.sh read "unique still enforced" from `psql ... && echo no || echo yes`, which reports yes for every failure psql can have, with `>/dev/null 2>&1` discarding the message so nothing else could tell either. Measured on the identical expression: a missing table, a wrong port, psql absent from PATH and a syntax error all produced yes. THE CONTRAST, RUN ON PG 18 rather than argued. The same mutation -- point the probe at a table that does not exist -- against both versions of the arm: main's arm PASS "unique still enforced" suite PASSED, rc=0, 12 passed this arm FAIL got [42P01] want [23505] suite FAILED, rc=1 The arm is also observable in the other direction. Replace the unique index with a plain one and the duplicate INSERT succeeds; the arm goes red there too, so it tests the constraint and not merely the SQLSTATE plumbing. 23505 is ERRCODE_UNIQUE_VIOLATION and comes from the index, where 42P01, 42601 and a connection failure do not. arrow_import.sh and audit.sh already read SQLSTATE this way. A SWEEP WITH NO POPULATION. local_open_race_free.sh summed `grep -rc ... "$SRC"` through `awk END{print s+0}` and compared it with 0. grep -rc prints nothing for a path it cannot open, so the awk manufactures the 0. The three arms above it read $OBJ, not $SRC. Measured: SRC=src files=56 premise=yes arm=0 both agree SRC=/no/such/tree files=0 premise=no arm=0 the premise catches it, the arm passes The premise counts the lines the recursive grep emitted, which is exactly what the arm's awk sums over. NEITHER SUITE NEEDS A LEDGER ROW, which is why these two and not a third. check_ledger.tsv holds zero rows for native_recluster and local_open_race_free, so the gate cannot refuse a new check there. selftest/400's tree-wide piped-loop sweep has the same shape but lives in harness_selftest, which the ledger covers with 934 rows, and its detector is already premised at :549; only its glob population is not. That one belongs to a change carrying the five-major ledger merge. Verified on PG 18 in the container: local_open_race_free.sh PASSED 11 checks, native_recluster.sh PASSED 12 checks, both mutations red, main's arm green under the same mutation. docs_style.sh PASSED (11 checks). The CHANGELOG's long-sentence count is unchanged at 857. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
linuxhikerpm
left a comment
There was a problem hiding this comment.
Approving ae008c237a667d7bf7ac9e4ed14cdc4d9c28ae70.
|| echo yes is the same shape as the collate-mismatch arm: every failure produces the value the check wants. Reading 23505 the way arrow_import.sh already does, with VERBOSITY=sqlstate, distinguishes the unique index from 42P01. The missing-table mutation going green on main and red here is the proof.
The recursive-grep premise counts grep -rc lines, which is what the arm's awk sums, so the two cannot drift. Leaving selftest/400 for a ledger-bearing change is the right cut: these two suites have zero ledger rows.
CI is green. No TESTS.md or expected_tests.txt collision. I will not merge it.
jdatcmd
left a comment
There was a problem hiding this comment.
Approving. The exit-code arm is exactly the shape CLAUDE.md warns about, one level cruder than the text greps, and your both-directions proof is what makes the replacement worth having rather than merely stricter.
Verified independently
native_recluster.sh:78-81 on main:
"$(... psql ... -c "INSERT INTO n VALUES (1, 0, 0, 'x');" >/dev/null 2>&1 && echo no || echo yes)" "yes"Driven on the idiom itself:
insert succeeds -> no
ANY failure at all -> yes
nonexistent command -> yes
The arm asserts yes, so every failure mode satisfies it. >/dev/null 2>&1 then throws away the only thing that could have discriminated. Your four measured rows — no such table, wrong port, psql absent, syntax error — are all the same fact.
This is worse than the five text-grep suites I measured today. A grep for permission denied for table at least constrains the message to a family. || echo yes constrains nothing at all: it is satisfied by the probe never reaching a server.
The both-directions proof is the part I would keep
baseline rc=0 unique-arm PASS
unique -> plain index rc=1 unique-arm FAIL
probe -> missing table rc=1 unique-arm FAIL got [42P01] want [23505]
The first mutation is the one that matters. Tightening an arm to a SQLSTATE can quietly turn it into a test of the SQLSTATE plumbing — green whenever something raises 23505, blind to whether the constraint is still there. Replacing the unique index with a plain one so the duplicate INSERT succeeds proves it still tests the constraint. Without that row I would have asked for it.
23505 is ERRCODE_UNIQUE_VIOLATION and comes from the index; 42P01, 42601 and a connection failure do not. Following arrow_import.sh and audit.sh's existing convention rather than inventing a helper is right.
And leaving the third gap out, with a measurement rather than a preference
That is the judgement I would most want made this way. A gap you have measured and decided not to close is a different object from one you did not notice, and the PR says which this is.
Merging.
Closes the two real gaps from #1033. The third is deliberately left out, and the reason is
measurable rather than a preference.
1. A UNIQUE constraint decided by an exit code
native_recluster.shread the verdict from psql's exit status:|| echo yescannot tell a unique violation from any other failure, and>/dev/null 2>&1throws the message away so nothing else can either. Measured on the identical expression:
The contrast, run on PG 18 in the container rather than argued. The same mutation --
point the probe at a table that does not exist -- against both versions of the arm:
And it is observable in the other direction too, which is the half a SQLSTATE change
can quietly lose. Replace the unique index with a plain one so the duplicate INSERT
succeeds, and the arm goes red there as well:
So it tests the constraint, not merely the SQLSTATE plumbing. 23505 is
ERRCODE_UNIQUE_VIOLATIONand comes from the index; 42P01, 42601 and a connection failuredo not.
arrow_import.shandaudit.shalready read SQLSTATE this way, so the helperfollows the existing convention.
2. A recursive sweep with no population
grep -rcprints onefile:countline per file and prints nothing for a path itcannot open, so
END{print s+0}manufactures the0the check wants. The three arms aboveit prove
opener_bodyworks, but they read$OBJ, not$SRC.The premise counts the lines the recursive grep emitted, which is exactly what the arm's
awksums over, so the two cannot drift apart.Why two and not three
selftest/400's tree-wide piped-loop sweep has the same shape, and is left alonedeliberately:
The ledger gate refuses a new check in a suite it covers, so the two suites here are free
and
selftest/400is not -- a new arm there needs a five-major run and a ledger merge.Its detector is also already premised:
:549proves it fires on a planted offence. Onlyits glob population is unproven, which is the weaker half. So it belongs to a change that
carries the ledger work, not to this one.
Verification
Every
RESULTline carried major18rather thanunknown, so the records are gate-shaped.Merge note
Touches three files: two
.shsuites andCHANGELOG.md. NoTESTS.md, noexpected_tests.txt, nocheck_ledger.tsv, so it carries none of the section-number orcount collisions that #1022, #1027, #1028 and #1029 have with each other. The CHANGELOG is
keep-both on conflict.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a