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

test/pytest/TESTS.md

@@ -1517,10 +1517,9 @@ performs the setup inside the block:
 - **a compound statement.** A `for` over the setup and the statement under test is
   one statement holding two; an `if`, a `with` or a `try` nests the same way.
 
-Measured against the shipped scan, both report `1 passed`, exit 0, and **zero
-offences**. `test_a_helper_hiding_the_setup_is_not_refused` and
-`test_a_compound_statement_hiding_the_setup_is_not_refused` assert exactly that, so
-the residual is a measurement rather than a sentence. Counting statements
+Both were measured reporting `1 passed`, exit 0 and **zero offences**, with the setup
+raising and the statement under test never running. **Both are now refused**, and the
+arms that recorded them as residuals assert the refusal instead. Counting statements
 recursively would catch both and would also refuse a legitimate single-statement
 loop; what would close the mode is a claim about WHICH statement raised, and
 `VACUITY_MODES.md` section 5 carries it as the next entry.
@@ -1588,10 +1587,74 @@ one condition faithfully, and require the copy to go blind.
 | `test_sqlstate_refuses_an_empty_set_of_codes` | and an empty tuple is satisfied by nothing, so the hatch is not the hole |
 | `test_the_raises_scan_leaves_the_unrunnable_state_alone` | a documented hatch the corpus never exercises: `cannot_run` still prints `UNRUN`, counts it, and exits 67 with this scan loaded |
 | `test_the_raises_scan_does_not_touch_a_recorder_made_in_the_body` | a test that fetches `expect` itself still satisfies the layer, because this scan runs at collection time |
-| `test_a_helper_hiding_the_setup_is_not_refused` | **residual 1 of 2, pinned.** One statement, a narrow class, a pinned SQLSTATE, and the setup inside the helper still raised: `1 passed`, no offence |
-| `test_a_compound_statement_hiding_the_setup_is_not_refused` | **residual 2 of 2, pinned.** A `for` holding the setup and the statement under test is one top-level statement: `1 passed`, no offence |
+| `test_a_helper_hiding_the_setup_is_refused` | a call to a function **defined in the same file** cannot say which statement raised |
+| `test_a_compound_statement_hiding_the_setup_is_refused` | a `for` holding the setup and the statement under test is one top-level statement, and refused |
+| `test_a_helper_hidden_in_an_assignment_is_refused_too` | the rule looks anywhere in the statement: `x = _helper()` hides the setup as well as a bare call |
+| `test_every_compound_statement_is_refused_not_only_a_loop` | `if`, `while`, `with` and `try` nest the same way, so all nine compound kinds are refused |
+| `test_a_raises_block_calling_an_imported_function_is_accepted` | the budget: four of the five blocks in this corpus call an imported function |
+| `test_a_raises_block_calling_a_method_is_accepted` | the fifth block's shape, accepted, with the residual it leaves stated |
 | `test_a_conftest_cannot_switch_the_broad_family_list_off` | the rule's own family list is not writable from the corpus it polices |
 | `test_a_bare_sqlstate_expression_does_not_pin_anything` | `exc.value.sqlstate` as a statement of its own asserts nothing, so mentioning the field is not pinning it |
+
+### How `raises-catches-setup` narrowed, and what is left
+
+The count rule refuses a block holding more than one top-level statement. Two shapes
+are **one** statement and still hide the setup inside the block, so the count saw
+nothing:
+
+```python
+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: one statement
+        conn.execute(stmt)                      # holding two
+```
+
+The fix is **not** a recursive count — that would also refuse a legitimate
+single-statement loop. It is a claim about which statement raised, in two rules:
+
+- **No compound statement.** All nine kinds Python has, looked up by name rather than
+  written out so a missing `TryStar` or `Match` is not a NameError at import.
+- **No call to a function defined in the same file**, anywhere in the statement — a
+  helper hides as well in `x = _helper()` as in a bare call. 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: five
+`pytest.raises` blocks, four calling `build_and_install` (imported) and one calling a
+method, and the scan reports **no offence** on any of them.
+
+### What is still reachable, measured
+
+The mode stays in `VACUITY_MODES.md` section 3, and the refused count did not move,
+because two ordinary spellings still reach it:
+
+| 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 defined 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. And `local_defs` is built from one file, so
+moving the helper one file over defeats it. Neither is a contrivance; both are ordinary
+Python. The last row is the rule's cost rather than a gap — an honest single-statement
+local helper is refused, and the author must inline it.
+
+A method that performs setup and then the statement is invisible for the same reason,
+and no static rule can see inside it.
+
+**The arm that should have caught the overclaim did not.**
+`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 whichever state the document
+claims. A first version of this work wrote the closure into section 3, added the row to
+section 2, moved the count to 29, and that arm passed. It now also requires the mode to
+be named outside a closure back-reference and to be absent from section 2; all three
+shapes of the overclaim redden it. Residuals named by @jdatcmd on review.
 | `test_a_sqlstate_assigned_and_never_read_does_not_pin_anything` | the same hole one step on: bound to a name nothing uses |
 | `test_one_hop_through_a_local_name_is_an_honest_pin` | the cost side — `code = exc.value.sqlstate` then `expect.text(code, ...)` stays collectable |
 | `test_the_keyword_form_is_checked_by_both_rules` | `pytest.raises(expected_exception=...)` is not an exemption from either rule |

test/pytest/VACUITY_MODES.md

@@ -186,33 +186,48 @@ Still open in this family:
 `DatabaseError`, `Exception` or `BaseException` does not collect unless the block
 binds the exception and the body pins its SQLSTATE. See section 2.
 
-- `raises-catches-setup` — **still open, and the statement rule only narrows it.**
-  The scan refuses a `pytest.raises` block holding more than one TOP-LEVEL
-  statement, so the spelling where the setup sits on the line above the statement
-  under test is gone. Two shapes walk straight past a count of top-level
-  statements, and each is one statement that performs the setup inside the block:
+- `raises-catches-setup` — **narrowed again, and still not closed.** The statement COUNT
+  rule refused a block holding more than one top-level statement, and two shapes are ONE
+  statement that still performs the setup inside the block:
 
       with pytest.raises(psycopg.errors.UndefinedObject) as exc:
           _setup_then_run(conn)          # a HELPER CALL: one statement
-      expect.sqlstate(exc.value, "42704", "the ALTER was refused")
 
       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
-      expect.sqlstate(exc.value, "42704", "the ALTER was refused")
-
-  Measured against the shipped scan: both report `1 passed`, exit 0, **zero
-  offences**, with the setup raising and the statement under test never running.
-  An `if`, a `with` or a `try` nests the same way. Counting statements RECURSIVELY
-  would catch these and would also refuse a legitimate single-statement loop, so
-  the fix is not a deeper count — it is a claim about WHICH statement raised: a
-  position, or a helper that runs exactly one statement and owns the assertion.
-
-  `test_a_helper_hiding_the_setup_is_not_refused` and
-  `test_a_compound_statement_hiding_the_setup_is_not_refused` in
-  `test_raises_sqlstate.py` assert the scan reports nothing on these two shapes, so
-  the gap is a measurement rather than a sentence, and `test_raises_sqlstate.py` requires both
-  arms plus this entry to still exist.
+
+  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. Both are
+  refused now, by two rules: no compound statement (all nine kinds Python has, looked up
+  by name so a missing `TryStar` or `Match` cannot silently narrow the rule), and no call
+  to a function DEFINED IN THE SAME FILE, anywhere in the statement.
+
+  WHAT REMAINS, measured rather than reasoned, which is why this entry stays in section 3
+  and the refused count did not move:
+
+      a `for` loop over two statements                     REFUSED
+      the same two statements 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 defined in THIS file   REFUSED (a false positive)
+
+  A comprehension and a tuple are EXPRESSIONS rather than compound statements, so a rule
+  about statement kinds cannot see them; and `local_defs` is built from one file, so
+  moving the helper one file over defeats it. Both are ordinary Python, not contrivances.
+  The last row is the rule's cost rather than a gap: an honest single-statement local
+  helper is refused, and the author must inline it.
+
+  THE FIX IS NOT A DEEPER COUNT, for the reason this entry always gave: counting
+  recursively would also refuse a legitimate single-statement loop. What would close it
+  is a claim about which statement raised that does not depend on the SHAPE of the
+  statement -- a helper that runs exactly one statement and owns the assertion. Measured,
+  the corpus has no SQL-raising `pytest.raises` block at all, so that helper would have
+  no call sites today and would be an instrument with nothing exercising it.
+
+  The two arms that used to assert these shapes were NOT refused now assert that they
+  are, so the narrowing is a measurement rather than a sentence. Residuals named by
+  @jdatcmd on review. See TESTS.md section 20.
 - `same-broken-helper-both-sides`, `truthy-error-string`, `assert-not-unset-error`,
   `zero-on-both-arms`, `tuple-assert-always-true`, `approx-of-nothing`
 - `same-broken-helper-both-sides`, `truthy-error-string`, `assert-not-unset-error`,
@@ -361,9 +376,10 @@ Each entry names the red test to write first.
    It closes `raises-too-broad` and narrows `raises-catches-setup`, which stays
    open in 3.4 with the two shapes it cannot see named there. What would close the
    sibling is the next entry:
-5. `test_layer_requires_the_raiser_to_be_the_statement_under_test` — closes
-   `raises-catches-setup`. It needs a claim about WHICH statement raised, not a
-   deeper statement count; 3.4 says why a recursive count is the wrong fix.
+5. ~~`test_layer_requires_the_raiser_to_be_the_statement_under_test` — closes
+   `raises-catches-setup`.~~ **Done, and it NARROWS rather than closes** — 3.4 lists what
+   remains, measured: a comprehension or a tuple instead of a `for`, and a helper defined
+   in another file. Both are ordinary Python. The refused count therefore did not move.
 
 The three that turned a whole run green rather than one test are done. What remains
 is per-assertion work, so the ordering matters less: take the sentinel first, since

test/pytest/pgc_vacuity.py

@@ -1390,6 +1390,20 @@ def _sqlstate_pinned_names(fn):
     return pinned
 
 
+# Every statement that can HOLD other statements. Built by lookup rather than
+# written out, because `TryStar` and `Match` exist only on newer Pythons and a
+# missing name would be a NameError at import rather than a rule that quietly does
+# less. The inventory named only the `for` spelling; a rule catching only that one
+# would leave three spellings of the same shape, which is closing an example rather
+# than a mode.
+_COMPOUND_STATEMENTS = tuple(
+    c for c in (getattr(ast, n, None) for n in (
+        "For", "AsyncFor", "While", "If", "With", "AsyncWith", "Try", "TryStar",
+        "Match",
+    )) if c is not None
+)
+
+
 def _raises_sites(path):
     """Every `with pytest.raises(...)` in one file, and what is wrong with it.
 
@@ -1475,6 +1489,14 @@ def _raises_sites(path):
         return []
     out = []
     name = pathlib.Path(path).name
+    # EVERY FUNCTION THIS FILE DEFINES, nested ones included. A `pytest.raises` block
+    # whose one statement calls one of these is the helper shape: the helper can run
+    # any number of statements and nothing in the block says which of them failed. A
+    # call to an IMPORTED function, or a method, is the thing under test -- which is
+    # the shape all five blocks in this corpus use, so the rule turns on where the
+    # function is DEFINED rather than on the statement being a call.
+    local_defs = {n.name for n in ast.walk(tree)
+                  if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))}
     for fn in [n for n in ast.walk(tree)
                if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]:
         pinned = _sqlstate_pinned_names(fn)
@@ -1515,13 +1537,18 @@ def _raises_sites(path):
                 broad = [c for c in _raises_class_names(expected)
                          if c in broad_families]
                 if broad and (bound is None or bound not in pinned):
-                    # THE OFFENCE PHRASE STAYS ON ONE SOURCE LINE. The message
-                    # assembly below filters for this exact substring, and selftest
-                    # 440 counts both copies to catch them drifting apart. Split as
-                    # `"... names no " f"SQLSTATE"` it reads identically at runtime
-                    # and the arm counts one where it wants two -- a guard that can
-                    # no longer see its own drift. Measured: that split is what
-                    # reddened 440 the first time this scan ran under it.
+                    # THE OFFENCE PHRASE STAYS ON ONE SOURCE LINE, and the reason
+                    # has CHANGED. It was selftest 440, which grepped this source for
+                    # the phrase and counted the copies, so a split into
+                    # `"... names no " f"SQLSTATE"` read identically at runtime while
+                    # the count saw one where it wanted two. **Selftest 440 no longer
+                    # exists** -- #927 deleted it under the harness-independence rule,
+                    # because a shell part asserting a text pin cannot prove a python
+                    # arm is caught. Nothing greps this source for the phrase today, so
+                    # the one-line form is now a convention rather than a guarded
+                    # property. What IS still load-bearing is the RUNTIME string: the
+                    # arms in test_raises_sqlstate.py match it against stderr, and a
+                    # split f-string would not change that at all.
                     where = f"{name}:{call.lineno}"
                     out.append(
                         f"{where} pytest.raises({broad[0]}) names no SQLSTATE"
@@ -1535,6 +1562,42 @@ def _raises_sites(path):
                     f"{held} {len(node.body)} statements, "
                     f"so which one raised is not pinned"
                 )
+            # AND ONE STATEMENT IS NOT ENOUGH, which is the half `raises-catches-setup`
+            # stayed open on. Two shapes are one top-level statement and still hide the
+            # setup inside the block, so the count rule above saw nothing:
+            #
+            #     with pytest.raises(...): _setup_then_run(conn)   # a helper call
+            #     with pytest.raises(...):                         # a compound
+            #         for stmt in (setup, under_test): run(stmt)
+            #
+            # Both were measured reporting `1 passed`, exit 0, zero offences, with the
+            # setup raising and the statement under test never running. The fix is not a
+            # RECURSIVE count -- that would also refuse a legitimate single-statement
+            # loop -- it is a claim about which statement raised.
+            elif sites:
+                only = node.body[0]
+                kind = type(only).__name__
+                if isinstance(only, _COMPOUND_STATEMENTS):
+                    # One source line for the phrase, as above.
+                    out.append(
+                        f"{name}:{node.lineno} the pytest.raises block holds a {kind}, "
+                        f"so which statement inside it raised is not pinned"
+                    )
+                else:
+                    # ANYWHERE IN THE STATEMENT, not only as the whole of it: a helper
+                    # hides just as well in `x = _helper()` or `assert _helper()` as it
+                    # does in a bare call.
+                    called = sorted({
+                        n.func.id for n in ast.walk(only)
+                        if isinstance(n, ast.Call) and isinstance(n.func, ast.Name)
+                        and n.func.id in local_defs
+                    })
+                    if called:
+                        out.append(
+                            f"{name}:{node.lineno} the pytest.raises block calls "
+                            f"{called[0]}(), defined in this file, so which statement "
+                            f"raised is not pinned"
+                        )
     return out
 
 
@@ -1591,8 +1654,13 @@ def pytest_collection_modifyitems(config, items):
         excepts = [o for o in offenders if "catches Exception broadly" in o]
         ordered = [o for o in offenders if "feeds an ordered claim" in o]
         raises_broad = [o for o in offenders if "names no SQLSTATE" in o]
-        raises_setup = [o for o in offenders
-                        if "which one raised is not pinned" in o]
+        # THE FILTER IS THE COMMON TAIL OF ALL THREE PHRASES. It was the exact
+        # sentence of the statement-COUNT rule, so the two rules added for the helper
+        # and compound shapes refused the run and then printed NOTHING -- the layer
+        # said "refuses this run: ." and the arms could not tell a fired rule from an
+        # unfired one. Measured: both new arms reddened on a missing message while the
+        # refusal itself was working.
+        raises_setup = [o for o in offenders if "raised is not pinned" in o]
         parts = []
         if skips:
             parts.append(
@@ -1625,9 +1693,10 @@ def pytest_collection_modifyitems(config, items):
             )
         if raises_setup:
             parts.append(
-                "a pytest.raises block holding more than one statement cannot say "
-                "which statement raised, so a failure in the SETUP passes for a "
-                "failure in the statement under test: "
+                "a pytest.raises block must say WHICH statement raised, or a "
+                "failure in the SETUP passes for a failure in the statement under "
+                "test -- more than one statement, a compound statement holding "
+                "several, or a call to a helper defined in the same file all hide it: "
                 + "; ".join(raises_setup)
                 + " -- move the setup above the block, leaving the statement under "
                   "test alone inside it"