From ff83aeb6f165700b30d3a6c5560b74a055d25121 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 09:30:45 -0600 Subject: [PATCH 1/3] test/pytest: resolve each record's verdict where the outcome is known (#937 phase 2) Phase 1 left every record PASS. The plan was to resolve the verdict from the exception in pytest_runtest_call, and @OffgridwithJD refuted that: proving a guard REFUSES means catching the AssertionError, which five tests in this corpus do (test_ordered.py:243, test_failed_query_sentinel.py:236, :326, :357, :382). Measured: 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 stayed PASS, in a passing test, with no exception reaching the hook. So the resolution happens INSIDE the assertion call, before any except in the test body can see the error. A WRAPPER, NOT A VERDICT PASSED AT THE CALL SITE. outcomes and refusal delegate to pytest's assert_outcomes, which raises a message this layer never composes -- there is no verdict for the call site to pass. The wrapper covers those without the methods knowing they are wrapped, and keeps one operation at every site. A REFUSAL MARKS NOTHING, with no special case for VacuityError being an AssertionError subclass: every VacuityError is raised BEFORE its record is taken, so no record exists to mark. Scanned, not trusted. AND ONE CLAIM IN THIS CHANGE WAS WRONG UNTIL A MUTATION CAUGHT IT. The comment said the index form was needed to survive nesting. Replacing self._records[taken] with self._records[-1] left all 235 tests green, because one call appends at most one record and the two always name it. The comment now says so, and the invariant it rests on is pinned by an arm instead of assumed -- a method that records twice reddens it. Removal proofs: the verdict is never set 3 red the reason is never recorded 1 red one method loses the wrapper 1 red (the drift arm, derived not listed) [-1] instead of [taken] green, correctly -- see above a method that records twice 2 red Verified: pytest 236 driver-free, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/pytest/TESTS.md | 40 +++++- test/pytest/pgc_vacuity.py | 76 +++++++++++ test/pytest/test_check_records.py | 216 ++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 2 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index bf41b3fd..cd6f597d 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -2432,5 +2432,41 @@ these records into a line, the shell's tab-and-newline defect returns — measur 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. +### Phase 2: the verdict is resolved where the outcome is known + +| test | what it pins | +|---|---| +| `test_a_failed_assertion_records_fail_even_when_the_test_catches_it` | the refutation's arm | +| `test_the_failure_reason_is_the_assertions_own_message` | one message per failure, not two that can drift | +| `test_the_assertions_before_a_failure_keep_their_verdicts` | verdicts are per assertion, not per test | +| `test_a_delegated_assertion_records_fail_too` | a failure raised by pytest's own `assert_outcomes` | +| `test_a_refusal_still_leaves_no_record` | the boundary a refusal must stay outside of | +| `test_every_refusal_precedes_its_record` | the static half, so that boundary cannot drift | +| `test_every_recording_method_resolves_its_verdict` | all 15, derived from the module rather than listed | +| `test_a_recording_method_takes_exactly_one_record_per_call` | the invariant the resolution rests on, pinned after a mutation showed it was assumed | + +**The first design was wrong and the corpus is what refuted it.** Resolving each +verdict from the exception in `pytest_runtest_call` assumed a raise ends the test. +Proving a guard refuses means *catching* the `AssertionError`, which five tests +here do. Measured before the fix: + +``` +count before/mid/after: 0 / 1 / 2 + record 0 'this comparison must fail' verdict PASS <- this one RAISED + record 1 'and the test continues' verdict PASS +1 passed +``` + +A genuinely failed assertion stayed `PASS`, in a passing test, with nothing +reaching the hook to correct it. The resolution now happens **inside** the +assertion call, before any `except` in the test body can see the error. + +**It is a wrapper rather than a verdict passed at the call site** because +`outcomes` and `refusal` delegate to pytest's `assert_outcomes`, which raises a +message this layer never composes — there is no verdict for the call site to pass. + +**A refusal marks nothing, with no special case** for `VacuityError` being an +`AssertionError` subclass: every `VacuityError` is raised *before* its record is +taken, so no record exists to mark. That is scanned rather than trusted. + +Still to come in #937: a session reconciliation that can fail. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index d4038679..5a360793 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -23,6 +23,7 @@ import numbers import pathlib +import functools import itertools import pytest @@ -259,6 +260,66 @@ def _plan_nodes(node): yield from _plan_nodes(entry) +def _resolving(method): + """Set the verdict of the record this call took, from the outcome. #937 phase 2. + + WHY NOT FROM THE EXCEPTION IN `pytest_runtest_call`. That was the first + design, and @OffgridwithJD refuted it: proving a guard REFUSES means catching + the AssertionError, which five tests in this corpus do (test_ordered.py:243, + test_failed_query_sentinel.py:236, :326, :357, :382). Measured before this: + + 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 stayed PASS, in a passing test, with no exception + reaching the hook. Here the resolution is INSIDE the assertion call, so it + happens before any `except` in the test body can see the error. + + WHY A WRAPPER RATHER THAN A VERDICT PASSED AT THE CALL SITE. `outcomes` and + `refusal` delegate to pytest's own `assert_outcomes`, which raises a message + this layer never composes -- there is no verdict for the call site to pass. A + wrapper covers those without the assertion methods knowing they are wrapped. + + IT MARKS THE RECORD THIS CALL TOOK, BY INDEX -- AND TODAY THAT IS THE SAME + RECORD AS THE LAST ONE. The first version of this comment claimed the index + form was needed to survive nesting, and a mutation refuted it: replacing + `self._records[taken]` with `self._records[-1]` left all 235 tests green, + because nothing distinguishes them. `row_set` delegates to `rows`, but + `row_set` takes no record of its own, so one call appends at most one record + and the two expressions always name it. + + The index form is kept because it stays correct if that stops being true, and + the invariant it depends on is now PINNED rather than assumed: + test_check_records.py asserts every recording method takes exactly one record + per call. If someone writes one that records twice, that arm reddens and this + comment is still true -- which is the opposite of how the first version of it + would have aged. + + A REFUSAL MARKS NOTHING, and that needs no special case for VacuityError being + an AssertionError subclass: every VacuityError in the recording methods is + raised BEFORE the record is taken, so no record exists to mark. That is not an + accident to rely on -- test_check_records.py scans the module and fails if + anyone adds one after. + """ + + @functools.wraps(method) + def wrapper(self, *args, **kwargs): + taken = len(self._records) + try: + return method(self, *args, **kwargs) + except AssertionError as exc: + if len(self._records) > taken: + rec = self._records[taken] + rec.verdict = "FAIL" + rec.reason = str(exc) + raise + + wrapper._pgc_resolves_verdict = True + return wrapper + + class Expect: """Records assertions, and refuses the ones that could not have failed.""" @@ -342,6 +403,7 @@ def _record(self, name, verdict="PASS", reason=""): self._records.append(_Record(name, verdict, reason)) # -- numbers ----------------------------------------------------------- + @_resolving def num(self, got, want, name): """Compare two numbers. Refuses anything that is not a number. @@ -381,6 +443,7 @@ def row_set(self, got, want, name, allow_empty=None): allow_empty=allow_empty) # -- ordered sequences --------------------------------------------------- + @_resolving def ordered_rows(self, got, want, name): """Compare two sequences IN ORDER, refusing the cases where order says nothing. @@ -420,6 +483,7 @@ def ordered_rows(self, got, want, name): f"{name}: same prefix, different length: got {len(g)} rows want {len(w)}" ) + @_resolving def ordering_observable(self, forward, reverse, name): """Assert this fixture can distinguish order at all, before relying on it. @@ -448,6 +512,7 @@ def ordering_observable(self, forward, reverse, name): ) # -- inequality ---------------------------------------------------------- + @_resolving def differ(self, got, want, name): """Assert that two arms of an A/B are observably different. @@ -561,6 +626,7 @@ def wrote(self, cur, want, name): self.num(count, want, name) # -- row sets ---------------------------------------------------------- + @_resolving def rows(self, got, want, name, allow_empty=None): """Compare two result sets. Refuses two empty sides unless declared. @@ -581,6 +647,7 @@ def rows(self, got, want, name, allow_empty=None): raise AssertionError(f"{name}: got {got!r} want {want!r}") # -- hashes and oracles ------------------------------------------------ + @_resolving def hash(self, got, want, name): """Compare two oracle hashes. Refuses self-comparison and error sentinels.""" if got is want: @@ -598,6 +665,7 @@ def hash(self, got, want, name): raise AssertionError(f"{name}: got {got!r} want {want!r}") # -- text -------------------------------------------------------------- + @_resolving def text(self, got, want, name): """Compare text exactly. Refuses an empty expectation and a failed query.""" self._refuse_failed_query(name, got, want) @@ -610,6 +678,7 @@ def text(self, got, want, name): raise AssertionError(f"{name}: got {got!r} want {want!r}") # -- SQLSTATE ---------------------------------------------------------- + @_resolving def sqlstate(self, exc, want, name): """Assert a raised database error carries EXACTLY this SQLSTATE. @@ -672,6 +741,7 @@ def sqlstate(self, exc, want, name): raise AssertionError(f"{name}: got SQLSTATE {got!r} want {want!r}") # -- plans ------------------------------------------------------------- + @_resolving def plan_node(self, plan, node_type=None, provider=None, name=None): """Assert a node exists, by EXACT equality on a typed EXPLAIN JSON field. @@ -718,6 +788,7 @@ def plan_node(self, plan, node_type=None, provider=None, name=None): ) # -- bounds ------------------------------------------------------------- + @_resolving def at_least(self, got, floor, name): """Assert got >= floor. Both sides must be numbers. @@ -740,6 +811,7 @@ def at_least(self, got, floor, name): raise AssertionError(f"{name}: got {got!r}, wanted at least {floor!r}") # -- the layer's own tests --------------------------------------------- + @_resolving def refusal(self, result, name, *patterns): """The inner run failed, AND it failed for the REASON named. @@ -787,6 +859,7 @@ def refusal(self, result, name, *patterns): # anywhere in the file. result.stdout.fnmatch_lines([f"E*{p}*" for p in patterns]) + @_resolving def outcomes(self, result, name, **want): """Assert on an INNER pytest run's outcomes, and count it. @@ -803,6 +876,7 @@ def outcomes(self, result, name, **want): self._record(name) result.assert_outcomes(**want) + @_resolving def run_failed(self, result, name): """Assert an inner run exited non-zero, and count it.""" self._record(name) @@ -811,6 +885,7 @@ def run_failed(self, result, name): f"{name}: the inner run exited 0, so nothing refused it." ) + @_resolving def plan_marker(self, plan, key, name=None, absent=False): """Assert a plan node carries (or does not carry) a Columnar property KEY. @@ -864,6 +939,7 @@ def plan_marker(self, plan, key, name=None, absent=False): ) # -- the third state --------------------------------------------------- + @_resolving def cannot_run(self, reason, detail=""): """Declare this test unrunnable. Not a pass, and not a silent skip. diff --git a/test/pytest/test_check_records.py b/test/pytest/test_check_records.py index 1c74beec..3b070ee4 100644 --- a/test/pytest/test_check_records.py +++ b/test/pytest/test_check_records.py @@ -120,3 +120,219 @@ def test_a_name_carrying_a_separator_survives_the_record(expect): 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") + + +# ---- phase 2: the verdict is resolved where the outcome is known ------------- +# +# The first version of this file's comment argued the verdict could be resolved +# from the exception in `pytest_runtest_call`, because assertions are sequential +# and a raise ends the test. @OffgridwithJD refuted it, and this corpus is what +# refutes it: proving a guard REFUSES means catching the AssertionError, which +# five tests do. Measured before the fix: +# +# count before/mid/after: 0 / 1 / 2 +# record 0 'this comparison must fail' verdict PASS <- this one RAISED +# record 1 'and the test continues' verdict PASS +# 1 passed +# +# A genuinely failed assertion stayed PASS, in a passing test, with nothing +# reaching the hook to correct it. So the verdict is set on the COMPARISON's own +# path instead, where the outcome is known and no propagation is needed. + + +def test_a_failed_assertion_records_fail_even_when_the_test_catches_it(expect): + """THE ARM FOR THE REFUTATION, and the shape five tests in this corpus use. + + Catching the AssertionError is how a test proves a guard refuses. If catching + it also erased the verdict, every one of those tests would be reporting on a + record stream that says its deliberate failure passed. + """ + e = pgc_vacuity.Expect("verdict::caught") + try: + e.num(1, 2, "this comparison must fail") + except AssertionError: + pass + expect.num(len(e.records), 1, "premise: the failed assertion was still counted") + expect.text(e.records[0].verdict, "FAIL", + "and it is recorded as FAIL, not as a pass the catcher hid") + + +def test_the_failure_reason_is_the_assertions_own_message(expect): + """A verdict with no reason sends the reader back to the source to find out + what happened. The message is the one the assertion already produces, not a + second one written for the record -- two messages for one failure is how they + drift.""" + e = pgc_vacuity.Expect("verdict::reason") + try: + e.num(1, 2, "one equals two") + except AssertionError as exc: + raised = str(exc) + expect.text(e.records[0].reason, raised, + "the recorded reason is the message the assertion raised") + + +def test_the_assertions_before_a_failure_keep_their_verdicts(expect): + """The verdicts are per assertion, not per test. A test that fails its third + assertion made two real claims first, and a stream that marked the whole test + would lose them.""" + e = pgc_vacuity.Expect("verdict::ordering") + e.num(1, 1, "first, true") + e.num(2, 2, "second, true") + try: + e.num(3, 4, "third, false") + except AssertionError: + pass + e.num(5, 5, "fourth, after the catch") + expect.ordered_rows([r.verdict for r in e.records], + ["PASS", "PASS", "FAIL", "PASS"], + "each assertion carries its own verdict, in order") + + +def test_a_delegated_assertion_records_fail_too(expect): + """`outcomes` and `refusal` hand the comparison to pytest's own + `assert_outcomes`, so the AssertionError is raised by code this layer does not + write and carries a message it did not compose. + + That is the case a verdict passed in at the call site could not cover, and it + is why the resolution wraps the comparison rather than describing it. + """ + e = pgc_vacuity.Expect("verdict::delegated") + + class _FakeResult: + ret = 0 + + def assert_outcomes(self, **want): + raise AssertionError("Outcomes do not match: expected passed=1") + + try: + e.outcomes(_FakeResult(), "a delegated comparison", passed=1) + except AssertionError: + pass + expect.num(len(e.records), 1, "premise: the delegated assertion was counted") + expect.text(e.records[0].verdict, "FAIL", + "and a failure raised by pytest's own code is still recorded") + + +def test_a_refusal_still_leaves_no_record(expect): + """The boundary, restated for phase 2 because the resolution wraps a region + that a refusal must stay outside of. + + Verified statically as well as here: in every recording method, each + `VacuityError` is raised BEFORE the record is taken, so no refusal is ever + inside the wrapped region. That is what keeps a refusal out of the stream + without a special case for `VacuityError` being a subclass of AssertionError. + """ + e = pgc_vacuity.Expect("verdict::refusal") + try: + e.num("100", 100, "a text comparison") + except pgc_vacuity.VacuityError: + expect.num(len(e.records), 0, "a refused assertion still records nothing") + else: + raise AssertionError("num() accepted a string, so this arm tested nothing") + + +def test_every_refusal_precedes_its_record(expect): + """The static half of the arm above, so the invariant cannot drift silently. + + If somebody adds a `VacuityError` after the record is taken, the refusal lands + inside the wrapped region and starts being recorded as a failed assertion. + Nothing else in the corpus would notice. + """ + import ast + import inspect + + tree = ast.parse(inspect.getsource(pgc_vacuity)) + scanned = [] + offenders = [] + for cls in ast.walk(tree): + if not (isinstance(cls, ast.ClassDef) and cls.name == "Expect"): + continue + for fn in cls.body: + if not isinstance(fn, ast.FunctionDef): + continue + recs = [n.lineno for n in ast.walk(fn) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) + and n.func.attr == "_record"] + if not recs: + continue + scanned.append(fn.name) + for n in ast.walk(fn): + if (isinstance(n, ast.Raise) and isinstance(n.exc, ast.Call) + and isinstance(n.exc.func, ast.Name) + and n.exc.func.id == "VacuityError" + and n.lineno > min(recs)): + offenders.append(f"{fn.name}:{n.lineno}") + # THE PREMISE IS THE POPULATION, and it is the difference between "no method + # offends" and "the scan matched no methods". An empty offender list is the + # answer to both, and only one of them is good news. + expect.at_least(len(scanned), 15, "premise: the scan found the recording methods") + expect.rows(offenders, [], "no refusal is raised after its record is taken", + allow_empty=True) + + +def test_every_recording_method_resolves_its_verdict(expect): + """THE LIST CANNOT DRIFT. A method that takes a record and is not wrapped + records a PASS it never revisits, so its failures are invisible in the stream + while the test still fails normally -- nothing else in the corpus would + notice. + + Derived from the module, not from a list written here: the population is every + method that calls `_record`, and the claim is that all of them are wrapped. + A list would have to be updated by whoever adds the sixteenth, which is + exactly the person who would forget. + """ + import ast + import inspect + + tree = ast.parse(inspect.getsource(pgc_vacuity)) + recording = [] + for cls in ast.walk(tree): + if not (isinstance(cls, ast.ClassDef) and cls.name == "Expect"): + continue + for fn in cls.body: + if not isinstance(fn, ast.FunctionDef): + continue + if any(isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) + and n.func.attr == "_record" for n in ast.walk(fn)): + recording.append(fn.name) + + expect.at_least(len(recording), 15, + "premise: the scan found the recording methods") + unwrapped = sorted( + nm for nm in recording + if not getattr(getattr(pgc_vacuity.Expect, nm), "_pgc_resolves_verdict", False) + ) + expect.rows(unwrapped, [], "every method that takes a record resolves its verdict", + allow_empty=True) + + +def test_a_recording_method_takes_exactly_one_record_per_call(expect): + """THE INVARIANT THE RESOLUTION RESTS ON, pinned because a mutation showed it + was assumed. + + `_resolving` marks `self._records[taken]`. Replacing that with + `self._records[-1]` left the whole corpus green, because one call appends at + most one record and the two always name it. That is a property of the methods, + not of the wrapper, and nothing was asserting it. + + If a method ever records twice, `[taken]` and `[-1]` stop agreeing, the + wrapper marks the first and the second keeps a verdict nobody set. This is the + arm that says so, rather than the comment. + """ + e = pgc_vacuity.Expect("records::one-per-call") + calls = [ + lambda: e.num(1, 1, "num"), + lambda: e.text("a", "a", "text"), + lambda: e.rows(["a"], ["a"], "rows"), + lambda: e.ordered_rows(["a", "b"], ["a", "b"], "ordered_rows"), + lambda: e.at_least(5, 1, "at_least"), + lambda: e.differ("x", "y", "differ"), + lambda: e.row_set(["a"], ["a"], "row_set -- delegates to rows"), + ] + deltas = [] + for call in calls: + before = e.count + call() + deltas.append(e.count - before) + expect.rows([str(d) for d in deltas], ["1"] * len(calls), + "every call, including the delegating one, took exactly one record") From 3b3469f2c0d1d8e799c735734a47195c48ec2bcb Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 09:35:46 -0600 Subject: [PATCH 2/3] test/pytest: the refusal-boundary scan must follow calls, not only literals (#937 phase 2) The arm asserted that no VacuityError is raised after its record is taken, which is what keeps refusals out of the record stream without a special case for VacuityError being an AssertionError subclass. If one lands inside the wrapped region it is recorded as a FAILED ASSERTION -- the stream lying in the most misleading direction available. The scan looked for a literal `raise VacuityError(...)` inside each method body. Most of these methods refuse through a HELPER (_refuse_failed_query), so a refusal moved after the record would have been invisible to it. Measured while attacking my own arm: methods that raise VacuityError directly 17 methods that can raise it, following calls 18 One method short of the real population. Both scans agree on today's code -- nothing calls a refusing helper after its record -- so this changes no verdict. It changes what the arm can see. Removal proof, the case the old scan could not catch: num() calling self._refuse_failed_query() after its record. old arm (literals only) would not have fired new arm 1 red, naming num:420 Verified: 242 driver-free. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/pytest/test_check_records.py | 79 +++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/test/pytest/test_check_records.py b/test/pytest/test_check_records.py index 3b070ee4..dc99a7a7 100644 --- a/test/pytest/test_check_records.py +++ b/test/pytest/test_check_records.py @@ -235,33 +235,74 @@ def test_every_refusal_precedes_its_record(expect): """The static half of the arm above, so the invariant cannot drift silently. If somebody adds a `VacuityError` after the record is taken, the refusal lands - inside the wrapped region and starts being recorded as a failed assertion. - Nothing else in the corpus would notice. + inside the wrapped region and starts being recorded as a FAILED ASSERTION. + Nothing else in the corpus would notice, and the stream would lie in the most + misleading direction available -- a refusal reported as a real failure. + + THIS SCAN FOLLOWS CALLS, and the first version did not. It looked for a + literal `raise VacuityError(...)` inside each method body, which would have + missed a refusal raised through a helper -- and `_refuse_failed_query` is + exactly such a helper, called by most of these methods. Measured while + attacking this arm: 17 methods raise it directly, 18 can raise it once calls + are followed, so the lexical scan was one method short of the real population. + + It happens that no method calls a refusing helper after its own record, so + both scans agree today. The arm is the transitive one anyway, because the + reason to write a guard is the case that does not exist yet. """ import ast import inspect tree = ast.parse(inspect.getsource(pgc_vacuity)) + methods = {} + for cls in ast.walk(tree): + if isinstance(cls, ast.ClassDef) and cls.name == "Expect": + for fn in cls.body: + if isinstance(fn, ast.FunctionDef): + methods[fn.name] = fn + + def _raises_here(fn): + return any(isinstance(n, ast.Raise) and isinstance(n.exc, ast.Call) + and isinstance(n.exc.func, ast.Name) + and n.exc.func.id == "VacuityError" + for n in ast.walk(fn)) + + def _self_calls(fn): + return [(n.func.attr, n.lineno) for n in ast.walk(fn) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) + and isinstance(n.func.value, ast.Name) and n.func.value.id == "self"] + + # Close over calls, so a method that refuses only through a helper is in the + # population too. + refusing = {nm for nm, fn in methods.items() if _raises_here(fn)} + growing = True + while growing: + growing = False + for nm, fn in methods.items(): + if nm in refusing: + continue + if any(attr in refusing for attr, _ in _self_calls(fn)): + refusing.add(nm) + growing = True + scanned = [] offenders = [] - for cls in ast.walk(tree): - if not (isinstance(cls, ast.ClassDef) and cls.name == "Expect"): + for nm, fn in methods.items(): + recs = [n.lineno for n in ast.walk(fn) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) + and n.func.attr == "_record"] + if not recs: continue - for fn in cls.body: - if not isinstance(fn, ast.FunctionDef): - continue - recs = [n.lineno for n in ast.walk(fn) - if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) - and n.func.attr == "_record"] - if not recs: - continue - scanned.append(fn.name) - for n in ast.walk(fn): - if (isinstance(n, ast.Raise) and isinstance(n.exc, ast.Call) - and isinstance(n.exc.func, ast.Name) - and n.exc.func.id == "VacuityError" - and n.lineno > min(recs)): - offenders.append(f"{fn.name}:{n.lineno}") + scanned.append(nm) + first = min(recs) + for n in ast.walk(fn): + if (isinstance(n, ast.Raise) and isinstance(n.exc, ast.Call) + and isinstance(n.exc.func, ast.Name) + and n.exc.func.id == "VacuityError" and n.lineno > first): + offenders.append(f"{nm}:{n.lineno} raises it directly after the record") + for attr, lineno in _self_calls(fn): + if attr in refusing and lineno > first: + offenders.append(f"{nm}:{lineno} calls self.{attr}() after the record") # THE PREMISE IS THE POPULATION, and it is the difference between "no method # offends" and "the scan matched no methods". An empty offender list is the # answer to both, and only one of them is good news. From cc77934cb262671c32e44218b37406a405dc9dcd Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 09:40:27 -0600 Subject: [PATCH 3/3] test/pytest: measure the drift arm's known gap instead of guarding it (#937 phase 2) @OffgridwithJD attacked the drift arm and named double-wrapping as the gap most likely to be reached: the marker sits on the outer wrapper, so a method wrapped twice is indistinguishable from one wrapped once. True, and it does not matter -- which is the answer rather than an excuse, and it is measured rather than argued: after double-wrapping @_resolving record 0 verdict FAIL 'a claim that is false: got 1 want 2' record 1 verdict PASS count 2 Both wrappers compute the same `taken` and resolve the same record to the same verdict and reason, because the inner call appends nothing before the outer one measures. The arm is NOT extended to catch it. A guard against a change that alters no behaviour is a false red waiting to happen, and this layer's budget forbids those more strictly than it forbids a gap. The gap is now pinned by a test that asserts the behaviour is identical, so if that ever stops being true the arm reddens for a reason that matters. "I think it is harmless" is the sentence that has been wrong three times today, so it is not the sentence shipped. Verified: 243 driver-free, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/pytest/TESTS.md | 1 + test/pytest/pgc_vacuity.py | 8 ++++++++ test/pytest/test_check_records.py | 33 +++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index cd6f597d..87b46565 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -2444,6 +2444,7 @@ and without this arm nothing would say so. | `test_every_refusal_precedes_its_record` | the static half, so that boundary cannot drift | | `test_every_recording_method_resolves_its_verdict` | all 15, derived from the module rather than listed | | `test_a_recording_method_takes_exactly_one_record_per_call` | the invariant the resolution rests on, pinned after a mutation showed it was assumed | +| `test_wrapping_a_method_twice_changes_nothing` | what the drift arm cannot see, measured and deliberately not guarded | **The first design was wrong and the corpus is what refuted it.** Resolving each verdict from the exception in `pytest_runtest_call` assumed a raise ends the test. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 5a360793..f5613ceb 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -297,6 +297,14 @@ def _resolving(method): comment is still true -- which is the opposite of how the first version of it would have aged. + WRAPPING TWICE CHANGES NOTHING, and the drift arm deliberately does not look + for it. The marker sits on the outer wrapper, so a doubly-wrapped method is + indistinguishable from a singly-wrapped one -- @OffgridwithJD named that as the + gap most likely to be reached. Measured: both wrappers compute the same + `taken` and write the same verdict and reason, because the inner call appends + nothing before the outer one measures. An arm against a change that alters no + behaviour would be a false red waiting to happen. + A REFUSAL MARKS NOTHING, and that needs no special case for VacuityError being an AssertionError subclass: every VacuityError in the recording methods is raised BEFORE the record is taken, so no record exists to mark. That is not an diff --git a/test/pytest/test_check_records.py b/test/pytest/test_check_records.py index dc99a7a7..770e2527 100644 --- a/test/pytest/test_check_records.py +++ b/test/pytest/test_check_records.py @@ -347,6 +347,39 @@ def test_every_recording_method_resolves_its_verdict(expect): allow_empty=True) +def test_wrapping_a_method_twice_changes_nothing(expect): + """WHAT THE ARM ABOVE CANNOT SEE, measured rather than left as a worry. + + @OffgridwithJD attacked the drift arm and named double-wrapping as the most + reachable thing it would miss: the marker is on the outer wrapper, so a method + wrapped twice looks exactly like one wrapped once. + + That is true, and it does not matter -- which is the answer, not an excuse. + Both wrappers compute the same `taken` and resolve the same record to the same + verdict and reason, because the inner call appends nothing before the outer one + measures. Measured here rather than argued, because "I think it is harmless" is + the sentence that has been wrong three times today. + + So the arm is not extended to catch it. A guard against a change that alters + nothing is a false red waiting to happen, and this layer's budget forbids those + more strictly than it forbids a gap. + """ + e = pgc_vacuity.Expect("double::wrapped") + original = pgc_vacuity.Expect.num + pgc_vacuity.Expect.num = pgc_vacuity._resolving(original) + try: + try: + e.num(1, 2, "a claim that is false") + except AssertionError: + pass + e.num(3, 3, "a claim that is true") + finally: + pgc_vacuity.Expect.num = original + expect.ordered_rows([r.verdict for r in e.records], ["FAIL", "PASS"], + "a doubly-wrapped method resolves exactly as a single one does") + expect.num(e.count, 2, "and still takes one record per call") + + def test_a_recording_method_takes_exactly_one_record_per_call(expect): """THE INVARIANT THE RESOLUTION RESTS ON, pinned because a mutation showed it was assumed.