[OffgridwithJD]

Fixes #1321.

native_upgrade_converge.sh called pgc_setup, which arms trap pgc_on_exit EXIT, and then armed its own trap cleanup EXIT. Bash keeps one EXIT trap, so every run, passing or failing, left its PostgreSQL server listening and /tmp/pgcolumnar-test.* behind, and a run that died lost #1233's TERMINATED line.

Changes

• The fix: cleanup calls pgc_on_exit first, so the wrapper reads the suite's exit status before anything overwrites $?.
• Selftest part 620: reads every suite's text for an EXIT trap whose handler reaches neither pgc_on_exit nor pgc_teardown, in a suite that calls pgc_setup. Comments are stripped first. The handler is resolved as either a quoted command or a function body, and a one-line function stops at its own line, never running on into the next function's.
  • Before the population arm, the detector is driven on eight suites written for it: two that replace the trap (one on EXIT INT TERM), one that names the teardown only in a comment, one where only the next function has it, three that chain correctly, and one with no pgc_setup.
  • Captured text reaches grep and awk as here-strings (part 080).

Evidence

• Red first, on main at PG17: 1227 passed + 1 failed, the failure naming native_upgrade_converge.sh and nothing else.
• Green with the fix: harness_selftest 1228/1228 on each of PG15–19.
• Driven: on PG17, native_upgrade_converge is 21/21, its workdir is gone, no test server is left, and no fixture .pgcbak remains. Before the fix, each converge run left one server: four started within a second of a run's last log line (the fifth predates the logs I kept), and one once held a flock long enough to stall a selftest.
• Five detector mutations, each red on its arm: never reports, reads comments as code, reads past a one-line handler into the next function, accepts only pgc_on_exit, and sees EXIT only at the end of the line.
• Ledger: part 620's 5 checks. Four are dated: the population arm by the pre-fix run, the detector arms by the mutations. The premise about lib.sh arrives never. The census goes 1686 → 1687 on 2008 rows, and gate passes (rc=0). test_mutation_ledger.py 39 passed; docs_style PASSED.

The 17 suites that chain pgc_teardown rather than pgc_on_exit are left alone here; the question about them is on #1321.

🤖 Generated with Claude Code

[jdatcmd]

Review of f51ca44. The fix is right, and I measured it: on PG17, native_upgrade_converge is 21/21 from this head, and the box had 5 stale /tmp/pgcolumnar-test servers and 3 workdirs before the run, and the same 5 and 3 after. A pre-fix run would have added one of each. The five leftovers are probably from earlier converge runs, mine or yours, before this fix. Part 620's detector design is good too: comments stripped, handlers resolved as commands or function bodies, a one-line handler stopping at its own line, and eight built suites driven before the population arm.

One thing blocks, the owner's twin rule: every test has a .sh twin and an independent pytest twin, and neither calls or imports the other. Part 620 is shell only. The precedent is #1286, where selftest parts 600 and 610 got test_capability_decline.py as their twin, written with pytest's own machinery and reading no shell harness.

For 620, a twin could parse each suite with its own Python reader:

• find the trap ... EXIT lines, resolve the handler, and require pgc_on_exit or pgc_teardown in a suite that calls pgc_setup;
• drive the same eight constructed suites, each written by the test from its own strings;
• run the population arm over the real tree.

Same check names, so compare_to_bash can grade it if you add the pair. Cluster-free, so it goes in NO_CLUSTER with a TESTS.md section.

Everything else I'd approve as is.

[OffgridwithJD]

The twin is in at 1ec4fe6, one commit on top of f51ca44. Nothing else changed.

test_suite_exit_trap.py reads the suites and lib.sh as text and never sources or runs either. It doesn't import the part, and its reader is its own:

• trap lines are split with shlex;
• comments are removed by the shell's rule: a # that starts a word, outside quotes, with backslash escapes. The part's rule is everything after #;
• a one-line handler is that one line, and any other runs to its closing brace.

It writes the same eight suites from its own strings and drives them before the population arm. The five check names match the part's.

Evidence

• Red first: run against 629f5ec (the tree before the fix), it fails only the population arm: got '[native_upgrade_converge.sh]' want '[]'.
• Five mutations of its reader, each reddening the same arms as the part's matching mutation:
  • never reports → the flag and comment/next-function arms;
  • keeps comments → the comment/next-function arm;
  • reads past a one-line handler → the comment/next-function arm;
  • accepts only pgc_on_exit → the chains arm and the population arm (17 suites chain pgc_teardown);
  • EXIT only last on the line → the flag arm.
• Registries: the file is in NO_CLUSTER; guard_tests 415 → 419 by collection, and the guard half passes 419/419. TESTS.md has section 97. docs_style passes.

Not paired in compare_to_bash. That file derives pairs as test/<stem>.sh ↔ test_<stem>.py, in both the declared-set arm and the grading loop, so a selftest part has no stem it can pair with. test_capability_decline.py isn't paired for the same reason. If you'd rather have selftest parts graded, that's a change to compare_to_bash's population, and I'd do it as its own issue.

TESTS.md 97 collides with #1319's 97. Whichever merges second renumbers.

[jdatcmd]

Approved at 1ec4fe6. The twin is what the rule asks for: test_suite_exit_trap.py reads lib.sh and the suites as text with its own Python reader, with no subprocess, bash or shell helper.

Run here:

• on this head: the twin and the relevant test_harness_deps tests give 6 passed.
• red on main, with the twin copied onto origin/main, where converge still has the bug: no suite's own EXIT trap leaves its cluster running: got '[native_upgrade_converge.sh]' want '[]', and the three detector tests pass. It fails for exactly the right reason.

Together with my earlier measurement (5 stale servers before a converge run and 5 after), this is good to merge once CI is green.

[jdatcmd]

For the record: my approval landed on 9df6615, pushed while I was testing 1ec4fe6. I've since read it. It's one entry in test_harness_deps.py's independence inventory, declaring that the twin reads shell text as data, which is correct. So the approval stands.

The PR is now DIRTY against main after #1319 merged, so CI hasn't run. Once you rebase, I'll compare each file's +/- lines against 9df6615 and merge when CI is green.

[OffgridwithJD]

Rebased onto 024e737 (after #1319). Head is now 12bec19, which replaces 9df6615.

• Per-file: every file's +/- lines are identical to 9df6615's against its base, except two restatements.
• The budget, restated from main: 1698 → 1699, rows 2059 + 5 = 2064, dated 361 + 4 = 365.
• TESTS.md: the section renumbered to 98, after feat: incremental rollups, phase 1 (append-only) (#1294) #1319's 97. Its text is unchanged.
• Ledger: git merged it cleanly. I checked that the result minus part 620 is byte-identical to main's, and that the 5 rows for part 620 are byte-identical to 9df6615's. gate rc=0.
• Guards: the guard half passes 419/419, and docs_style passes.

[jdatcmd]

Re-approved at 12bec19. I checked each file's +/- lines against 9df6615 myself (base b6656ac, versus 024e737 now). Everything is identical except check_ledger_budget.txt and TESTS.md, the two restatements you named. Merging when CI is green; #1311 also takes TESTS.md section 98, so whichever merges second renumbers.

[jdatcmd]

@OffgridwithJD, I merged #1311 first; both were green and approved. This now needs one more rebase: TESTS.md section 99 and the budget restated. I'll re-approve by the per-file comparison as before.

[OffgridwithJD]

@jdatcmd I rebased onto f25863b (the #1311 merge). The head is now c18d020.

Per-file patch identity against the head you approved (12bec19 on 024e737). Eight of the ten files are byte-identical. The other two:

• test/check_ledger_budget.txt: both entries are kept. docs, test: gap-fill, LOCF and interpolation recipes, held to a reference (#1296 phase 1) #1311's goes first, then this PR's, restated on top of it as 1700 → 1701, with ledger rows 2069 + 5 = 2074, never 1700 + 1 = 1701, and dated 369 + 4 = 373. The census of the merged ledger agrees: 2074 rows, 1701 never, 373 dated.
• test/pytest/TESTS.md: test_timeseries_recipes.py keeps 98, and test_suite_exit_trap.py becomes 99, both in the index and in the section. The text of the section is unchanged.

Checked on the rebased tree:

• The gate, given the five clean harness_selftest logs and the SUITES of this tree, returns rc=0: 279 registered, 247 not covered (at the ceiling), and the census agrees.
• The guard half, run as CI runs it, gives 419 collected and 419 passed. The cluster half collects 513, which is main's count; this PR adds no cluster test.

#1311 touched no selftest part, so part 620 and its manifest line are as you approved them.

[jdatcmd]

Re-approved at c18d020. My own per-file check against 12bec19: every file's +/- lines are identical except check_ledger_budget.txt and TESTS.md, which are restatements. The budget's 1701 matches the count of never rows in this head's ledger. Merging when CI is green.

[jdatcmd]

Merging now: approved at c18d020, CI green.