test: a conftest cannot stub an Expect method on the class (#967)

CHANGELOG.md

@@ -2241,6 +2241,27 @@ true until the next version shipped.
   either a new ledger row or a rename that would orphan an existing one, and `main` has no
   tool to remove an orphan until #983 lands -- so the sequencing is written into the arm's
   comment rather than quietly skipped.
+- A `conftest.py` can no longer stub an `Expect` method on the class 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.
+
+  What this does not close: `expect.num = stub` on the instance, or a subclass
+  yielded by an overridden `expect` fixture. Both still keep the count and drop
+  the comparison. A snapshot of `Expect.__dict__` cannot see either. #967 stays
+  open for those two routes.
+
 - A collection-time vacuity refusal keeps its reason under pytest-xdist (#963).
 
   `pytest_collection_modifyitems` raises `UsageError`. Serial, that is rc 4 and

test/pytest/TESTS.md

@@ -296,6 +296,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` **on the class** 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 |
 
@@ -318,6 +320,12 @@ a lock: the criterion #924 set is that silencing a rule must cost more than stat
 reason. **The shell harness needs no equivalent** — `selftest/260` greps `lib.sh` from
 a separate process and never sources the file it judges.
 
+**#967 closes the class, not the instance.** `pgc_vacuity.Expect.num = stub` is
+refused at collection. `expect.num = stub` on the instance, or a subclass yielded
+by an overridden `expect` fixture, still keep the count and drop the comparison:
+`Expect.__dict__` is unchanged, so the snapshot cannot see them. A stub that still
+records satisfies the zero-assertion backstop. Those two routes stay #967.
+
 
 **Two failed queries are not two observable arms.** `query_error()` produces a value
 unique per occurrence precisely so two failures cannot compare **equal** and pass an

test/pytest/pgc_vacuity.py

@@ -2050,6 +2050,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()
@@ -2071,12 +2078,48 @@ 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), and it is the
+    CLASS dictionary. An instance attribute or a subclass yielded by an
+    overridden fixture is a different object: `Expect.__dict__` is untouched,
+    so this snapshot cannot see it. 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(session, 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
@@ -2086,14 +2129,22 @@ def pytest_collection_modifyitems(session, 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
+        # #963's reporter rather than a bare `raise`: a UsageError raised in a
+        # WORKER never reaches the controller, so the refusal arrived as a bare
+        # exit code. Both surfaces this hook now guards report through it.
         _collection_usage_error(
             session, config, items,
             "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 "
@@ -2210,3 +2261,4 @@ def pytest_collection_modifyitems(session, 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()

test/pytest/test_layer.py

@@ -729,6 +729,85 @@ def test_raises_compound(expect):
     hatched.stderr.fnmatch_lines(["*_raises_sites*"])
 
 
+@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
+    increments the count. An instance attribute or a subclass fixture is a
+    different object and is not this test.
+
+    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"
+    )
+    # 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", *extra)
+    finally:
+        pgc_vacuity.Expect.num = original
+    _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):
+    """#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