From 50c83770c8d2ebaaafdf8fc8396d50b830838cd0 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:41:27 +0000 Subject: [PATCH 1/2] test: a conftest cannot stub an Expect method so a false claim passes (#967) #964 snapshots module bindings. Expect.num = a stub that still counts is not a rebind of Expect, so 1 == 2 reported as a pass. Public methods are now snapshotted by identity. _record is excluded, so stubbing it still fails closed via count 0. --- CHANGELOG.md | 16 +++++++++ test/pytest/TESTS.md | 2 ++ test/pytest/pgc_vacuity.py | 56 ++++++++++++++++++++++++++++--- test/pytest/test_layer.py | 68 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 720ff606..e20b0c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1816,6 +1816,22 @@ true until the next version shipped. the extracted block holds one and the premise passes. The static caller sweep catches it -- injected, both the premise and the arm report `got [4] want [3]`. +- A `conftest.py` can no longer stub an `Expect` method so a false claim reports + as a pass (#967). + + #964 snapshots the module's bindings. `Expect.num = a stub that still counts` + is not a rebind of `Expect` -- the name still points at the same class -- so a + test asserting `1 == 2` printed `1 passed` and exited 0. That is strictly worse + than switching off a meta-rule: the comparison never happens, the count still + rises, and every guard downstream is satisfied by a test that concluded nothing. + + Public methods of `Expect` are now snapshotted by identity, the same way the + module bindings are. The refusal names `Expect.num` and restores the method + before raising, so an in-process `pytester` inner session cannot poison the + rest of the file. Names that start with `_` are excluded: stubbing `_record` + still leaves the count at 0 and is refused by `pytest_runtest_call`, which is a + different mechanism and the control this issue asked to keep. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8104a84e..1c36131b 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -266,6 +266,8 @@ the general case was hand-rolled. | `test_a_conftest_cannot_switch_off_the_broad_except_scan` | the same hatch, a second scan | | `test_a_conftest_cannot_switch_off_the_raises_scan` | and a third | | `test_the_refusal_names_the_binding_that_changed` | an honest run, refused, naming the conftest's name | +| `test_a_conftest_cannot_stub_an_expect_method_so_a_false_claim_passes` | a conftest replacing `Expect.num` with a stub that still counts is refused at collection, naming `Expect.num` | +| `test_stubbing_the_recorder_still_fails_closed_by_count` | **control**: stubbing `_record` still fails via count 0, so the public-method snapshot did not swallow the recorder's own protection | | `test_a_new_attribute_on_the_layer_is_not_a_rebind` | the control: a check that fires on anything is not a check | | `test_the_rebind_does_not_leak_into_this_session` | the binding is restored, so `pytester` does not poison the outer run | diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 10ded547..b85698a0 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -1999,6 +1999,13 @@ def _raises_sites(path): # assignment are not. It also checks at COLLECTION only, so a test that rebinds a # name inside its own body and restores it is untouched; two arms in # test_failed_query_sentinel.py do exactly that, legitimately. +# +# Module bindings are not the only writable surface. `Expect.num = a stub` leaves +# the name `Expect` pointing at the same class (#967), so the snapshot below cannot +# see it. Public methods of Expect are snapshotted separately, by identity, the +# same way. Leading-underscore names including `_record` are excluded: stubbing +# the recorder leaves the count at 0 and `pytest_runtest_call` refuses the test, +# which is a different mechanism and must stay the one that fires. def _binding_guard(): snapshot = {} missing = object() @@ -2020,12 +2027,45 @@ def restore(ns): return arm, changed, restore +def _public_attr_guard(cls): + """Snapshot the public attributes of a class, by identity. + + #964's module snapshot cannot see `Expect.num = a stub`: the binding + `Expect` is unchanged. This is the next frame (#967). Names that start + with `_` are excluded, so `Expect._record` stays the control: stubbing it + leaves the count at 0 and is refused by `pytest_runtest_call`, not here. + """ + snapshot = {} + missing = object() + + def arm(): + snapshot.update( + {k: v for k, v in cls.__dict__.items() if not k.startswith("_")} + ) + + def changed(): + return sorted( + f"{cls.__name__}.{k}" + for k, v in snapshot.items() + if cls.__dict__.get(k, missing) is not v + ) + + def restore(): + for k, v in snapshot.items(): + setattr(cls, k, v) + + return arm, changed, restore + + _arm_bindings, _changed_bindings, _restore_bindings = _binding_guard() +_arm_expect, _changed_expect, _restore_expect = _public_attr_guard(Expect) def pytest_collection_modifyitems(config, items, _changed=_changed_bindings, - _restore=_restore_bindings): + _restore=_restore_bindings, + _changed_methods=_changed_expect, + _restore_methods=_restore_expect): """Refuse a bare skip, which exits 0 and reads as success. Measured: two skipped tests report `2 skipped` and exit 0. A skip is allowed @@ -2035,13 +2075,18 @@ def pytest_collection_modifyitems(config, items, # Captured in this signature at DEFINITION time, so rebinding `_changed_bindings` # or `_restore_bindings` on the module does not reach what runs here. _rebound = _changed(globals()) - if _rebound: - _restore(globals()) + _methods = _changed_methods() + if _rebound or _methods: + if _rebound: + _restore(globals()) + if _methods: + _restore_methods() + names = _rebound + _methods raise pytest.UsageError( "the pgColumnar vacuity layer refuses this run: a conftest or plugin " "rebound the layer's own " - + ("names " if len(_rebound) > 1 else "name ") - + ", ".join(_rebound) + + ("names " if len(names) > 1 else "name ") + + ", ".join(names) + " -- a rule this layer enforces is read through that binding, so the " "run would have reported on rules that were switched off. The bindings " "have been restored. If a check is wrong for your case, say so where the " @@ -2153,3 +2198,4 @@ def pytest_collection_modifyitems(config, items, # module is fully executed before pytest imports any conftest, so nothing the policed # tree writes can be in the snapshot. _arm_bindings(globals()) +_arm_expect() diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index f153f4fe..52c071d3 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -647,6 +647,74 @@ def test_raises_compound(expect): hatched.stderr.fnmatch_lines(["*_raises_sites*"]) +def test_a_conftest_cannot_stub_an_expect_method_so_a_false_claim_passes(pytester, expect): + """#967. #964 snapshots module bindings. `Expect.num = a stub` is not a + rebind of `Expect` -- the name still points at the same class -- so a false + claim reports as a pass if the stub still increments the count. + + The three rows the issue named, and a fix must keep 1 and 3 while turning 2 + into a refusal: + + no conftest failed, correctly + Expect.num stubbed, still counting PASSED -- the hatch + Expect._record stubbed failed, count 0 (separate control) + """ + pytester.makepyfile( + """ + def test_one_equals_two(expect): + expect.num(1, 2, "one equals two, which it does not") + """ + ) + plain = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(plain, "premise: a false claim fails when nobody stubs", + failed=1, passed=0) + + import pgc_vacuity + original = pgc_vacuity.Expect.num + pytester.makeconftest( + "import pgc_vacuity\n" + "pgc_vacuity.Expect.num = " + "lambda self, got, want, name: self._record(name)\n" + ) + try: + hatched = pytester.runpytest("-p", "pgc_vacuity") + finally: + pgc_vacuity.Expect.num = original + expect.run_failed(hatched, "stubbing Expect.num so it still counts is refused") + hatched.stderr.fnmatch_lines(["*Expect.num*"]) + + +def test_stubbing_the_recorder_still_fails_closed_by_count(pytester, expect): + """#967 row 3. Stubbing `_record` leaves the count at 0, so the test is + refused for making no counted assertion. That is a different mechanism from + the public-method snapshot, and it must stay the one that fires -- a snapshot + of `_record` would swallow this into a collection-time refusal and the + control would no longer mean what it says. + """ + pytester.makepyfile( + """ + def test_one_is_one(expect): + expect.num(1, 1, "one is one") + """ + ) + import pgc_vacuity + original = pgc_vacuity.Expect._record + pytester.makeconftest( + "import pgc_vacuity\n" + "pgc_vacuity.Expect._record = lambda self, name: None\n" + ) + try: + result = pytester.runpytest("-p", "pgc_vacuity") + finally: + # pytester is in-process: the inner conftest writes the shared class. + # This arm deliberately does not snapshot `_record`, so nothing restores + # it for us -- and the outer expect.outcomes would then count nothing. + pgc_vacuity.Expect._record = original + expect.outcomes(result, "stubbing _record is refused by count 0, not by snapshot", + failed=1, passed=0) + result.stdout.fnmatch_lines(["*no counted assertion*"]) + + def test_the_refusal_names_the_binding_that_changed(pytester, expect): """A refusal that does not say what was rebound sends the reader to the wrong file. The offending test here is HONEST -- the only thing wrong with From 5d51adaf2cb97d6bc423126e5c79b262b3d7fb11 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sat, 12 Sep 2026 09:23:10 -0600 Subject: [PATCH 2/2] test: the method-stub refusal is asserted under xdist too (#967, #963) The merge two commits ago routed BOTH guarded surfaces through `_collection_usage_error`. Nothing asserted the new half. Raised by @OffgridwithJD, whose words were that a composition neither PR's CI could produce should stop being true the moment it is composed. WHICH GAP, PRECISELY, because their diagnosis named a different test and the distinction decides what to fix: test_the_refusal_names_the_binding_that_changed unparametrized on MAIN too, so a pre-existing gap the METHOD-stub test (#986's own) the surface this merge newly routes through the reporter Only the second is a gap this merge created, so only the second is fixed here. The first is real and belongs to whoever widens #963's table. REMOVAL PROOF, with the mutation asserted to have applied at an exact line -- my first attempt matched THREE call sites and did not apply at all, and reported "2 passed", which is the clean-pass-that-means-nothing this tree keeps catching: revert that call site to `raise pytest.UsageError(` [xdist] FAILED [serial] passed Serial is blind to it. That is the whole argument for the arm: without it, someone reverting this to a bare raise gets a green CI and a bare exit code under `-n`, which is the defect #963 exists to close. It reports through `_collection_refusal_row`, #963's own table row, so the arm asserts exit status, the sentence, AND zero INTERNALERROR lines rather than just a non-zero exit -- `run_failed` alone would have been satisfied by the very INTERNALERROR this closes. the arm, both modes 2 passed, 6 checks the 14 database-free files 267 passed, 633 checks, 0 fail No check renamed, so no ledger movement and TESTS.md needs no new row. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/pytest/test_layer.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index 2881049e..1fb23c39 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -729,7 +729,8 @@ def test_raises_compound(expect): hatched.stderr.fnmatch_lines(["*_raises_sites*"]) -def test_a_conftest_cannot_stub_an_expect_method_so_a_false_claim_passes(pytester, expect): +@pytest.mark.parametrize("mode", ["serial", "xdist"]) +def test_a_conftest_cannot_stub_an_expect_method_so_a_false_claim_passes(pytester, expect, mode): """#967, the class-attribute frame. #964 snapshots module bindings. `Expect.num = a stub` is not a rebind of `Expect` -- the name still points at the same class -- so a false claim reports as a pass if the stub still @@ -760,12 +761,20 @@ def test_one_equals_two(expect): "pgc_vacuity.Expect.num = " "lambda self, got, want, name: self._record(name)\n" ) + # BOTH MODES, because this surface reaches the reporter only after the merge + # that composed #963 and #967. Before it, the method branch raised + # `pytest.UsageError` directly -- and a UsageError raised in an xdist WORKER + # never reaches the controller, so `-n 2` gave a bare exit code instead of the + # sentence. Serial alone cannot see that: it is the same green either way. + extra = ("-n", "2") if mode == "xdist" else () try: - hatched = pytester.runpytest("-p", "pgc_vacuity") + hatched = pytester.runpytest("-p", "pgc_vacuity", *extra) finally: pgc_vacuity.Expect.num = original - expect.run_failed(hatched, "stubbing Expect.num so it still counts is refused") - hatched.stderr.fnmatch_lines(["*Expect.num*"]) + _collection_refusal_row( + hatched, expect, rc=4, reason_glob="*Expect.num*", + name=f"stubbed Expect.num {mode}", + ) def test_stubbing_the_recorder_still_fails_closed_by_count(pytester, expect):