From c223bf6131323b96532b5180e5ab42b90f647f0b Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 09:14:06 -0600 Subject: [PATCH 1/2] test/pytest: every counted assertion is a record, and the count is derived from them (#937) The shell harness makes counting and recording the same call, so no path can do either alone, and reconciles the totals afterwards. The pytest half reaches the same property through Python rather than through the shell's format: nothing here reads, sources or derives from test/*.sh. It reaches it more strongly, because Python can remove the possibility instead of policing it: @property def count(self): return len(self._records) A count with no setter cannot drift from the stream it counts. That is #937's property 1 by construction rather than by discipline. Measured before this: _counted() at 15 call sites, self.count incremented by one line and read by one (pytest_runtest_call), and zero per-assertion records anywhere. THE VERDICT IS NOT PASSED IN. _counted() is called before the comparison at all 15 sites, so the call site does not know the outcome, and passing it would split one operation back into two. Assertions in a body are sequential and a raise ends the test, so the failing assertion is the LAST record -- the verdict is resolved from the exception in a later phase, and every refusal message stays byte-identical here. cannot_run() records UNRUN rather than a pass. A refused assertion records nothing: a VacuityError means the assertion never ran, so the stream is outcomes, not attempts. Removal proofs: the record is never appended 228 red (the whole corpus) a faithful, writable second counter 1 red, and only one: test_the_count_cannot_be_moved_without_a_record The second is the one that matters. Every other arm accepts an implementation that keeps two numbers updated together; only the construction arm refuses it. Verified: pytest 228 driver-free, 326 full corpus against a live PG16 cluster, harness_selftest 803/803 on PG16, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- CHANGELOG.md | 30 ++++++++ test/pytest/TESTS.md | 45 +++++++++++ test/pytest/pgc_vacuity.py | 102 ++++++++++++++++++++----- test/pytest/test_check_records.py | 122 ++++++++++++++++++++++++++++++ test/pytest/test_harness_deps.py | 5 ++ 5 files changed, 286 insertions(+), 18 deletions(-) create mode 100644 test/pytest/test_check_records.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 46049bc9..8b48381b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,36 @@ true until the next version shipped. ### Added +- Every counted assertion in the pytest harness produces a record, and the + count is derived from them (#937, first phase). + + The shell harness makes counting and recording the same call, so no path can + do either alone, and reconciles the totals afterwards. The pytest half reaches + the same property through Python rather than through the shell's format. + + It reaches it more strongly, because Python can remove the possibility instead + of policing it. The count is not a second variable kept in step with the + records; it is `len(self._records)`, a property with no setter. A count that + cannot be written cannot drift from the stream it counts. + + Measured before this: `_counted()` at 15 call sites, the counter incremented + by one line and read by one, and zero per-assertion records. + + The record is an object on the recorder, not a formatted line. The shell's + record is tab separated, so `pgc_record` has to strip tabs and newlines out of + a check name. There is no separator here to smuggle, and a name carrying both + is asserted to round-trip byte-identical, so the class of defect cannot return + silently if these ever become a line. + + `cannot_run()` records `UNRUN` rather than a pass. An assertion that declined + to run is an outcome like any other. + + A refused assertion leaves no record: a `VacuityError` means the assertion + never ran, so the stream is outcomes rather than attempts. + + Still to come in #937: the verdict resolved from the outcome, and a session + reconciliation that can fail. + - A serial inner Hash Join can push the build-side keys into a direct columnar scan (#752). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 4f8e4b30..37db360d 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -71,6 +71,7 @@ behaviour, the source of that number is named. - [23. test_mutation_ledger.py: which checks have ever been red](#23-test_mutation_ledgerpy-which-checks-have-ever-been-red) - [24. test_loop_coverage_premise.py: a loop that never ran asserted nothing](#24-test_loop_coverage_premisepy-a-loop-that-never-ran-asserted-nothing) - [25. test_join_runtime_filter.py: serial join runtime filter](#25-test_join_runtime_filterpy-serial-join-runtime-filter) +- [26. test_check_records.py: every counted assertion is a record](#26-test_check_recordspy-every-counted-assertion-is-a-record) ## 1. How to read a test in here @@ -2363,3 +2364,47 @@ crash on this shape when it drained the tap through `ExecProcNode`. A non-key fact-table qual with late materialization off. The attach used to force the two-pass path with only the join key decoded, so the qual dropped every row. Heap is the oracle. Independent of the shell conjunction arm. + +## 26. test_check_records.py: every counted assertion is a record + +#937, first phase. The shell harness makes counting and recording the same call, +so no path can do either alone, and then reconciles the totals. **The pytest half +reaches the same property through Python instead of through the shell's format**, +which is what "parallel in functionality only" requires: nothing here reads, +sources or derives from `test/*.sh`. + +It is reached more strongly, because Python can remove the possibility rather than +police it. The count is not a second variable kept in step with the records: + +```python +@property +def count(self): + return len(self._records) +``` + +Measured before this file existed: `_counted()` at 15 call sites, `self.count` +incremented by one line and read by one, and **zero** per-assertion records. + +| test | what it pins | +|---|---| +| `test_each_counted_assertion_appends_exactly_one_record` | three assertions leave three records, from a premise of zero | +| `test_the_count_is_the_record_stream` | the count tracks the records at every step, not only at the end | +| `test_the_count_cannot_be_moved_without_a_record` | **the construction proof**: the count has no setter | +| `test_a_record_names_the_assertion_that_made_it` | the names, in order | +| `test_a_refused_assertion_leaves_no_record` | a `VacuityError` is not an outcome; the stream is not a log of attempts | +| `test_a_name_carrying_a_separator_survives_the_record` | a tab and a newline in a name round-trip byte-identical | + +**The construction arm is the one that matters.** A test checking only that the +count agrees with the records would pass on an implementation keeping two numbers +that happen to be updated together — which is exactly the drift the shell side +needs a reconciliation to catch. Asserting the count cannot be written at all is +what makes the agreement structural. + +**The separator arm looks redundant and is not.** There is no separator in an +object, so it cannot fail today. It exists because the moment somebody formats +these records into a line, the shell's tab-and-newline defect returns — measured +there as a record of four fields for a tabbed name and two lines for a newline — +and without this arm nothing would say so. + +Still to come in #937: the verdict resolved from the outcome, and a session +reconciliation that can fail. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 99598ad6..8d6002d3 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -125,6 +125,36 @@ def _failed_query(v, _prefix=QUERY_ERROR): _WRITE_TAGS = ("INSERT", "UPDATE", "DELETE", "MERGE", "COPY") +class _Record: + """One counted assertion's outcome. #937. + + THE COUNT IS THIS LIST'S LENGTH, which is the whole design. The shell harness + keeps `PGC_CHECKS` and a record stream honest by reconciling them, because in + bash they must be two things. Here they need not be, so the count is a derived + property and `_records` is the only state -- nothing can increment a count + without a record existing, by construction rather than by discipline. + + AN OBJECT, NOT A FORMATTED LINE, and that is load-bearing rather than + idiomatic. The shell's record is tab separated, so `pgc_record` had to strip + tabs AND newlines out of a check name: measured there, a tab gave a record of + four fields and a newline gave two lines. A name is carried here as an + attribute, so there is no separator to smuggle -- and test_check_records.py + asserts that with a name holding both, because the moment these become a line + the whole class of defect returns and nothing would say so. + + The verdict is filled in by the phase that resolves it from the outcome; a + record created here is an assertion that ran, which is the fact the call site + knows. + """ + + __slots__ = ("name", "verdict", "reason") + + def __init__(self, name, verdict="PASS", reason=""): + self.name = name + self.verdict = verdict + self.reason = reason + + class _Write: """One write statement's outcome. @@ -234,9 +264,31 @@ class Expect: def __init__(self, nodeid): self.nodeid = nodeid - self.count = 0 + self._records = [] self.unrunnable = None + @property + def records(self): + """The assertions this test has concluded, in the order it concluded them. + + A TUPLE, so a caller cannot append to the stream without going through the + recorder. `pytest_runtest_call` reaches the list itself to resolve the + verdict of the assertion that raised; everything else reads this. + """ + return tuple(self._records) + + @property + def count(self): + """How many assertions were counted. NOT a second variable. + + There is no setter, deliberately. An arm in test_check_records.py asserts + that assigning to it raises, because a test checking only that the count + AGREES with the records would pass on an implementation that keeps two + numbers and happens to update both -- which is exactly the drift the shell + side needs a reconciliation to catch. + """ + return len(self._records) + # -- the recorder ------------------------------------------------------- def _refuse_failed_query(self, name, got, want): """Refuse a comparison where either side is a failed query. @@ -257,8 +309,17 @@ def _refuse_failed_query(self, name, got, want): f"failure you expect rather than comparing two of them." ) - def _counted(self): - self.count += 1 + def _record(self, name, verdict="PASS", reason=""): + """Count an assertion by recording it. One operation, no other path. + + Called BEFORE the comparison, at every site, because that is where the + call site knows the assertion ran. The verdict is not passed in: passing it + would need the outcome, which would split this back into two steps. It is + resolved from the exception in `pytest_runtest_call` instead -- assertions + in a body are sequential and a raise ends the test, so a failed test's + failing assertion is the LAST record and every earlier one passed. + """ + self._records.append(_Record(name, verdict, reason)) # -- numbers ----------------------------------------------------------- def num(self, got, want, name): @@ -273,7 +334,7 @@ def num(self, got, want, name): f"{type(got).__name__}={got!r} and {type(want).__name__}={want!r}. " f"A text comparison here is the defect this harness removes." ) - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -328,7 +389,7 @@ def ordered_rows(self, got, want, name): f"is the same, so the reverse ordering is identical and the claim " f"asserts nothing beyond what rows() already asserts." ) - self._counted() + self._record(name) if g != w: for i, (a, b) in enumerate(zip(g, w)): if a != b: @@ -358,7 +419,7 @@ def ordering_observable(self, forward, reverse, name): f, r = list(forward), list(reverse) if not f and not r: raise VacuityError(f"{name}: both directions are empty.") - self._counted() + self._record(name) if f == r: raise AssertionError( f"{name}: the forward and reverse readings are identical, so nothing " @@ -400,7 +461,7 @@ def differ(self, got, want, name): f"this assertion would report the mutation as observable. Assert " f"the failure you expect instead of differencing two of them." ) - self._counted() + self._record(name) if got == want: raise AssertionError( f"{name}: arms-do-not-differ: {got!r} on both arms. An A/B whose arms " @@ -495,7 +556,7 @@ def rows(self, got, want, name, allow_empty=None): f"failed. If an empty result is the point, pass " f"allow_empty='why it is empty'." ) - self._counted() + self._record(name) if list(got) != list(want): raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -512,7 +573,7 @@ def hash(self, got, want, name): self._refuse_failed_query(name, got, want) if _empty(got) and _empty(want): raise VacuityError(f"{name}: both hashes are empty.") - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -524,7 +585,7 @@ def text(self, got, want, name): raise VacuityError( f"{name}: the expected text is empty, so anything empty satisfies it." ) - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -580,7 +641,7 @@ def sqlstate(self, exc, want, name): f"`exc.value` inside a `with pytest.raises(...) as exc` block, not " f"`exc`." ) - self._counted() + self._record(name) got = exc.sqlstate if got is None: raise AssertionError( @@ -627,7 +688,7 @@ def plan_node(self, plan, node_type=None, provider=None, name=None): continue if provider is not None and pv != provider: continue - self._counted() + self._record(name) return node raise AssertionError( @@ -654,7 +715,7 @@ def at_least(self, got, floor, name): f"{name}: a floor of {floor!r} is satisfied by any count, so this " f"asserts nothing." ) - self._counted() + self._record(name) if not got >= floor: raise AssertionError(f"{name}: got {got!r}, wanted at least {floor!r}") @@ -678,7 +739,7 @@ def refusal(self, result, name, *patterns): f"{name}: refusal() with no pattern asserts only that something " f"failed, which is the defect it exists to remove." ) - self._counted() + self._record(name) result.assert_outcomes(failed=1, passed=0) # ANCHORED TO pytest's ERROR-LINE PREFIX, and that is the whole point. # @@ -719,12 +780,12 @@ def outcomes(self, result, name, **want): raise VacuityError( f"{name}: outcomes() with no expectation asserts nothing." ) - self._counted() + self._record(name) result.assert_outcomes(**want) def run_failed(self, result, name): """Assert an inner run exited non-zero, and count it.""" - self._counted() + self._record(name) if result.ret == 0: raise AssertionError( f"{name}: the inner run exited 0, so nothing refused it." @@ -774,7 +835,7 @@ def plan_marker(self, plan, key, name=None, absent=False): if key in node: found = True - self._counted() + self._record(name) if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") if not absent and not found: @@ -802,7 +863,12 @@ def cannot_run(self, reason, detail=""): f"unrunnable reason {reason!r} is not one of {UNRUNNABLE_REASONS}" ) self.unrunnable = (reason, detail) - self._counted() + # UNRUN, NOT PASS. An assertion that declined to run is an outcome like any + # other -- #937 property 4 -- and the shell's verdict vocabulary has the same + # four values for the same reason. The record is named by the reason CODE, + # which is from a closed list, so the stream stays keyable when the detail is + # free text. + self._record(name=reason, verdict="UNRUN", reason=detail) @pytest.fixture diff --git a/test/pytest/test_check_records.py b/test/pytest/test_check_records.py new file mode 100644 index 00000000..1c74beec --- /dev/null +++ b/test/pytest/test_check_records.py @@ -0,0 +1,122 @@ +"""Every counted assertion produces a record, and the count is derived from them. + +#937. The shell harness records every check outcome as a machine-readable line, +and `pgc_record` counts and records in one call so no path can do either alone. +`pgc_reconcile_records` then reconciles the record count against `checks run:`. + +THE PYTEST HALF REACHED THE SAME PROPERTY THROUGH PYTHON RATHER THAN THROUGH THE +SHELL'S FORMAT, which is what "parallel in functionality only" requires. Nothing +here reads, sources or derives from `test/*.sh`; `test_harness_deps.py` proves +that for the whole corpus rather than this file asserting it about itself. + +AND IT IS STRONGER THAN THE SHELL'S, because Python can remove the possibility +instead of policing it. The shell keeps a counter and a record stream honest by +reconciling two variables that could drift. Here the count IS the record stream: + + @property + def count(self): + return len(self._records) + +A derived count cannot be incremented without a record existing. That is property +1 of #937 reached by construction rather than by discipline, and the arms below +attack the construction rather than trusting the sentence. + +Measured before this file existed: `_counted()` at 15 call sites, `self.count` +incremented by one line and read by one (`pytest_runtest_call`), and ZERO +per-assertion records anywhere. +""" + +import pgc_vacuity + + +def test_each_counted_assertion_appends_exactly_one_record(expect): + """Three assertions, three records. The premise is that a fresh recorder has + none, or the arm would pass on a recorder that ignored every call.""" + e = pgc_vacuity.Expect("records::one-each") + expect.num(len(e.records), 0, "premise: a fresh recorder holds no records") + e.num(1, 1, "first") + e.num(2, 2, "second") + e.num(3, 3, "third") + expect.num(len(e.records), 3, "three assertions left three records") + + +def test_the_count_is_the_record_stream(expect): + """Not "the count agrees with the records" -- that is the shell's property, + and it needs a reconciliation because the two can drift. Here they are the + same object, so the arm asserts identity of the NUMBER at every step rather + than equality at the end.""" + e = pgc_vacuity.Expect("records::derived") + seen = [] + for i in range(4): + e.num(i, i, f"assertion {i}") + seen.append((e.count, len(e.records))) + expect.rows([f"{c}/{r}" for c, r in seen], + ["1/1", "2/2", "3/3", "4/4"], + "the count tracks the records at every step") + + +def test_the_count_cannot_be_moved_without_a_record(expect): + """THE CONSTRUCTION PROOF, and the reason this is not just a second counter. + + A test that only checked `count == len(records)` after a run would pass on an + implementation that keeps two variables and happens to update both. This + asserts the count cannot be written at all, which is what makes the agreement + structural rather than maintained. + """ + e = pgc_vacuity.Expect("records::readonly") + e.num(1, 1, "one assertion") + try: + e.count = 99 + except AttributeError: + expect.num(e.count, 1, "the count refused to be written and did not move") + else: + raise AssertionError( + "the count was assignable, so it is a second variable that can drift " + "from the records rather than being derived from them" + ) + + +def test_a_record_names_the_assertion_that_made_it(expect): + """A record that cannot be traced to a call site answers no question worth + asking. The names are asserted IN ORDER, because the order is what lets a + later phase say which assertion failed.""" + e = pgc_vacuity.Expect("records::named") + e.num(1, 1, "the first question") + e.text("a", "a", "the second question") + e.num(2, 2, "the third question") + expect.ordered_rows([r.name for r in e.records], + ["the first question", "the second question", + "the third question"], + "each record carries the name its call site gave") + + +def test_a_refused_assertion_leaves_no_record(expect): + """A VacuityError means the assertion never ran, so it is not an outcome. + + This is the arm that stops the record stream becoming a log of attempts. It + matters for the reconciliation in phase 5: a refused assertion that left a + record would make the totals disagree with what the run reported. + """ + e = pgc_vacuity.Expect("records::refused") + try: + e.num("100", 100, "a text comparison") + except pgc_vacuity.VacuityError: + expect.num(len(e.records), 0, "a refused assertion recorded nothing") + else: + raise AssertionError("num() accepted a string, so this arm tested nothing") + + +def test_a_name_carrying_a_separator_survives_the_record(expect): + """#937 property 3, asserted rather than assumed. + + The shell had to strip tabs and newlines from a check name because its record + is a tab-separated line: a name with a tab gave four fields, and a name with a + newline gave two lines. The record here is an object, so there is no separator + to smuggle -- but that must be a test, because the moment somebody formats + these into a line the class of defect comes back and nothing would say so. + """ + e = pgc_vacuity.Expect("records::separators") + nasty = "a name with\ta tab and\na newline" + e.num(1, 1, nasty) + expect.num(len(e.records), 1, "a name with separators made exactly one record") + expect.text(e.records[0].name, nasty, "and the name came back byte-identical") diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index 64b2627e..c7d4b517 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -90,6 +90,11 @@ # agrees. The fourth time this arm has caught a merge-order consequence # rather than a mistake, which is the argument for it. "test_mutation_ledger.py", + # #937's first phase. It exercises `Expect` directly -- no connection, no + # cluster, no driver -- so the classifier puts it here and the declaration must + # agree. The fifth time this arm has decided a membership rather than been told + # one. + "test_check_records.py", ] From 3a3d1e688be8023b27917d2336210b4ef5014016 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 09:20:43 -0600 Subject: [PATCH 2/2] test/pytest: correct the phase-2 argument in _record's comment (#937) The comment argued that a later phase could resolve each record's verdict from the exception, because assertions in a body are sequential and a raise ends the test, so the failing assertion would be the last record. That is false HERE, and this corpus is what makes it false: proving a guard refuses means catching the AssertionError, which five tests do (test_ordered.py:243, test_failed_query_sentinel.py:236, :326, :357, :382). Driven on this branch: count before/mid/after: 0 / 1 / 2 record 0 'this comparison must fail' verdict PASS <- this RAISED record 1 'and the test continues' verdict PASS 1 passed A genuinely failed assertion stays PASS, in a passing test, and nothing reaches pytest_runtest_call to correct it. Found by @OffgridwithJD attacking the argument rather than the code. Nothing this PR CLAIMS is affected: the count above is 2 and correct, and phase 1 claims the count and its underivability, not verdicts. But a comment that argues for a guarantee the code does not provide is worse than no comment, because it stops the next person checking -- the same reason pgc_write_source_stamp carries that warning. The comment now records the measurement and says the verdict has to be set on the comparison's own path, where the outcome is known and no propagation is needed. Comment only. No behaviour change; 228 driver-free still green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/pytest/pgc_vacuity.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 8d6002d3..d929f8c5 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -314,10 +314,30 @@ def _record(self, name, verdict="PASS", reason=""): Called BEFORE the comparison, at every site, because that is where the call site knows the assertion ran. The verdict is not passed in: passing it - would need the outcome, which would split this back into two steps. It is - resolved from the exception in `pytest_runtest_call` instead -- assertions - in a body are sequential and a raise ends the test, so a failed test's - failing assertion is the LAST record and every earlier one passed. + would need the outcome, which would split this back into two steps. + + SO EVERY RECORD IS `PASS` UNTIL A LATER PHASE SETS IT, AND THAT PHASE + CANNOT RESOLVE IT FROM THE EXCEPTION. The first version of this comment + argued it could: assertions in a body are sequential and a raise ends the + test, so the failing assertion would be the last record. **That is false + here, and the corpus is what makes it false** -- proving a guard refuses + means catching the AssertionError, which five tests do + (test_ordered.py:243, test_failed_query_sentinel.py:236, :326, :357, :382). + Driven on this branch: + + count before/mid/after: 0 / 1 / 2 + record 0 'this comparison must fail' verdict PASS <- this RAISED + record 1 'and the test continues' verdict PASS + 1 passed + + A genuinely failed assertion stays PASS, in a passing test, and nothing + reaches `pytest_runtest_call` to correct it. Found by @OffgridwithJD + attacking the argument rather than the code. + + The verdict therefore has to be set on the comparison's own path, where + the outcome is known and no propagation is needed. That stays one + operation; it is phase 2's work and is not claimed here. What IS claimed + here is the count, which the probe above shows is 2 and correct. """ self._records.append(_Record(name, verdict, reason))