Skip to content

test: count a check and record it in one operation (#917) - #923

Open
jdatcmd wants to merge 1 commit into
feat/916-reconcile-accountingfrom
feat/917-machine-readable-results
Open

test: count a check and record it in one operation (#917)#923
jdatcmd wants to merge 1 commit into
feat/916-reconcile-accountingfrom
feat/917-machine-readable-results

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Closes #917 — phase 3 of #858. Stacked on #922; base is feat/916-reconcile-accounting, so review that one first.

What was wrong

Check results were prose. check, check_num and check_text printed PASS or FAIL and nothing else, so proving that a mutation reddened one named check meant grepping text. That is how every mutation proof in this repository is currently made: a person reading FAIL <name> out of a log and retyping it into a comment.

It is also how a reverted guard once reported plain green while the check count fell from 190 to 186 — the suite passed, and the only evidence anything had changed was a number nobody was comparing.

Why this is not a second emitter

A second source of truth for how many checks ran is the defect this family of issues exists to close.

lib.sh had eleven places that bumped PGC_CHECKS, each with its own outcome line beside it. Eleven chances to add a twelfth and forget the line — which is exactly what projections.sh's expect_fail did, with ten call sites, for as long as it existed, invisible because nothing reconciled the totals.

So counting a check and recording it are one operation. A helper cannot report an outcome without being counted, and cannot be counted without reporting one, because no code path does either alone. checks run: N and the N record lines are the same increment seen twice.

lib.sh:  11 sites bumping PGC_CHECKS  ->  1, inside pgc_record

The arm that holds it is structural, not behavioural: lib.sh may bump PGC_CHECKS in exactly one place, and that place must be pgc_record. That is what stops the next expect_fail from being written, rather than catching it after a year.

The record

RESULT<TAB>suite<TAB>check name<TAB>verdict<TAB>reason

Tab separated so a check name containing spaces survives. The reason carries #915's REASON_CODE, which is what makes this more than a reformat: an unrunnable check is distinguishable from a passing one without parsing prose.

A verdict pgc_record does not recognise is recorded as a FAIL, not dropped — dropping it would leave PGC_CHECKS bumped with no outcome recorded, which is the reconciliation pgc_summary already refuses.

The human lines did not move

DISPLAY is passed to pgc_record whole rather than composed inside it, and both harnesses pin the exact strings for check, check_text, check_num and check_unrunnable. 3,762 call sites, with suites, selftests and CI all grepping ^PASS and ^FAIL, is far past what a careful refactor can be trusted on.

PGC_SUITE is resolved once at load rather than per check: pgc_record runs at every one of those call sites, and a basename fork at each is 3,762 forks a suite does not need.

The runner reconciles the two artifacts

A suite's log states checks run: N and carries N records. One function produces both, so this cannot fail by drifting — but it can fail, which is why it is asserted: a suite killed mid-way, a truncated log, a helper that prints an outcome without recording it.

A log with no count at all never reached its summary. That is a different fault from a miscount and is reported as one, rather than reading as a clean reconciliation because there was nothing to compare against.

Only a suite that reached its summary has a count to reconcile — which is why this stacks on #916, for pgc_log_shows_accounting.

Evidence

selftest    exit=0  FAILs=0
            checks run: 535   accounting: 535 passed + 0 failed + 0 unrunnable = 535
            records:    535
            registered=251 | declares accounting=239, does not=12, absent=0 | sum=251

pytest      14 passed  (test_check_results_are_machine_readable.py + test_suite_accounting.py)
shellcheck  -S error -s bash test/*.sh test/selftest/*.sh   rc=0
docs_style  PASSED (9 checks)

The matrix is the load-bearing check here and it has not run yet. This change adds a line to the stdout of every check in every suite, so the two suites (PG N) jobs over all 251 suites are what proves it breaks nothing. I will report what they say rather than assume.

TESTS.md gains section 15; sections 15-17 renumbered to 16-18, with the table of contents verified against the headers programmatically (contiguous 1..18, ToC == headers).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

jdatcmd added a commit that referenced this pull request Sep 10, 2026
…s it (#486)

#923's `suites (PG 17)` went red on a pytest name that EXISTS, while PG 18 passed
the same commit:

    FAIL  every test the document names exists in the corpus:
          got [[1: test_the_partition_over_the_registered_suites_adds_up]]

The cause is the shape selftest 080 already forbids. Its membership test was
`printf '%s\n' "$ondisk" | grep -qxF "$name"`, and the selftest runs under
`set -o pipefail`: grep -q exits the moment it matches, printf takes EPIPE, and
pipefail reports the pipeline failed though the name WAS present. The name is
then recorded absent.

WHY IT SURVIVED, AND WHY IT SURFACED NOW. Selftest 080's sweep is deliberately
non-recursive and never looked inside test/selftest/ -- the directory scoping was
never a decision, it fell out of writing "$TESTDIR"/*.sh, exactly as the bench/
hole did before it. Three fragments held the forbidden shape: 350's corpus
membership test, 300's directory coverage test, 340's Makefile sweep.

At corpus size the writer is small enough to win the race on an idle machine,
which is why it passed for so long. Measured, 170 names over 400 trials:

    printf | grep -qxF     idle: 0 false absences    under load: 6
    grep -cxF <<<          idle: 0                   under load: 0

A four-way CI matrix is the loaded case, and adding names to the corpus made the
window wider. This is the third appearance of this bug class here after #473 and
#476, and the second today.

All three sites now use grep -c on a here-string, and the sweep covers
test/selftest/ with its own coverage arm, because 080 already records that a
conditionally added glob narrows silently and a file-count premise cannot see it.

THE EXEMPTION IS DERIVED, NOT LISTED. 080's own control is inside a quoted
heredoc, and so is any deliberate demonstration of the shape; a line inside one is
text being written to a file, not a pipeline the suite runs. Comment lines are
excluded for the same reason -- the rule's explanation, and the note beside each
site fixed here, necessarily spell the shape out. A filename allowlist would have
to be maintained, and this rule exists because things that must be maintained are
not.

Two defects in the exemption itself, both found by running it rather than reading
it. It printed NR where it meant FNR, so from the second file onwards it reported
line numbers from a running total and no key matched -- the neighbouring question
answered plausibly, since single-file runs agree because NR == FNR there. And it
kept heredoc state across inputs, so it now resets per file. Its own arms pin
both: the sweep sees both lines of a probe, the exemption covers the one inside
the heredoc and not the one above it.

The "did not swallow the corpus" premise is a PROPORTION rather than a guessed
ceiling. The first version used a bare 2000 and went red at 2,502 heredoc lines in
a corpus that was entirely healthy -- a hand-written number failing the way
hand-written numbers fail here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Check results were prose. `check`, `check_num` and `check_text` printed PASS or
FAIL and nothing else, so proving that a mutation reddened one NAMED check meant
grepping text -- which is how every mutation proof in this repository is currently
made, a person reading `FAIL  <name>` out of a log and retyping it.

That is also how a reverted guard once reported plain green while the check count
fell from 190 to 186: the suite passed, and the only evidence anything had changed
was a number nobody was comparing.

THE FIX IS NOT A SECOND EMITTER. A second source of truth for how many checks ran
is the defect this family of issues exists to close. lib.sh had ELEVEN places that
bumped PGC_CHECKS, each with its own outcome line beside it -- eleven chances to
add a twelfth and forget the line, which is exactly what projections.sh's
expect_fail did with ten call sites for as long as it existed.

So counting a check and recording it are ONE operation, pgc_record. A helper
cannot report an outcome without being counted, and cannot be counted without
reporting one, because no code path does either alone. `checks run: N` and the N
record lines are the same increment seen twice. Eleven sites became one, and the
arm that holds it is structural: lib.sh may bump PGC_CHECKS in exactly one place,
and that place must be pgc_record.

The record is tab separated -- suite, name, verdict, reason -- so a check name
containing spaces survives. The reason carries #915's REASON_CODE, which is what
makes this more than a reformat: an unrunnable check is distinguishable from a
passing one without parsing prose. A verdict pgc_record does not recognise is
recorded as a FAIL rather than dropped, because dropping it would leave PGC_CHECKS
bumped with no outcome recorded -- the reconciliation pgc_summary already refuses.

THE HUMAN LINES DID NOT MOVE. DISPLAY is passed to pgc_record whole rather than
composed inside it, and both harnesses pin the exact strings for check, check_text,
check_num and check_unrunnable. 3,762 call sites, with suites, selftests and CI all
grepping `^PASS` and `^FAIL`, is far past what a careful refactor can be trusted on.

PGC_SUITE is resolved once at load rather than per check: pgc_record runs at every
one of those call sites, and a basename fork at each is 3,762 forks a suite does
not need.

The runner reconciles the two artifacts per suite: a log states `checks run: N` and
carries N records. That cannot fail by drifting, since one function does both, but
it can fail -- a suite killed mid-way, a truncated log, a helper that prints an
outcome without recording it. A log with no count at all never reached its summary,
which is a different fault from a miscount and is reported as one rather than
reading as a clean reconciliation.

Stacked on #916, which supplies pgc_log_shows_accounting: only a suite that reached
its summary has a count to reconcile against.

Evidence: selftest exit 0, 535 checks, 535 records, 0 failures; 14 pytest tests;
shellcheck -S error rc=0; docs_style PASSED.

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.

1 participant