Skip to content

test/pytest: the raiser has to be the statement under test (#432) - #939

Merged
jdatcmd merged 5 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-the-raiser-is-the-statement
Sep 11, 2026
Merged

test/pytest: the raiser has to be the statement under test (#432)#939
jdatcmd merged 5 commits into
commandprompt:mainfrom
OffgridwithJD:audit/432-the-raiser-is-the-statement

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

raises-catches-setup — entry 5, the last on VACUITY_MODES.md's list of what to add next.

The statement count rule refuses a pytest.raises block holding more than one top-level statement. Two shapes are one statement and still perform the setup inside the block, so the count saw nothing:

with pytest.raises(psycopg.errors.UndefinedObject) as exc:
    _setup_then_run(conn)                       # a HELPER CALL: one statement

with pytest.raises(psycopg.errors.UndefinedObject) as exc:
    for stmt in (setup_sql, sql_under_test):    # a COMPOUND STATEMENT: one
        conn.execute(stmt)                      # statement holding two

Both were measured against the shipped scan reporting 1 passed, exit 0, zero offences, with the setup raising and the statement under test never running. The two arms that recorded them as residuals now assert the refusal, so the closure is a measurement rather than a sentence.

The fix is not a deeper count

Counting recursively would catch both and would also refuse a legitimate single-statement loop — which is why the entry always said the fix is a claim about which statement raised. Two rules:

  • No compound statement, and all nine kinds rather than the one the inventory named. if, while, with and try nest exactly as for does, and a rule catching only for closes an example rather than a mode. The kinds are looked up by name, because TryStar and Match exist only on newer Pythons and a missing attribute would be a NameError at import rather than a rule that quietly does less.
  • No call to a function defined in the same file, anywhere in the statement. Such a function can run any number of statements and nothing in the block says which failed. A call to an imported function, or to a method, is the thing under test and stays allowed.

The rule turns on where the function is defined, not on the statement being a call, and that is what makes the budget zero.

Measured first, and it changed the design

Before writing either rule: five pytest.raises blocks in real code across the corpus, and none touch a database. Four call build_and_install, imported from the module under test; one calls a method. The scan reports no offence on any of them.

The entry suggests "a helper that runs exactly one statement and owns the assertion" — a DB helper. With zero SQL-raising blocks in the corpus that helper would have had no call sites, an instrument with nothing exercising it, which this directory refuses to build. The shape rules close the same mode against the code that exists.

The residual is a method. A method that performs setup and then the statement is invisible to this rule, and no static rule can see inside it. Stated rather than hidden, and pinned by the arm that accepts the method shape.

Prove by removal

Five mutations, each by exact string with a parse assertion, and the restore verified byte-identical rather than by git diff:

mutation test_raises_sqlstate.py
control 34 passed
compound rule removed 2 failed
local-def rule removed 2 failed
only for counts as compound 1 failed
only a bare call is searched 1 failed
the message filter narrowed 4 failed

Two of my own arms could not fail until I mutated them

  • The local-def rule searches the whole statement, and my first arm used a bare call only — so narrowing the search to Expr changed nothing and x = _helper() stayed open.
  • The every-compound-kind arm exists because without it, reducing the kind list to For alone was invisible.

And the rules fired while printing nothing

The message assembly buckets offenders by substring, and that bucket's filter was the exact sentence of the count rule. So both new rules refused the run and the layer printed refuses this run: . with an empty list — the arms reddened on a missing message while the refusal itself worked, which is a guard that cannot be told from an unfired one. The filter is now the common tail of all three phrases.

Recorded while I was in there

The comment defending the one-source-line offence phrase cited selftest 440 grepping this file and counting the copies. Selftest 440 no longer exists#927 deleted it under the harness-independence rule. Nothing greps this source for the phrase today, so that form is now a convention rather than a guarded property; what is still load-bearing is the runtime string the pytest arms match, which a split f-string would not change at all. The comment says that instead.

The count is a function of the merge

#935 moved the refused total 27 → 28. This branch moved it 27 → 28 as well. The merge was clean and therefore wrong: git took one 28 where the answer is 29, because the number is a property of both closures rather than of either. That is the failure TESTS.md's own header records about counts in this document, and the reason they are checked by arms. Every number below is what the arms reported against the ids on disk, not a number I chose:

refused today   28 -> 29      not refused   44 -> 43
demonstrated    48 -> 47      README.md     28 -> 29
TESTS.md prose  28 -> 29, and 44/43 -> 43/42

Gate

harness_selftest   588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED
driver-free job    10 files, 174 passed, psycopg absent from the venv
full corpus       257 passed with a cluster on pg18a
the raises scan   zero offences over the corpus, all 9 compound kinds recognised

Merged with main 79882992 (#935 landed). Independent of #936, which closes a different mode.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

OffgridwithJD and others added 4 commits September 10, 2026 21:50
…ompt#432)

`raises-catches-setup`, entry 5 and the last on VACUITY_MODES.md's list of what to add
next. The statement COUNT rule refused a `pytest.raises` block holding more than one
top-level statement. Two shapes are ONE statement and still perform the setup inside
the block, so the count saw nothing:

    with pytest.raises(psycopg.errors.UndefinedObject) as exc:
        _setup_then_run(conn)          # a HELPER CALL: one statement

    with pytest.raises(psycopg.errors.UndefinedObject) as exc:
        for stmt in (setup_sql, sql_under_test):   # a COMPOUND STATEMENT: one
            conn.execute(stmt)                     # statement holding two

Both were measured against the shipped scan reporting `1 passed`, exit 0, zero
offences, with the setup raising and the statement under test never running. The two
arms that recorded them as residuals now assert the refusal, so the closure is a
measurement rather than a sentence.

THE FIX IS NOT A DEEPER COUNT, for the reason the entry always gave: counting
recursively would also refuse a legitimate single-statement loop. It is a claim about
WHICH statement raised, in two rules.

NO COMPOUND STATEMENT, and all nine kinds rather than the one the inventory named. The
document named the `for` spelling; `if`, `while`, `with` and `try` nest identically, and
a rule catching only `for` would close an example rather than a mode. The kinds are
looked up by name instead of written out, because `TryStar` and `Match` exist only on
newer Pythons and a missing attribute would be a NameError at import rather than a rule
that quietly does less.

NO CALL TO A FUNCTION DEFINED IN THE SAME FILE, anywhere in the statement. Such a
function can run any number of statements and nothing in the block says which failed. A
call to an IMPORTED function, or to a METHOD, is the thing under test and stays allowed.

THE RULE TURNS ON WHERE THE FUNCTION IS DEFINED, NOT ON THE STATEMENT BEING A CALL, and
that is what makes the budget zero. Measured over the corpus before writing either rule:
five `pytest.raises` blocks in real code, NONE touching a database -- four call
`build_and_install`, imported from the module under test, and one calls a method. The
scan reports no offence on any of them.

MEASURED FIRST, AND IT CHANGED THE DESIGN. The entry suggests "a helper that runs
exactly one statement and owns the assertion", which would be a DB helper. There are
zero SQL-raising `pytest.raises` blocks in the corpus, so that helper would have had no
call sites -- an instrument with nothing exercising it, which is the thing this
directory refuses to build. The shape rules close the same mode against the code that
exists.

THE RESIDUAL IS A METHOD. A method that performs setup and then the statement is
invisible to this rule, and no static rule can see inside it. Stated rather than hidden,
and pinned by the arm that accepts the method shape.

Prove by removal, five mutations, each by exact string with a parse assertion, and the
restore verified byte-identical rather than by `git diff`:

    control                           34 passed
    compound rule removed              2 failed
    local-def rule removed             2 failed
    only `for` counts as compound      1 failed
    only a bare call is searched       1 failed
    the message filter narrowed        4 failed

TWO OF MY OWN ARMS COULD NOT FAIL UNTIL I MUTATED THEM. The local-def rule searches the
WHOLE statement, and my first arm used a bare call only -- so narrowing the search to
`Expr` changed nothing and `x = _helper()` stayed open. And the every-compound-kind arm
exists because without it, reducing the kind list to `For` alone was invisible.

AND THE RULES FIRED WHILE PRINTING NOTHING. The message assembly buckets offenders by
substring, and the bucket's filter was the exact sentence of the COUNT rule -- so both
new rules refused the run and the layer said `refuses this run: .` with an empty list.
The arms reddened on a missing message while the refusal itself worked, which is a
guard that cannot be told from an unfired one. The filter is now the common tail of all
three phrases.

RECORDED WHILE I WAS IN THERE: the comment defending the one-source-line offence phrase
cited selftest 440 grepping this file and counting the copies. **Selftest 440 no longer
exists** -- commandprompt#927 deleted it under the harness-independence rule. Nothing greps this
source for the phrase today, so that form is now a convention rather than a guarded
property; what is still load-bearing is the RUNTIME string the pytest arms match, which
a split f-string would not change at all. The comment says that instead.

Gate:
    harness_selftest   588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED
    driver-free job    9 files, 161 passed, psycopg absent from the venv
    full corpus        241 passed with a cluster on pg18a

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

commandprompt#935 closed `insert-wrote-no-rows` and moved the refused total 27 -> 28. This branch
closes `raises-catches-setup` and moved it 27 -> 28 as well. The merge was CLEAN and
therefore wrong: git took one 28 where the answer is 29, because the number is a
property of both closures rather than of either.

That is the exact failure TESTS.md's own header records about counts in this document
-- "a claim whose correct value is a function of the MERGE rather than of either
branch, so it collided on essentially every rebase" -- and the reason the counts are
checked by arms instead of trusted. The arms named every number:

    refused today      28 -> 29
    not refused        44 -> 43
    demonstrated       48 -> 47   (section 3's sentence)
    TESTS.md prose     28 -> 29, and 44/43 -> 43/42
    README.md          28 -> 29

I did not decide any of them; each one is what `test_the_prose_totals_match_the_counted_modes`
and its siblings reported against the ids on disk.

Gate on the merged tree:
    harness_selftest   588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED
    driver-free job    10 files, 174 passed, psycopg absent from the venv
    full corpus        257 passed with a cluster on pg18a
    the raises scan    zero offences over the corpus, all 9 compound kinds recognised

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

@jdatcmd jdatcmd 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 exact head 3fb80aba26a205c33aac3764375e36cb9f47e291. Everything below was measured on that head, not on the one I started reading: I composed #936's main into 4c8f7c19 myself, and the resulting tree oid is 8a89aac922a1e8ae1c10c9115a145c4ad5a5e615identical to your head's tree. So the composition is not a claim either of us has to trust.

Your five mutations, re-run rather than read

My first control said 28 failed, 6 passed and I nearly reported your table as wrong. It was my instrument: test_raises_sqlstate.py is not in NO_CLUSTER, and the driver-free venv makes every inner pytester run fail to collect on ModuleNotFoundError: psycopg. With the driver installed the control is 34 passed, and every row of your table reproduces:

mutation mine yours
control 34 passed 34 passed
compound rule removed 2 failed 2 failed
local-def rule removed 2 failed 2 failed
only For counts as compound 1 failed 1 failed
only a bare call is searched 1 failed 1 failed
the message filter narrowed 4 failed 4 failed

Each mutation asserted its anchor present and the result parsed before running, and the file's md5 was compared back to baseline after each restore.

The count survived #936, derived rather than argued

You said the arms decided it. I ran them and printed what they derived:

refused ids (from section 2 on disk)   29
not refused                            43
sum                                    72   == named
README.md contains "29 refused"        1 occurrence

Population, on the merged head: 17 files, 5 pytest.raises blocks, 0 offences.

Three residuals, measured

The entry's own standard is that a rule catching only for closes an example rather than a mode. By that standard the two new rules each leave a spelling open. None of the three is present in the corpus today — that is why none blocks — but the residual section names only "a method", and these are not that.

1. The loop is refused as a statement and allowed as an expression. One token from the shape you close:

REFUSED   with pytest.raises(...): for stmt in (setup, under_test): conn.execute(stmt)
passes    with pytest.raises(...): [conn.execute(s) for s in (setup, under_test)]
passes    with pytest.raises(...): (conn.execute(setup), conn.execute(under_test))

Controls, so the probe can fail: an honest single statement and an honest single-statement loop are both not flagged.

2. The local-def rule is defeated by moving the helper one file over. Your own offending shape D, with nothing about what runs changed:

REFUSED   def _setup_then_run(conn): ...   (defined here)
passes    from pgc_helpers import _setup_then_run

3. The same key refuses an honest helper. A local _run() that executes exactly one statement is refused, and becomes clean by being imported instead.

2 and 3 are one coin: where the function is defined is neither necessary nor sufficient for the vice. I think the design call is still right — four of the five real blocks call build_and_install, imported from the module under test, so keying on locality is what makes the budget zero against the code that exists. My ask is only that §3's residual paragraph say so, because a reader who moves a helper into pgc_helpers.py will get a green scan and no hint that the mode came with it.

Noted, not asked for

Your comment correcting the selftest-440 rationale is the right shape: the one-line phrase is now a convention and the runtime string is the guarded property. That is a comment that will still be true after the next deletion, which is more than the one it replaced managed.

…not have moved

I claimed a closure I can defeat three ways, and @jdatcmd's review named the residuals.
Measuring them settles the classification against me:

    a `for` loop over two statements                     REFUSED
    the same two as a list comprehension                 allowed
    the same two as a tuple of calls                     allowed
    a helper defined in ANOTHER file                      allowed
    an honest one-statement helper in THIS file           REFUSED (a false positive)

A comprehension and a tuple are EXPRESSIONS, not compound statements, so a rule about
statement kinds cannot see them; `local_defs` is built from one file, so moving the
helper one file over defeats it. Neither is a contrivance -- both are ordinary Python.

So by this document's own convention the mode stays in section 3, the section-2 row is
withdrawn, and the refused count goes back to 28 with not-refused back to 44. The arms
derive it: refused=28, not_refused=44, and `raises-catches-setup` is not in the refused
set. Section 5's entry now says "Done, and it NARROWS rather than closes", beside the two
other entries that say the same thing.

THE ARM THAT EXISTS TO CATCH THIS OVERCLAIM DID NOT CATCH IT, and that is the part worth
keeping. `test_the_mode_this_layer_only_narrows_is_still_listed_as_open` required the mode
to be named in section 3 -- and section 3 keeps a back-reference for every mode that
MOVES, "`X` is now closed", so the id is present in section 3 in BOTH states. My first
version wrote the closure into section 3, added the row to section 2, moved the count to
29, and the arm passed. Its own docstring says what it was for: "A document that quietly
moved the mode to section 2 would claim a closure this scan does not make."

It now also requires the mode to be named OUTSIDE a closure back-reference, and to be
absent from section 2. Proven by removal, each shape of the overclaim separately:

    control (the document as it stands)   1 passed
    entry rewritten as a back-reference   1 failed
    row added back to section 2           1 failed
    both, which is what my branch did     1 failed

AND MY FIRST TIGHTENING WAS WRONG IN THE SAME FAMILY. I wrote
`section3.split("is now closed")[0]`, which truncates at the FIRST back-reference in the
section -- today `insert-wrote-no-rows`, which sits above this entry -- so it reported
"only as a back-reference" about a document that names the mode correctly. A positional
test over a section holding several back-references is a test about their order. It is
per-line now.

Gate:
    harness_selftest   588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED
    driver-free job    10 files, 183 passed, psycopg absent from the venv
    full corpus        266 passed with a cluster on pg18a
    derived counts     refused 28, not refused 44, named 72

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

Copy link
Copy Markdown
Collaborator Author

Pushed 0d1d0279. This reclassifies the mode from closed to NARROWED, so your APPROVE at 4c8f7c19 no longer covers it — please re-review rather than let it stand.

You asked only that §3 name the three residuals. Measuring them to write them down settled the classification against me:

shape verdict
a for loop over two statements refused
the same two as a list comprehension allowed
the same two as a tuple of calls allowed
a helper defined in another file allowed
an honest one-statement helper in this file refused — a false positive

A comprehension and a tuple are expressions, not compound statements, so a rule about statement kinds cannot see them; local_defs is built from one file, so moving the helper one file over defeats it. Neither is a contrivance. By this document's own convention that is §3, not §2 — so the row is withdrawn, the refused count goes back to 28, not-refused back to 44, and §5's entry joins the other two that say "Done, and it NARROWS rather than closes". The arms derive it: refused=28 not_refused=44, and raises-catches-setup is not in the refused set.

The arm that exists to catch this overclaim did not catch it

test_the_mode_this_layer_only_narrows_is_still_listed_as_open required the mode to be named in §3 — and §3 keeps a back-reference for every mode that moves, `X` is now closed, so the id is present in §3 in both states. My first version wrote the closure into §3, added the row to §2, moved the count to 29, and that arm passed. Its own docstring says what it was for: "A document that quietly moved the mode to section 2 would claim a closure this scan does not make."

It now also requires the mode to be named outside a closure back-reference and to be absent from §2. Proven by removal, each shape separately:

control (the document as it stands)   1 passed
entry rewritten as a back-reference   1 failed
row added back to section 2           1 failed
both, which is what my branch did     1 failed

And my first tightening was wrong in the same family. I wrote section3.split("is now closed")[0], which truncates at the first back-reference in the section — today insert-wrote-no-rows, which sits above this entry — so it reported "only as a back-reference" about a document that names the mode correctly. A positional test over a section holding several back-references is a test about their order. It is per-line now.

Gate

harness_selftest   588 checks, 588 passed + 0 failed + 0 unrunnable, PASSED
driver-free job    10 files, 183 passed, psycopg absent from the venv
full corpus        266 passed with a cluster on pg18a
derived counts     refused 28, not refused 44, named 72

On your #941 residual: you are right that greedy .* can run past this grep's comparison to an unrelated one later on the same line, and that my -ne plant cannot see it because it holds only one comparison. Zero such lines in the tree today, so I am recording it rather than widening the regex again — a two-comparison plant would pin that budget the way (a)b pins the other direction, and I would rather add it when a line exists than guess the shape.

And your reconciliation residue is the nicer half of that exchange: printing 61 inputs = 0 string + 58 numeric and chasing the three rather than adjusting the buckets is what found that my assembled plants are a different population. That is the same discipline that caught my 52, applied to yourself.

@jdatcmd
jdatcmd merged commit 368c4b2 into commandprompt:main Sep 11, 2026
13 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 11, 2026
…grep -c branch

# Conflicts:
#	CHANGELOG.md
jdatcmd added a commit that referenced this pull request Sep 11, 2026
…eadable branch

Part 010 conflicted, and resolving it found a latent defect of MINE that main's
side fixes. Taking main's side wholesale, not as a compromise.

THIS BRANCH KEPT `exit 0` IN A SOURCED PART, which is exactly the defect #934
names and #940 closed. My change to part 010 swapped the bare `echo "SKIP ..."`
for `check_skip`, to route the outcome through pgc_record as @linuxhikerpm asked,
and left the `exit 0` below it untouched.

AND THE `check_skip` COULD NEVER HAVE RECORDED ANYTHING. `lib.sh` is sourced by
part 020; part 010 runs before it, which is the same fact #940's own comment
turns on when it spells 66 as a literal rather than `$PGC_EXIT_SKIPPED`. So
`check_skip` in part 010 is not a call to a helper, it is a command that does not
exist yet. Measured rather than argued:

    type check_skip, at the point part 010 runs   NOT DEFINED
    calling it under the driver's `set -uo pipefail`
        bash: check_skip: command not found, rc=127, execution CONTINUES

Under `-u -o pipefail` without `-e` that is not fatal, so the bail path would
have printed a bash error, recorded nothing, and then exited the driver with
status 0 -- the false green, with a diagnostic line in front of it. It has never
fired because the squatter-port path does not trigger in a normal run, which is
why no gate has ever executed those two lines.

main's version is correct on all three axes: `exit 66` rather than `exit 0`, the
`SKIPPED (ran no checks)` marker the runners pair with 66, and no call to a
function that has not been defined yet.

PART 010 IS THEREFORE AN EXCEPTION TO THE RULE THIS PR ARGUES, and it should be
named as one rather than quietly left out. Every per-check outcome routes through
`pgc_record` except in the one part that runs before `pgc_record` exists. There
the right shape is a part-level bail, which is what #940 built.

Checked rather than eyeballed, on the merged tree:

    parts sorting before 020                        010 only
    part 010: exit 0 sites / exit 66 / markers      0 / 2 / 2
    part 010: check_skip calls                      0
    part 430's arms: PGC_EXIT_SKIPPED=66 in lib.sh  1
    exit-0 and bare-exit sweep over all parts       0
    driver handed a bad pg_config                   rc=2, no summary
    accounting assertions still on the 4-field form none
    driver-free pytest job                          10 files, 183 passed

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
jdatcmd added a commit that referenced this pull request Sep 11, 2026
…ew rules (#918)

Merging the updated #923 base, which now carries #939, #940 and #941, puts this
branch under two rules that did not exist when it was written. Neither is a
conflict git could have shown me: both are arms that read the WHOLE tree, so a
file this branch never touched changed what this branch has to satisfy.

THE MANIFEST, which is this PR's own arm firing correctly. Part 420 compares
`parts.manifest` against the glob in both directions. #940 and #941 added parts
430 and 440 to main, so the manifest was two short and the arm would have gone
red -- which is the guard working rather than a merge problem. @OffgridwithJD
called this exact ordering on both of their PRs before either landed.

    on disk 44, listed 42, only-on-disk: 430-..., 440-...
    after, on disk 44, listed 44, both directions empty
    still live: drop 430 from the comparison and it is named again

Added in sorted position rather than appended, because the file is sorted and a
manifest that stops being sorted is a diff nobody can read.

AND ONE MORE STRING-COMPARED `grep -c`, MINE, at `run_all_versions.sh:1382`:

    [ "$(grep -c '^RESULT\t' "$builddir/${s}.log" || true)" != 0 ]

Zero such sites on main and zero on the #923 base, so this branch added it --
#929's defect, reintroduced while #941 was closing it. `grep -c` prints nothing
on a pattern that does not compile and `[ "" != 0 ]` is TRUE, so this answers
"the log has RESULT records" for a question grep never asked. One input to grep,
checked, which is #941's condition for the conversion being behaviour-preserving.

    #941's sweep on the merged tree, before   1
    after                                      0
    still live: one planted back               1

Checked on the merged tree, not on this branch alone:

    #940's exit-0 and bare-exit sweep          0
    the driver handed a bad pg_config          rc=2, no summary
    ledger control / BOGUS verdict / one       rc=0 / rc=2 / rc=2
      --mutation against two failures
    driver-free pytest job                     195 passed

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
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