diff --git a/CHANGELOG.md b/CHANGELOG.md index ee62d1c1..62dea51f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,84 @@ true until the next version shipped. ### Added +- A broad `pytest.raises` must name a SQLSTATE, and the block must hold one + statement (#432). + + `pytest.raises(psycopg.Error)` claims that one of 254 SQLSTATEs arrived, across 42 + SQLSTATE classes, counted against psycopg 3.3.5. It does not claim even that much. + Measured on this tree before the guard landed, this reported `1 passed`, exit 0: + + with pytest.raises(psycopg.Error): + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + conn.execute("SELECT pgc_definitely_no_such_function()") + expect.num(1, 1, "the server rejected the call") + + What satisfied it was `OperationalError` with `sqlstate` None: the connect failed, + nothing reached a server, and the statement under test never executed. + + `pgc_vacuity.py` now refuses two shapes at collection time, found by walking the + `ast` rather than matching lines, so an offending file does not collect at all. A + `raises` over `Error`, `DatabaseError`, `Exception` or `BaseException` must bind the + exception and pin its SQLSTATE, and any `raises` block must hold exactly one + top-level statement. `expect.sqlstate(exc.value, "42883", name)` is the honest form + the refusal points at; it compares a typed field, refuses a two-character SQLSTATE + class as the prefix claim it is, and refuses an empty set of codes so the tuple + escape hatch cannot become the hole. + + The family list is bound inside the scan rather than at module level. Every + `conftest.py` under `test/pytest/` is imported before collection, so a module-level + tuple is writable from the corpus the rule polices -- `import pgc_vacuity` then + `pgc_vacuity. = ()` -- after which the scan reports zero offences for + ever and the suite is green with the guard off and nothing saying so. An arm writes + three spellings of the name onto the module and requires the refusal to still + arrive. + + This CLOSES `raises-too-broad`, which moves to `VACUITY_MODES.md` section 2, and + only NARROWS `raises-catches-setup`, which stays in section 3.4. The statement rule + counts TOP-LEVEL statements, so two shapes still walk past it, each being one + statement that performs the setup inside the block: a call to a helper, and a + compound statement such as a `for` holding the setup and the statement under test. + Both are measured at `1 passed`, exit 0, zero offences, and both have an arm + asserting the scan reports nothing on them, so the residual is a measurement rather + than a sentence. A recursive statement count would catch them and would also refuse + a legitimate single-statement loop; what would close the mode is a claim about which + statement raised. + + Parsed rather than grepped, because the suite writes the forbidden shape inside a + `pytester.makepyfile` string in every arm. Over `test/pytest/*.py` the `ast` finds + 5 `pytest.raises` call sites and reports 0 offences, while a `pytest.raises(` line + regex matches 35 lines, 30 of them inside a string literal or a comment. Swapping + `ast.parse` for that regex makes the layer refuse its own test suite with 22 + invented offences and exit 4, which is mutation 11 of 11 in the removal proof. + + THE ARMS LIVE IN ONE HARNESS. `test/pytest/test_raises_sqlstate.py` carries all of + them, through `pytester` and through the scan directly. An earlier version of this + change also shipped a shell mirror, `test/selftest/440-a-raises-must-name-a-sqlstate.sh`, + which checked the scan by GREPPING ITS SOURCE: 44 of its 55 checks were `grep -c` + against the function's text and it invoked `python3` zero times. Reviewing it, + @linuxhikerpm measured three faithful neuterings -- `False and` prefixed, nothing + renamed, every pinned substring left in place -- and all three left that part at 55 + passed while the scan went blind. A text pin catches a rewrite or a deletion; it + cannot catch `False and`, which is how a guard actually dies. + + The mirror is gone, and the second reason is the one that settles it: the shell + harness and the pytest corpus are parallel in functionality and do not drive each + other. A shell part whose whole subject is another harness's source text is a + dependency rather than a parallel guard -- it asserts against an implementation + instead of against the product. So the neutering proof is now two arms that copy the + layer, disable one condition faithfully, and require the copy to go blind while still + containing the text a grep arm would have pinned. + + THREE DEFECTS @linuxhikerpm FOUND IN THE GUARD ITSELF, each reproduced before it was + fixed. A bare `exc.value.sqlstate`, and a `code = exc.value.sqlstate` never read, both + satisfied the pin while asserting nothing -- the rule counted any attribute named + `sqlstate` anywhere in the body. It now requires the read to reach a CALL, following + one hop of assignment so the honest `code = ...` / `expect.text(code, ...)` form is not + refused. And `pytest.raises(expected_exception=...)` escaped BOTH rules, because the + class was read from `call.args[0]` and the item was skipped before it was recorded, + which made the statement rule silently conditional on the class being positional while + the documentation stated it unconditionally. + - Exact zone-map boundary coverage now lives in matching shell and pytest tests (#831). diff --git a/test/pytest/README.md b/test/pytest/README.md index 7f19f184..0a5745f2 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -5,7 +5,7 @@ This is the issue #432 pilot. It runs beside `test/*.sh`, and replaces nothing. - `TESTS.md` in this directory documents every test and every assertion helper. - `VACUITY_MODES.md` is the inventory of ways a pytest harness can report a false pass: 79 modes produced by the enumeration, 72 of them named in that - file, 73 demonstrated by a run, and 25 refused by this layer today. + file, 73 demonstrated by a run, and 26 refused by this layer today. VACUITY_MODES.md section 1a gives the counting rule and reconciles the run's totals against what is actually written down. - `design/ISSUE_432_PYTEST_HARNESS.md` holds the design and the measurements diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 2f591ddd..5551594e 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -63,6 +63,7 @@ behaviour, the source of that number is named. - [15. Adding a test](#15-adding-a-test) - [16. What this corpus does NOT yet refuse](#16-what-this-corpus-does-not-yet-refuse) - [17. Traps this corpus records](#17-traps-this-corpus-records) +- [18. test_raises_sqlstate.py: which error, and which statement](#18-test_raises_sqlstatepy-which-error-and-which-statement) ## 1. How to read a test in here @@ -1178,14 +1179,23 @@ story from #473 and #476. ## 16. What this corpus does NOT yet refuse `VACUITY_MODES.md` is the inventory: 79 ways a pytest harness can report a pass while -asserting nothing, 73 of them demonstrated by an actual run. **This layer refuses 25 -of them.** The other 47, of which 46 were demonstrated, are listed there with the +asserting nothing, 73 of them demonstrated by an actual run. **This layer refuses 26 +of them.** The other 46, of which 45 were demonstrated, are listed there with the refusal design each would need and the order worth building them in. -Read it before adding a test. The gaps most likely to affect a new test are that -`pytest.raises` is still allowed to be broad enough that an unrelated failure of the -same family satisfies it, and that a write is not required to have written anything. -Both are named there with the refusal each needs. +Read it before adding a test. Two gaps are most likely to affect a new test now. + +**A write is not required to have written anything.** `INSERT ... SELECT ... WHERE +false` writes nothing, raises nothing, and leaves a `rowcount` of 0 that nobody +reads. + +**A `pytest.raises` block can still catch a failure from its own setup.** Section 17 +closed `raises-too-broad` — a broad family with no SQLSTATE pinned does not collect — +and only NARROWED `raises-catches-setup`. The block must hold one top-level +statement, so two shapes still walk past it: a call to a helper that performs the +setup, and a compound statement such as a `for` holding the setup and the statement +under test. Both are pinned by arms that assert the scan reports nothing on them, and +`VACUITY_MODES.md` section 3.4 says what would close the mode. ## 17. Traps this corpus records @@ -1218,3 +1228,131 @@ process. Walking `/proc//cmdline` is the reliable instrument. `test_one_tree_hashes_one_way_however_the_locale_is_set` requires one tree to give one fingerprint across every installed locale. + +## 18. test_raises_sqlstate.py: which error, and which statement + +Numbered 17 rather than inserted after section 4, where a reader looking for a +per-file section would expect it. Renumbering twelve headings and their Contents +anchors while sibling branches are editing this file buys a reader nothing and +costs a merge; the Contents entry above is what makes it findable. + +**What this file is for.** `pytest.raises(psycopg.Error)` claims that one of 254 +SQLSTATEs arrived, across 42 SQLSTATE classes — counted against psycopg 3.3.5 by +asking how many classes in `psycopg.errors` carry a `sqlstate` and subclass that +family. It does not claim even that much. Measured on this tree before the guard +landed: + + with pytest.raises(psycopg.Error): + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + conn.execute("SELECT pgc_definitely_no_such_function()") + expect.num(1, 1, "the server rejected the call") + +reported `1 passed`, exit 0. What satisfied the claim was `OperationalError` with +`sqlstate` None: the connect failed, nothing reached a server, and the statement the +test is about never executed. Against a live PostgreSQL 18.4 the same shape raises +`InvalidName` 42602 from the SETUP line while the statement under test raises +`UndefinedObject` 42704 — two different SQLSTATEs, one `raises`, one green test. + +### Both directions are enforced, and they close different amounts + +A `pytest.raises` over `Error`, `DatabaseError`, `Exception` or `BaseException` must +pin a SQLSTATE, and the block must hold exactly one top-level statement whatever the +class. Neither is a convention: both are an `ast` walk in +`pytest_collection_modifyitems`, so an offending file does not collect at all rather +than collecting and passing. + +**The first rule closes `raises-too-broad`.** It moved to `VACUITY_MODES.md` +section 2. + +**The second only MITIGATES `raises-catches-setup`, which stays in section 3.4.** +It counts TOP-LEVEL statements, so it removes the spelling where the setup sits on +the line above — and two shapes walk straight past it, each being one statement that +performs the setup inside the block: + +- **a helper call.** `_setup_then_run(conn)` is one statement, and the setup runs + inside the helper. +- **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 +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. + +### Why the scan parses instead of grepping + +Every arm below writes the forbidden shape inside a `pytester.makepyfile` string, +because that is how the layer's own tests drive an inner run. A line regex fires on +those strings and refuses the file that proves the guard — the false positive the +broad-`except` scan already paid for once. Swept over `test/pytest/*.py`, the AST +finds **5** `pytest.raises` call sites and reports **0** offences, while a +`pytest.raises(` line regex matches **35** lines, **30** of them inside a string +literal or a comment. `test_the_raises_scan_reads_code_not_a_string_literal` pins +it. + +### The rule's own parameters are not reachable from the corpus + +The list of broad families is bound **inside** the scan, not at module level. Every +`conftest.py` under `test/pytest/` is imported before collection, so a module-level +tuple is writable from the tree the rule polices — `import pgc_vacuity` then +`pgc_vacuity. = ()` — after which the scan reports zero offences for ever +and the suite is green with the guard switched off and nothing saying so. +`test_a_conftest_cannot_switch_the_broad_family_list_off` writes three plausible +spellings of the name onto the module and requires the refusal to still arrive. +Rebinding the scan FUNCTION from a conftest is still possible; that is true of every +name in every Python plugin, and `test_guards_pinned.py` is what +notice a scan that stopped being called. + +### The static half + +**THE ARMS LIVE HERE AND NOWHERE ELSE.** An earlier version of this work carried a +shell mirror, `test/selftest/440-a-raises-must-name-a-sqlstate.sh`, which checked this +scan by grepping its source: 44 of its 55 checks were `grep -c` against the function's +text and it invoked `python3` zero times. @jdatcmd showed what that cannot do — +three faithful neuterings (`False and` prefixed, nothing renamed, every pinned +substring left in place) left the part at 55 passed while the scan went blind. + +The mirror is gone, for two reasons that point the same way. A text pin cannot see a +disabled arm, so the proof has to RUN the scan; and the shell harness and this corpus +are **parallel in functionality without driving each other** — a shell part whose whole +subject is this file's source text is a dependency, not a parallel guard. So the +neutering proof is the two `test_disabling_*` arms above, which copy the layer, disable +one condition faithfully, and require the copy to go blind. + +### The arms + +| test | what it pins | +| --- | --- | +| `test_raises_requires_a_sqlstate` | the red test `VACUITY_MODES.md` section 5 names, byte for byte the shape that reported `1 passed` on main | +| `test_a_raises_that_pins_the_sqlstate_is_accepted` | the positive control that matters most: the honest form must still collect and pass | +| `test_a_raises_pinned_by_reading_the_field_is_accepted` | the second honest spelling, `exc.value.sqlstate` read directly, is a claim about a typed field too | +| `test_a_narrow_raises_needs_no_sqlstate` | scope control: a one-SQLSTATE class already names the error, so a second spelling would be noise | +| `test_a_raises_tuple_hides_a_broad_member` | @jdatcmd's #905 hole, closed before shipping: `(ValueError, psycopg.Error)` is still broad | +| `test_raises_exception_is_refused_like_a_broad_except` | `except Exception` was already uncollectable; `pytest.raises(Exception)` swallows the same failures | +| `test_setup_inside_a_raises_block_is_refused` | two statements in the block: narrow and pinned, and still unable to say which raised | +| `test_a_raises_block_with_one_statement_is_accepted` | the control for it, differing in exactly one property — the setup moved above the block | +| `test_the_raises_scan_reads_code_not_a_string_literal` | the false positive the scan is AST-based to avoid, pinned so it cannot return | +| `test_sqlstate_refuses_a_sqlstate_class_prefix` | `"42"` is a SQLSTATE CLASS — a prefix claim wearing the spelling of an exact one | +| `test_sqlstate_refuses_an_empty_expectation` | an empty `want` names no error, so nothing could have failed it | +| `test_sqlstate_refuses_an_object_carrying_no_sqlstate` | passing `exc` instead of `exc.value` would compare `None` against a real code for ever | +| `test_sqlstate_fails_when_the_failure_never_reached_the_server` | the measured case: `OperationalError` with `sqlstate` None is a `psycopg.Error` that is no server error | +| `test_sqlstate_fails_on_a_different_sqlstate` | the whole point: the setup raised 42602 and the statement under test raises 42704 | +| `test_sqlstate_accepts_the_exact_sqlstate` | positive control for the refusals above | +| `test_sqlstate_accepts_one_of_several_named_codes` | majors 15 through 19 can differ, so a tuple widens the claim by exactly the codes it names | +| `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_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 | +| `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_the_keyword_form_with_a_pin_is_collectable` | and it is not refused merely for being the keyword form | +| `test_disabling_the_sqlstate_rule_makes_the_scan_blind` | the neutering proof: a copy of the layer with `False and` prefixed, nothing renamed, goes blind while still containing the pinned text | +| `test_disabling_the_statement_rule_makes_the_scan_blind` | the same for the second condition, so neither rule rests on the other's arm | +| `test_the_mode_this_layer_only_narrows_is_still_listed_as_open` | `raises-catches-setup` must stay in section 3 of the mode inventory | diff --git a/test/pytest/VACUITY_MODES.md b/test/pytest/VACUITY_MODES.md index 8ffb92ac..2d2d173b 100644 --- a/test/pytest/VACUITY_MODES.md +++ b/test/pytest/VACUITY_MODES.md @@ -47,8 +47,8 @@ recollection of the run: | | modes | | --- | ---: | -| named in section 2, refused today | 25 | -| named in section 3, not refused | 47 | +| named in section 2, refused today | 26 | +| named in section 3, not refused | 46 | | **named in this document** | **72** | | produced by the enumeration run | 79 | | **named nowhere here** | **7** | @@ -65,7 +65,7 @@ an id can be read, argued with and turned into a test, and a number cannot. ## 2. What the layer refuses today -25 of the 79, counted by section 1a's rule. Each is enforced by a mechanism, not a convention, and each has a red +26 of the 79, counted by section 1a's rule. Each is enforced by a mechanism, not a convention, and each has a red test in `test_layer.py` that fails without it. | mechanism | modes it closes | @@ -86,6 +86,7 @@ test in `test_layer.py` that fails without it. | the reported node-id set is reconciled against the collected one | `crashed-worker-silently-loses-tests` | | an empty parameter set fails the run, with its own message | `empty-parametrize-is-a-silent-skip` | | a skip during fixture setup fails the run | `session-fixture-skip-greens-the-whole-suite` | +| a broad `pytest.raises` must pin a SQLSTATE, found by AST | `raises-too-broad` | Three of those were added after checking this layer against the inventory rather than reasoning about it, and all three had passed silently before: @@ -118,7 +119,7 @@ guard whose subject is false greens has no business emitting a false red. ## 3. What it does not refuse -55 modes by the run's count, **47 of them named below**, **49 demonstrated by a run**. 51 have a refusal already designed. +55 modes by the run's count, **46 of them named below**, **49 demonstrated by a run**. 51 have a refusal already designed. Grouped by what a reader needs to decide about them. ### 3.1 The run can lose tests and still exit 0 @@ -179,8 +180,39 @@ Still open in this family: ### 3.4 The assertion is shaped so it cannot fail -- `raises-too-broad`, `raises-catches-setup` — `pytest.raises(psycopg.Error)` is - satisfied by an unrelated failure of the same family. +**`raises-too-broad` is now closed.** A `pytest.raises` over `Error`, +`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: + + 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. +- `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`, `zero-on-both-arms`, `tuple-assert-always-true`, `approx-of-nothing` @@ -236,6 +268,25 @@ now parses with `ast`, where a handler inside a string literal is not an `ExceptHandler` node. **A line regex over source cannot tell code from a string, which is the same mistake as matching a plan by substring.** +The `raises` scan was written with `ast` for the same reason, and the cost of the +alternative is measured rather than argued. Swept over +`test/pytest/*.py` — 16 files, a superset of the 12 the collection hook reaches, +because a module that is not collected today can be collected tomorrow: + +| | count | +| --- | ---: | +| `pytest.raises` call sites the AST finds | 5 | +| offences the scan reports on them | **0** | +| lines a `pytest.raises(` line regex would match | 35 | +| of those, inside a string literal or a comment | 30 | + +The 30 are not hypothetical. 22 are in `test_raises_sqlstate.py`, which writes the +forbidden shape inside a `pytester.makepyfile` string in every arm, and 8 are in +`pgc_vacuity.py` itself, where the scan's own comments quote what it refuses. +Replacing `ast.parse` with a line regex is mutation M11 of the removal proof: the +layer then refuses **its own test suite** with 22 invented offences, `pytest.UsageError`, +exit 4, and no tests run at all. + ## 5. What to add next, in order Each entry names the red test to write first. @@ -244,7 +295,13 @@ Each entry names the red test to write first. `QUERY_ERROR.`; the constant exists and nothing writes it. 2. `test_layer_requires_a_write_to_have_written` — closes `insert-wrote-no-rows`. 3. `test_layer_requires_ab_arms_to_differ` — closes `mutation-arm-unobservable`. -4. `test_raises_requires_a_sqlstate` — closes `raises-too-broad`. +4. ~~`test_raises_requires_a_sqlstate` — closes `raises-too-broad`.~~ **Done.** + 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. 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 @@ -257,6 +314,6 @@ have tried to defeat them did not run. Every design states its own residual, and those residuals are the authors' own, unchallenged. So treat §2 as measured, §3 as measured, and §5 as a plan that has not yet met an -adversary. The layer is known to refuse 25 demonstrated modes -- the ids named in section 2, +adversary. The layer is known to refuse 26 demonstrated modes -- the ids named in section 2, not the run's larger total, for the reason section 1a gives. It is not known to be undefeatable on any of them. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index e312cbef..24dca22d 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -65,6 +65,13 @@ def _empty(v): return v is None or (hasattr(v, "__len__") and len(v) == 0) +def _is_sqlstate(v): + # Five characters of [0-9A-Z], per SQL/PostgreSQL. Explicit ranges rather than + # str.isdigit(), which is True for other scripts' digits. + return (isinstance(v, str) and len(v) == 5 + and all(("0" <= c <= "9") or ("A" <= c <= "Z") for c in v)) + + def _plan_nodes(node): """Every node of an EXPLAIN (FORMAT JSON) tree, as parsed by psycopg.""" if isinstance(node, dict): @@ -250,6 +257,68 @@ def text(self, got, want, name): if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") + # -- SQLSTATE ---------------------------------------------------------- + def sqlstate(self, exc, want, name): + """Assert a raised database error carries EXACTLY this SQLSTATE. + + `pytest.raises(psycopg.Error)` asserts that one of 254 SQLSTATEs arrived, + across 42 SQLSTATE classes -- measured against psycopg 3.3.5 in the audit + container by counting the classes in `psycopg.errors` that carry a + `sqlstate` and subclass `psycopg.Error`. An unrelated failure of the same + family satisfies it, and the worst case is not even a server error: a + connect to a socket that does not exist raises `OperationalError` with + `sqlstate` None, having never reached a server at all. + + So this is the typed field that says WHICH error, and it is the same move + `plan_marker` makes: a typed field rather than a substring of a message. + `str(exc.value).count("does not exist")` is the grep this layer exists to + remove, wearing a different spelling. + + `want` may be a tuple when an error code legitimately differs across + majors -- this tree supports 15 through 19. Every member is still checked + to be a real SQLSTATE, so a tuple widens the claim by exactly the codes it + names and no further. + + Refuses, rather than compares: + + - a `want` that is not five characters of [0-9A-Z]. `""` and `None` are + satisfied by nothing, and `"42"` is a SQLSTATE CLASS -- a prefix claim + wearing the spelling of an exact one. + - an `exc` with no `sqlstate` attribute at all. The usual cause is passing + pytest's `ExceptionInfo` instead of `exc.value`, which would otherwise + compare `None` against a real SQLSTATE for ever. + """ + wants = tuple(want) if isinstance(want, (tuple, list)) else (want,) + if not wants: + raise VacuityError( + f"{name}: an empty set of SQLSTATEs is satisfied by nothing, so " + f"this could not have passed and asserts nothing about which " + f"error arrived." + ) + for w in wants: + if not _is_sqlstate(w): + raise VacuityError( + f"{name}: {w!r} is not a SQLSTATE. A SQLSTATE is five " + f"characters of [0-9A-Z]; a two-character class is a prefix " + f"claim, and an empty one names no error." + ) + if not hasattr(exc, "sqlstate"): + raise VacuityError( + f"{name}: a {type(exc).__name__} carries no sqlstate, so this " + f"comparison is about the wrong object. Pass the exception itself: " + f"`exc.value` inside a `with pytest.raises(...) as exc` block, not " + f"`exc`." + ) + self._counted() + got = exc.sqlstate + if got is None: + raise AssertionError( + f"{name}: a {type(exc).__name__} carrying no SQLSTATE, so the " + f"failure never reached the server: {exc}. Wanted {want!r}." + ) + if got not in wants: + raise AssertionError(f"{name}: got SQLSTATE {got!r} want {want!r}") + # -- plans ------------------------------------------------------------- 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. @@ -841,6 +910,255 @@ def _broad_except_sites(path): return out +def _walk_own(node): + """Walk one function body, NOT descending into a nested def or lambda. + + A nested function is its own scope. `ast.walk` would attribute its `with` + blocks to the outer function as well, reporting one site twice under two + different sets of pinned names. + """ + stack = list(getattr(node, "body", [])) + while stack: + child = stack.pop() + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + continue + yield child + stack.extend(ast.iter_child_nodes(child)) + + +def _raises_class_names(arg): + """The tail identifiers a pytest.raises() first argument names. + + `psycopg.Error` -> ["Error"]. `(ValueError, psycopg.Error)` -> both. + + A TUPLE IS THE SHAPE PEOPLE ACTUALLY WRITE, and it is how a narrow claim gets + widened under pressure. The broad-except scan paid for that lesson already: + it looked only at a bare `ast.Name`, so `except Exception` was refused while + `except (ValueError, Exception)` passed (@jdatcmd, #905 review). Same hole, + same shape, closed here before it was shipped rather than after. + """ + out = [] + for node in (arg.elts if isinstance(arg, ast.Tuple) else [arg]): + if isinstance(node, ast.Name): + out.append(node.id) + elif isinstance(node, ast.Attribute): + out.append(node.attr) + return out + + +def _root_name(node): + while isinstance(node, (ast.Attribute, ast.Subscript)): + node = node.value + return node.id if isinstance(node, ast.Name) else None + + +def _sqlstate_pinned_names(fn): + """Names whose SQLSTATE this function body ASSERTS something about. + + Two spellings, because both are honest and the scan must accept whichever the + caller chose: + + expect.sqlstate(exc.value, "42883", name) # the helper + expect.text(exc.value.sqlstate, "42883", name) # the field, read directly + + THE ATTRIBUTE HAS TO REACH A CALL. The first version counted any `ast.Attribute` + named `sqlstate` anywhere in the body, so MENTIONING the field switched the rule + off. Measured, both collecting clean against the first version and both being the + exact vacuity this rule is named for -- any of the 254 SQLSTATEs satisfies them: + + exc.value.sqlstate # a bare expression, asserts nothing + code = exc.value.sqlstate # assigned, never read + + Reported by @jdatcmd, who ran the scanner over six constructed files rather than + reading it. + + So a read counts when it is an ARGUMENT to a call, and one hop of assignment is + followed -- `code = exc.value.sqlstate` then `expect.text(code, ...)` is honest and + common, and refusing it would be a false positive on a form nobody should have to + stop writing. A second hop is not followed: this is a floor, and the floor is + stated rather than implied. + """ + pinned = set() + # Names a sqlstate read was assigned to, and names that appear as call arguments. + assigned_from_sqlstate = {} + call_arg_names = set() + for node in _walk_own(fn): + if isinstance(node, ast.Call): + for arg in list(node.args) + [k.value for k in node.keywords]: + for sub in ast.walk(arg): + if isinstance(sub, ast.Name): + call_arg_names.add(sub.id) + if isinstance(sub, ast.Attribute) and sub.attr == "sqlstate": + root = _root_name(sub.value) + if root: + pinned.add(root) + if isinstance(node, ast.Assign): + for sub in ast.walk(node.value): + if isinstance(sub, ast.Attribute) and sub.attr == "sqlstate": + root = _root_name(sub.value) + for t in node.targets: + if isinstance(t, ast.Name) and root: + assigned_from_sqlstate[t.id] = root + if (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) + and node.func.attr == "sqlstate"): + for sub in ast.walk(node): + if isinstance(sub, ast.Name): + pinned.add(sub.id) + # One hop: assigned from a sqlstate read, and later handed to a call. + for local, root in assigned_from_sqlstate.items(): + if local in call_arg_names: + pinned.add(root) + return pinned + + +def _raises_sites(path): + """Every `with pytest.raises(...)` in one file, and what is wrong with it. + + PARSED, NOT GREPPED, and `test_layer.py` is why. The layer's own tests drive an + inner pytest run, so the forbidden shape appears inside a `pytester.makepyfile` + STRING in the very file that proves the guard. A line regex fires on it. That + is the false positive the broad-except scan already paid for once, and a guard + that rejects legitimate tests gets switched off -- after which the thing it + replaced is gone too. A call inside a string literal is not an `ast.Call`. + + WHAT THIS DOES NOT SEE, stated because a guard's blind spots are part of its + meaning. It reads `with` blocks inside functions, so a `raises` at module level + or used as a plain call (`pytest.raises(E, fn, arg)`) is invisible. It counts + TOP-LEVEL statements in the block, so a single `for` or `if` holding several + statements counts as one, and a call to a helper that performs the setup counts + as one as well -- those two shapes are why `raises-catches-setup` stays open in + `VACUITY_MODES.md` section 3.4, and each has an arm in + `test_raises_sqlstate.py` asserting this scan reports nothing on it. It does + not follow a SQLSTATE pin into a helper, and it matches a pin by name, so an + unrelated argument that happens to share the bound name's spelling would + satisfy it. It is a floor, not a proof that the assertion is about the + statement under test. + """ + # A BROAD pytest.raises IS SATISFIED BY AN UNRELATED FAILURE OF THE SAME FAMILY. + # + # Measured against psycopg 3.3.5 in the audit container, by counting the classes + # in `psycopg.errors` that carry a `sqlstate` and subclass each family: + # + # psycopg.Error 254 SQLSTATEs 42 SQLSTATE classes + # psycopg.DatabaseError 254 42 + # psycopg.OperationalError 88 15 + # psycopg.DataError 68 1 + # psycopg.ProgrammingError 57 10 + # psycopg.InternalError 20 5 + # psycopg.IntegrityError 7 1 + # psycopg.NotSupportedError 1 1 + # psycopg.Warning 0 0 + # psycopg.InterfaceError 0 0 + # + # `pytest.raises(psycopg.Error)` therefore claims "one of 254 server errors + # arrived", and does not even claim that: measured on this tree, a connect to a + # socket that does not exist raises OperationalError with sqlstate None, so + # + # with pytest.raises(psycopg.Error): + # conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + # conn.execute("SELECT pgc_definitely_no_such_function()") + # + # reported `1 passed`, exit 0, with the statement under test never executed. + # + # WHY THIS LIST AND NOT THE WHOLE FAMILY, and it is the measurement above + # deciding it rather than taste. `Warning` and `InterfaceError` cover ZERO + # SQLSTATEs, so demanding one of them would be a guard nobody could satisfy -- + # and an unsatisfiable guard is how a guard gets switched off. The four + # intermediate DB-API classes are not refused either: narrowing to one of them + # is already a real claim about the error, and OperationalError legitimately + # arrives with no SQLSTATE when the connection itself failed. + # + # WHY IT IS BOUND HERE AND NOT AT MODULE LEVEL. A rule's own parameters must not + # be reachable from the tree the rule polices. Any `conftest.py` under + # `test/pytest/` is imported before collection, so a module-level tuple can be + # rewritten from the corpus: + # + # import pgc_vacuity + # pgc_vacuity. = () + # + # after which this scan reports zero offences for ever and the suite is green. + # Bound inside the function, those lines do nothing -- the name is not looked up + # in the module namespace at all. (Rebinding this FUNCTION from a conftest is + # still possible. That is true of every name in every Python plugin and is not + # something the placement of a tuple can fix. What notices a scan that stopped + # being CALLED is selftest 440, which requires the wiring line in the collection + # hook, plus the 5 arms in `test_raises_sqlstate.py` that match the refusal on + # stderr -- measured, as mutation M5 of that suite's removal proof: deleting the + # wiring reddens those same 5. `test_guards_pinned.py` is the layer's census of + # "every refusal pinned to its own message" and does NOT yet carry these two; + # adding them there is the honest next step, and saying so is better than citing + # a file that does not mention them.) + broad_families = ("Error", "DatabaseError", "Exception", "BaseException") + + try: + tree = ast.parse(pathlib.Path(path).read_text()) + except (OSError, SyntaxError): + return [] + out = [] + name = pathlib.Path(path).name + for fn in [n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]: + pinned = _sqlstate_pinned_names(fn) + for node in _walk_own(fn): + if not isinstance(node, (ast.With, ast.AsyncWith)): + continue + sites = [] + for item in node.items: + call = item.context_expr + if not isinstance(call, ast.Call): + continue + f = call.func + tail = (f.attr if isinstance(f, ast.Attribute) + else f.id if isinstance(f, ast.Name) else None) + if tail != "raises": + continue + # THE CLASS MAY ARRIVE BY KEYWORD. `not call.args` skipped the item + # BEFORE it was appended, so `pytest.raises(expected_exception=E)` + # was checked by neither rule -- and the statement rule was therefore + # silently conditional on the class being positional, which the + # documentation stated unconditionally. Reported by @jdatcmd, who + # built the positional and keyword forms as a pair that differ in + # nothing else: the positional one was an offence and the keyword one + # was clean. + expected = None + if call.args: + expected = call.args[0] + else: + for kw in call.keywords: + if kw.arg == "expected_exception": + expected = kw.value + break + if expected is None: + continue + sites.append(item) + bound = (item.optional_vars.id + if isinstance(item.optional_vars, ast.Name) else None) + 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. + where = f"{name}:{call.lineno}" + out.append( + f"{where} pytest.raises({broad[0]}) names no SQLSTATE" + ) + # THE RAISER HAS TO BE THE STATEMENT UNDER TEST. Once per `with`, not + # once per item: two raises in one `with` share one body. + if sites and len(node.body) != 1: + # One source line for this phrase too, for the reason above. + held = f"{name}:{node.lineno} the pytest.raises block holds" + out.append( + f"{held} {len(node.body)} statements, " + f"so which one raised is not pinned" + ) + return out + + def pytest_collection_modifyitems(config, items): """Refuse a bare skip, which exits 0 and reads as success. @@ -864,6 +1182,7 @@ def pytest_collection_modifyitems(config, items): for site in _broad_except_sites(f): offenders.append(f"{site} catches Exception broadly") offenders.extend(_sorted_ordered_sites(f)) + offenders.extend(_raises_sites(f)) # An empty parametrize is not a bare skip and deserves its own message: pytest # generates ONE skipped placeholder for an empty argvalues list, so a corpus glob @@ -892,6 +1211,9 @@ def pytest_collection_modifyitems(config, items): skips = [o for o in offenders if "@pytest.mark." in o] 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] parts = [] if skips: parts.append( @@ -913,6 +1235,24 @@ def pytest_collection_modifyitems(config, items): + "; ".join(ordered) + " -- pass the rows in the order the query returned them" ) + if raises_broad: + parts.append( + "pytest.raises over a whole error family is satisfied by an " + "unrelated failure of the same family, and psycopg.Error covers " + "254 SQLSTATEs while a failed connect carries none at all: " + + "; ".join(raises_broad) + + " -- pin the error with expect.sqlstate(exc.value, '42883', name)" + ", or name the specific exception class" + ) + 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: " + + "; ".join(raises_setup) + + " -- move the setup above the block, leaving the statement under " + "test alone inside it" + ) raise pytest.UsageError( "the pgColumnar vacuity layer refuses this run: " + ". ".join(parts) + "." ) diff --git a/test/pytest/test_raises_sqlstate.py b/test/pytest/test_raises_sqlstate.py new file mode 100644 index 00000000..5b5a76bf --- /dev/null +++ b/test/pytest/test_raises_sqlstate.py @@ -0,0 +1,756 @@ +"""A raised database error must name WHICH error, and the block must hold one statement. + +`VACUITY_MODES.md` section 3.4 listed `raises-too-broad` and `raises-catches-setup` +together, and the measurement that opened this file shows why they belong together. +Run on unmodified main at de8fca4, with the layer loaded and nothing skipped: + + with pytest.raises(psycopg.Error): + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + conn.execute("SELECT pgc_definitely_no_such_function()") + expect.num(1, 1, "the server rejected the call") + +reported `2 passed`, exit 0. The error that satisfied the claim was +`OperationalError` with `sqlstate` None -- it never reached a server, and the +statement the test is about never executed. Against a live PostgreSQL 18.4 the same +shape raised `InvalidName` 42602 from the SETUP line while the statement under test +raises `UndefinedObject` 42704: two different SQLSTATEs satisfy one `raises`. + +So there are two properties, and each needs its own arm: + + 1. a broad family must be narrowed to a SQLSTATE -- `psycopg.Error` covers 254 of + them across 42 SQLSTATE classes, measured against psycopg 3.3.5; + 2. the block must hold exactly one statement, or nothing says which one raised. + +THE TWO PROPERTIES CLOSE DIFFERENT AMOUNTS, and that asymmetry is the reason the +last three arms exist. Property 1 CLOSES `raises-too-broad`: a broad family with +nothing pinned cannot be collected. Property 2 only MITIGATES +`raises-catches-setup`, because it counts TOP-LEVEL statements: a call to a helper +that performs the setup is one statement, and so is a `for` holding the setup and +the statement under test. `test_a_helper_hiding_the_setup_is_not_refused` and +`test_a_compound_statement_hiding_the_setup_is_not_refused` assert this scan +reports NOTHING on those two shapes, which is why `raises-catches-setup` stays in +`VACUITY_MODES.md` section 3.4 rather than moving to section 2. + +THE SCAN IS AST, NOT A LINE REGEX, and this file is the reason. Every arm below +writes the forbidden shape inside a `pytester.makepyfile` string, because that is +how the layer's own tests drive an inner run. A line regex fires on this file and +refuses it -- the false positive the broad-except scan already paid for once. +`test_the_raises_scan_reads_code_not_a_string_literal` pins that. +""" + +import pathlib + + +CONF = "pytest_plugins = ['pgc_vacuity']" + + +def _inner(pytester, body): + pytester.makeconftest(CONF) + pytester.makepyfile(body) + return pytester.runpytest("-p", "pgc_vacuity") + + +# --------------------------------------------------------------------------- +# The static scan: which error, and which statement. +# --------------------------------------------------------------------------- + +def test_raises_requires_a_sqlstate(pytester, expect): + """The red test VACUITY_MODES.md section 5 item 4 names, and the real offender. + + Byte for byte the shape measured as `2 passed` on main. It is refused before + collection finishes, so it cannot run at all rather than running and passing. + """ + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(psycopg.Error): + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + conn.execute("SELECT pgc_definitely_no_such_function()") + expect.num(1, 1, "the server rejected the call") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a broad raises with no SQLSTATE must not be collectable") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + + +def test_a_raises_that_pins_the_sqlstate_is_accepted(pytester, expect): + """The positive control, and the one that matters most. + + A guard that rejects the honest form gets switched off. The honest form is the + same `pytest.raises(psycopg.Error)` with the error named afterwards, and it + must collect and pass. + """ + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_an_unknown_function_is_42883(expect): + with pytest.raises(psycopg.Error) as exc: + raise psycopg.errors.UndefinedFunction("no such function") + expect.sqlstate(exc.value, "42883", "an unknown function is 42883") + ''') + expect.outcomes(result, "the honest form collects and passes", passed=1, failed=0) + + +def test_a_raises_pinned_by_reading_the_field_is_accepted(pytester, expect): + """The second honest spelling: the field read directly rather than through the + helper. Both are claims about a typed field, so the scan accepts both.""" + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_the_field_itself(expect): + with pytest.raises(psycopg.Error) as exc: + raise psycopg.errors.UndefinedFunction("no such function") + expect.text(exc.value.sqlstate, "42883", "the SQLSTATE field itself") + ''') + expect.outcomes(result, "reading the field counts as pinning it", + passed=1, failed=0) + + +def test_a_narrow_raises_needs_no_sqlstate(pytester, expect): + """Control for the scope of the rule. A single-SQLSTATE class already names + the error, so demanding a second spelling of the same claim would be noise.""" + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_narrow(expect): + with pytest.raises(psycopg.errors.UndefinedFunction): + raise psycopg.errors.UndefinedFunction("no such function") + expect.num(1, 1, "the narrow class is the claim") + ''') + expect.outcomes(result, "a one-SQLSTATE class is not refused", passed=1, failed=0) + + +def test_a_raises_tuple_hides_a_broad_member(pytester, expect): + """@jdatcmd's #905 hole, in the guard written after it. + + The broad-except scan looked only at a bare name, so `except Exception` was + refused and `except (ValueError, Exception)` passed. A tuple is how a narrow + claim gets widened under pressure, which is the exact moment the guard is for. + """ + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_tuple(expect): + with pytest.raises((ValueError, psycopg.Error)): + raise psycopg.errors.UndefinedFunction("no such function") + expect.num(1, 1, "something of some family failed") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a broad member of a tuple is still broad") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + + +def test_raises_exception_is_refused_like_a_broad_except(pytester, expect): + """`except Exception` is already uncollectable; `pytest.raises(Exception)` was + not, and it swallows the same failures for the same reason.""" + pytester.makepyfile( + """ + import pytest + + def test_anything_at_all(expect): + with pytest.raises(Exception): + raise RuntimeError("the real failure") + expect.num(1, 1, "something went wrong") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "pytest.raises(Exception) must not be collectable") + result.stderr.fnmatch_lines(["*pytest.raises(Exception) names no SQLSTATE*"]) + + +def test_setup_inside_a_raises_block_is_refused(pytester, expect): + """`raises-catches-setup`, in the spelling this rule DOES close. The block is + narrow AND pinned, and still vacuous: two statements inside it, so the failure + may be either one. + + THE INNER FILE IS SELF-CONTAINED ON PURPOSE. Written against a bare `conn` it + failed with `NameError`, which satisfies `run_failed` without anything having + refused it -- the arm would have been green on a tree with no guard at all. + With the stub below and no guard, the inner run reports `1 passed`: the SET + raises 42704, the pin accepts it, and the ALTER the test is named for never + runs. That is the defect, and it is what this arm has to redden. + """ + pytester.makepyfile( + """ + import psycopg + import pytest + + class _Conn: + def execute(self, sql): + raise psycopg.errors.UndefinedObject("no such object: " + sql) + + conn = _Conn() + + def test_which_one_raised(expect): + with pytest.raises(psycopg.errors.UndefinedObject) as exc: + conn.execute("SET pgcolumnar.no_such_guc = 1") + conn.execute("ALTER TABLE t SET ACCESS METHOD no_such_am") + expect.sqlstate(exc.value, "42704", "the ALTER was refused") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "two statements in the block must not be collectable") + result.stderr.fnmatch_lines(["*holds 2 statements, so which one raised*"]) + + +def test_a_raises_block_with_one_statement_is_accepted(pytester, expect): + """Control for the arm above, differing in exactly one property: the setup + moved above the block, leaving the statement under test alone inside it.""" + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_one_statement(expect): + with pytest.raises(psycopg.errors.UndefinedFunction) as exc: + raise psycopg.errors.UndefinedFunction("no such function") + expect.sqlstate(exc.value, "42883", "the statement under test failed") + ''') + expect.outcomes(result, "one statement in the block is the honest form", + passed=1, failed=0) + + +def test_the_raises_scan_reads_code_not_a_string_literal(pytester, expect): + """The false positive this scan is written to avoid, pinned so it cannot return. + + The inner file contains the refused shape inside a `makepyfile` string, exactly + as every arm in this file does. A line regex refuses it; `ast` does not see a + handler or a call inside a string. + """ + result = _inner(pytester, ''' + def test_quotes_the_shape(expect): + forbidden = """ + import psycopg, pytest + def test_inner(expect): + with pytest.raises(psycopg.Error): + conn = psycopg.connect("host=/nope") + conn.execute("SELECT 1") + """ + expect.at_least(len(forbidden), 1, + "the shape above is a string, not code") + ''') + expect.outcomes(result, "the forbidden shape inside a string is not refused", + passed=1, failed=0) + + +# --------------------------------------------------------------------------- +# expect.sqlstate, the honest form the scan points at. +# --------------------------------------------------------------------------- + +def test_sqlstate_refuses_a_sqlstate_class_prefix(pytester, expect): + """"42" is a SQLSTATE CLASS. Accepting it would make the pin a prefix claim + wearing the spelling of an exact one -- the substring defect, one layer up.""" + expect.refusal(_inner(pytester, ''' + import psycopg + + def test_prefix(expect): + expect.sqlstate(psycopg.errors.UndefinedFunction("x"), "42", "a class") + '''), "sqlstate refuses a two-character class", "is not a SQLSTATE") + + +def test_sqlstate_refuses_an_empty_expectation(pytester, expect): + """An empty want names no error, so nothing could have failed it.""" + expect.refusal(_inner(pytester, ''' + import psycopg + + def test_empty_want(expect): + expect.sqlstate(psycopg.errors.UndefinedFunction("x"), "", "nothing") + '''), "sqlstate refuses an empty expectation", "is not a SQLSTATE") + + +def test_sqlstate_refuses_an_object_carrying_no_sqlstate(pytester, expect): + """Passing pytest's ExceptionInfo instead of `exc.value` is the mistake that + produces it, and it would otherwise compare None against a real code forever.""" + expect.refusal(_inner(pytester, ''' + def test_wrong_object(expect): + expect.sqlstate(RuntimeError("not a database error"), "42883", "wrong") + '''), "sqlstate refuses an object with no sqlstate", "carries no sqlstate") + + +def test_sqlstate_fails_when_the_failure_never_reached_the_server(pytester, expect): + """The measured case. A connect to a socket that does not exist raises + OperationalError with sqlstate None: a psycopg.Error that is no server error.""" + result = _inner(pytester, ''' + import psycopg + + def test_no_sqlstate_at_all(expect): + exc = psycopg.OperationalError("connection failed") + expect.sqlstate(exc, "42883", "an unknown function") + ''') + expect.outcomes(result, "a failure with no SQLSTATE is not the named error", + failed=1, passed=0) + result.stdout.fnmatch_lines(["E*carrying no SQLSTATE*"]) + + +def test_sqlstate_fails_on_a_different_sqlstate(pytester, expect): + """The whole point: the error measured from the setup line was 42602 and the + statement under test raises 42704. One `raises` accepts both; this does not.""" + result = _inner(pytester, ''' + import psycopg + + def test_the_other_error(expect): + expect.sqlstate(psycopg.errors.InvalidName("bad name"), "42704", + "the ALTER was refused") + ''') + expect.outcomes(result, "a different SQLSTATE fails", failed=1, passed=0) + result.stdout.fnmatch_lines(["E*got SQLSTATE '42602' want '42704'*"]) + + +def test_sqlstate_accepts_the_exact_sqlstate(pytester, expect): + """Positive control for all five arms above.""" + result = _inner(pytester, ''' + import psycopg + + def test_exact(expect): + expect.sqlstate(psycopg.errors.UndefinedFunction("x"), "42883", + "an unknown function is 42883") + ''') + expect.outcomes(result, "the exact SQLSTATE passes", passed=1, failed=0) + + +def test_sqlstate_accepts_one_of_several_named_codes(pytester, expect): + """This tree supports majors 15 through 19, so an error code can legitimately + differ between them. A tuple widens the claim by exactly the codes it names.""" + result = _inner(pytester, ''' + import psycopg + + def test_either_code(expect): + expect.sqlstate(psycopg.errors.UndefinedObject("x"), ("42883", "42704"), + "one of the two codes this major uses") + ''') + expect.outcomes(result, "a named pair of codes passes", passed=1, failed=0) + + +def test_sqlstate_refuses_an_empty_set_of_codes(pytester, expect): + """The escape hatch must not become the hole. An empty tuple is satisfied by + nothing, so it could not have passed and says nothing about which error came.""" + expect.refusal(_inner(pytester, ''' + import psycopg + + def test_no_codes(expect): + expect.sqlstate(psycopg.errors.UndefinedFunction("x"), (), "nothing") + '''), "sqlstate refuses an empty set of codes", + "an empty set of SQLSTATEs is satisfied by nothing") + + +# --------------------------------------------------------------------------- +# What this scan must NOT do. Both of these were proven failures on a sibling +# branch in this layer, so they are arms here rather than a sentence. +# --------------------------------------------------------------------------- + +def test_the_raises_scan_leaves_the_unrunnable_state_alone(pytester, expect): + """A documented hatch the corpus does not exercise, exercised. + + `VACUITY_MODES.md` section 3.3 promises `cannot_run` is not a skip: the run + prints `UNRUN`, counts it, and exits `EXIT_INCOMPLETE`. A sibling guard in this + layer measured zero false positives over all 148 tests and still broke that, + because nothing in the corpus declares itself unrunnable. The file here also + holds a legitimate pinned `raises`, so the scan has run over it. + """ + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_pins_its_error(expect): + with pytest.raises(psycopg.Error) as exc: + raise psycopg.errors.UndefinedFunction("no such function") + expect.sqlstate(exc.value, "42883", "an unknown function is 42883") + + def test_cannot_run_here(expect): + expect.cannot_run("MISSING_DEPENDENCY", "duckdb is not installed here") + ''') + expect.num(result.ret, 67, "the run still exits EXIT_INCOMPLETE") + result.stdout.fnmatch_lines(["*checks unrunnable: 1*"]) + + +def test_the_raises_scan_does_not_touch_a_recorder_made_in_the_body(pytester, expect): + """A recorder fetched during the CALL phase must still satisfy the layer. + + A sibling guard read the recorder before pytest's call hook yielded, so a test + that asks for `expect` itself was reddened with a neighbouring guard's message. + This scan runs at collection time and cannot do that; the arm is here so a later + edit that moves it cannot do it either. + """ + result = _inner(pytester, ''' + import psycopg + import pytest + + def test_asserts_in_its_own_body(request): + expect = request.getfixturevalue("expect") + expect.num(2 + 2, 4, "arithmetic, concluded by the test body itself") + + def test_pins_its_error(expect): + with pytest.raises(psycopg.Error) as exc: + raise psycopg.errors.UndefinedFunction("no such function") + expect.sqlstate(exc.value, "42883", "an unknown function is 42883") + ''') + expect.outcomes(result, "a recorder made in the body still satisfies the layer", + passed=2, failed=0) + + +# --------------------------------------------------------------------------- +# THE RESIDUAL, PINNED. `raises-catches-setup` is MITIGATED by the one-statement +# rule, NOT closed, and the two shapes that walk past it get an arm each. They +# assert the scan reports NOTHING, which is what makes the residual a measurement +# rather than a sentence -- and what stops the next reader of the scan from +# assuming the sibling mode is handled. +# +# The one-statement rule counts TOP-LEVEL statements in the block. Both shapes +# below are one top-level statement that performs the setup inside the block. +# --------------------------------------------------------------------------- + +def test_a_helper_hiding_the_setup_is_not_refused(pytester, expect): + """Residual shape 1 of 2: a CALL TO A HELPER that performs the setup. + + One statement, a narrow class, a pinned SQLSTATE, and the setup still raised. + The block holds one top-level statement, so the statement rule is satisfied; + the class names a single SQLSTATE and the test pins it, so the SQLSTATE rule is + satisfied; and the error came from inside the helper rather than from the + statement the test is about. Against this guard: `1 passed`, exit 0, and the + scan reported no offence. + + What would close it is a claim about WHICH statement raised -- a position, or a + helper that runs exactly one statement and owns the assertion -- and this scan + makes no such claim. So `raises-catches-setup` stays in `VACUITY_MODES.md` + section 3.4. + """ + result = _inner(pytester, ''' + import psycopg + import pytest + + def _do_the_thing(flag): + if flag: + raise psycopg.errors.UndefinedObject("SET pgcolumnar.no_such_guc") + raise psycopg.errors.UndefinedFunction("the statement under test") + + def test_the_alter_is_refused(expect): + with pytest.raises(psycopg.errors.UndefinedObject) as exc: + _do_the_thing(True) + expect.sqlstate(exc.value, "42704", "the statement under test") + ''') + expect.outcomes(result, "the setup hidden in a helper is NOT refused -- the " + "first residual this guard leaves open", + passed=1, failed=0) + # `passed=1` IS THE ZERO-OFFENCE ASSERTION, and it is the whole of it. An + # offence makes the layer raise pytest.UsageError, which collects nothing and + # exits 4, so `passed=1` is reachable only when the scan reported nothing at + # all. A `result.stderr.no_fnmatch_line("*names no SQLSTATE*")` after this line + # was tried and removed: `expect.outcomes` fails first in the only case where an + # offence could appear, so the extra line can never execute and reads as a check + # while being none. Measured -- under a mutation that flags every block, both + # residual arms redden on `outcomes` and the stderr lines are never reached. + + +def test_a_compound_statement_hiding_the_setup_is_not_refused(pytester, expect): + """Residual shape 2 of 2: a COMPOUND STATEMENT holding the setup and the + statement under test. + + A `for` over two statements is ONE top-level statement, so `len(node.body) == 1` + and the rule is satisfied. The loop raises on its FIRST iteration -- the setup -- + and the statement the test names never runs. Measured here as `1 passed`, exit 0, + with the scan reporting no offence, which is the same failure + `test_setup_inside_a_raises_block_is_refused` catches when the two statements sit + side by side instead. + + An `if`, a `with`, or a `try` nests the same way. Counting statements RECURSIVELY + would catch this 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. + """ + result = _inner(pytester, ''' + import psycopg + import pytest + + def _run(stmt): + if stmt.startswith("SET"): + raise psycopg.errors.UndefinedObject("no such GUC") + raise psycopg.errors.UndefinedFunction("the statement under test") + + def test_the_alter_is_refused(expect): + with pytest.raises(psycopg.errors.UndefinedObject) as exc: + for stmt in ("SET pgcolumnar.no_such_guc = 1", + "ALTER TABLE t SET ACCESS METHOD no_such_am"): + _run(stmt) + expect.sqlstate(exc.value, "42704", "the statement under test") + ''') + expect.outcomes(result, "a compound statement hiding the setup is NOT refused " + "-- the second residual this guard leaves open", + passed=1, failed=0) + # Zero offences, by the argument given in the arm above. + + +# --------------------------------------------------------------------------- +# The rule's own parameters must not be reachable from the tree it polices. +# --------------------------------------------------------------------------- + +def test_a_conftest_cannot_switch_the_broad_family_list_off(pytester, expect): + """A conftest that rewrites the scan's family list must change nothing. + + Every `conftest.py` under `test/pytest/` is imported before collection, so a + module-level tuple is writable from the corpus the rule polices: four lines in a + conftest empty the list, the scan then reports zero offences, and the suite is + green with the guard switched off and nothing saying so. + + The list is therefore bound inside `_raises_sites` rather than at module level. + The conftest below writes BOTH plausible spellings of the name onto the module + and then presents the refused shape. The run must still be refused. + """ + pytester.makeconftest(''' + import pgc_vacuity + + pgc_vacuity._BROAD_RAISES = () + pgc_vacuity.broad_families = () + pgc_vacuity.BROAD_RAISES = () + ''') + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(psycopg.Error): + raise psycopg.errors.UndefinedFunction("no such function") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a conftest writing the family list does not disable " + "the rule") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + + +# ---- MENTIONING THE FIELD IS NOT PINNING IT ---------------------------------- +# +# `_sqlstate_pinned_names` counted any `ast.Attribute` named `sqlstate` anywhere in +# the body, so naming the field switched the broad-family rule off. Both shapes below +# collected CLEAN against that version, and both are byte-for-byte the vacuity this +# file is named for -- any of the 254 SQLSTATEs satisfies them. Reported by @jdatcmd, +# who drove the scan over six constructed files instead of reading it. +# +# The rule now requires the read to reach a CALL, and follows one hop of assignment +# so the honest `code = exc.value.sqlstate` / `expect.text(code, ...)` form is not +# refused. One hop, not two: this is a floor, and the floor is stated. + + +def test_a_bare_sqlstate_expression_does_not_pin_anything(pytester, expect): + """`exc.value.sqlstate` as a statement of its own asserts nothing at all.""" + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(psycopg.Error) as exc: + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + exc.value.sqlstate + expect.num(1, 1, "the server rejected the call") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "mentioning the field is not pinning it") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + + +def test_a_sqlstate_assigned_and_never_read_does_not_pin_anything(pytester, expect): + """The same hole with one more step: bound to a name nothing uses.""" + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(psycopg.Error) as exc: + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + code = exc.value.sqlstate + expect.num(1, 1, "the server rejected the call") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "an unread assignment is not a pin") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + + +def test_one_hop_through_a_local_name_is_an_honest_pin(pytester, expect): + """The cost side. Refusing this would outlaw a form nobody should stop writing, + and a guard that refuses honest tests is a guard somebody switches off.""" + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(psycopg.Error) as exc: + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + code = exc.value.sqlstate + expect.text(code, "42883", "the function does not exist") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a pin reached through one local name is collectable", + passed=0, failed=1, errors=0) + result.stdout.no_fnmatch_line("*names no SQLSTATE*") + + +# ---- THE CLASS MAY ARRIVE BY KEYWORD ---------------------------------------- +# +# `if tail != "raises" or not call.args: continue` skipped the item BEFORE it was +# appended, so `pytest.raises(expected_exception=...)` was checked by neither rule -- +# and the statement rule was therefore silently conditional on the class being +# positional, which this file's own prose stated unconditionally. Reported by +# @jdatcmd, who built the two forms as a pair differing in nothing else: the +# positional one reported two offences and the keyword one reported none. + + +def test_the_keyword_form_is_checked_by_both_rules(pytester, expect): + """Broad, unbound, and two statements, with the class passed by keyword.""" + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(expected_exception=psycopg.Error): + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + conn.execute("SELECT pgc_definitely_no_such_function()") + expect.num(1, 1, "the server rejected the call") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "the keyword form is not an exemption") + result.stderr.fnmatch_lines(["*pytest.raises(Error) names no SQLSTATE*"]) + result.stderr.fnmatch_lines(["*holds 2 statements*"]) + + +def test_the_keyword_form_with_a_pin_is_collectable(pytester, expect): + """And it is not refused merely for being the keyword form.""" + pytester.makepyfile( + """ + import psycopg + import pytest + + def test_an_unknown_function_is_rejected(expect): + with pytest.raises(expected_exception=psycopg.Error) as exc: + conn = psycopg.connect("host=/nonexistent-socket-dir dbname=pgc") + expect.sqlstate(exc.value, "42883", "the function does not exist") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a pinned keyword form is collectable", + passed=0, failed=1, errors=0) + result.stdout.no_fnmatch_line("*names no SQLSTATE*") + + +# ---- THE NEUTERING PROOF, WHICH A TEXT PIN CANNOT CARRY --------------------- +# +# `test/selftest/440` checks this scan by grepping its source, and a text pin catches +# a rewrite or a deletion but not `False and` -- which is how a guard actually dies. +# @jdatcmd measured three faithful neuterings (prefix `False and`, rename nothing, +# leave every pinned substring in place) and all three left that part at 55 passed +# while the scan went blind. +# +# THE PROOF BELONGS HERE, not there: the shell harness and this corpus are parallel +# in functionality and do not drive each other. So this arm copies the layer, disables +# one condition faithfully, imports the copy, and requires the copy to go blind to a +# file the real layer refuses. It fails if the scan stops being load-bearing, whatever +# the neutering looks like. + + +def _layer_copy(tmp_path, old, new, name): + """A copy of pgc_vacuity with ONE condition disabled, imported under its own name.""" + import importlib.util + src = pathlib.Path(__file__).with_name("pgc_vacuity.py").read_text() + assert old in src, "the condition to disable is not in the layer verbatim: %r" % old + twin = tmp_path / (name + ".py") + twin.write_text(src.replace(old, new, 1)) + spec = importlib.util.spec_from_file_location(name, twin) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod, twin + + +def _offence_fixture(tmp_path): + f = tmp_path / "fixture_case.py" + f.write_text( + "import psycopg\n" + "import pytest\n\n" + "def test_it(conn, expect):\n" + " with pytest.raises(psycopg.Error):\n" + " conn.execute('SELECT pgc_no_such()')\n" + " conn.execute('SELECT 1')\n" + " expect.num(1, 1, 'rejected')\n" + ) + return f + + +def test_disabling_the_sqlstate_rule_makes_the_scan_blind(tmp_path, expect): + """Faithful: `False and` prefixed, nothing renamed, the pinned text left intact.""" + import pgc_vacuity + case = _offence_fixture(tmp_path) + real = [s for s in pgc_vacuity._raises_sites(case) if "names no SQLSTATE" in s] + expect.num(len(real), 1, "the real layer reports the unpinned broad raises") + twin, path = _layer_copy( + tmp_path, + "if broad and (bound is None or bound not in pinned):", + "if False and broad and (bound is None or bound not in pinned):", + "twin_sqlstate", + ) + expect.at_least(path.read_text().count("bound not in pinned"), 1, + "and the twin still CONTAINS the text a grep arm pins") + blind = [s for s in twin._raises_sites(case) if "names no SQLSTATE" in s] + expect.num(len(blind), 0, "while the twin reports none, which no text pin can see") + + +def test_disabling_the_statement_rule_makes_the_scan_blind(tmp_path, expect): + """The second condition, same shape, so neither rule rests on the other's arm.""" + import pgc_vacuity + case = _offence_fixture(tmp_path) + real = [s for s in pgc_vacuity._raises_sites(case) if "statements" in s] + expect.num(len(real), 1, "the real layer reports the two-statement block") + twin, path = _layer_copy( + tmp_path, + "if sites and len(node.body) != 1:", + "if False and sites and len(node.body) != 1:", + "twin_statements", + ) + expect.at_least(path.read_text().count("len(node.body) != 1"), 1, + "and the twin still contains the pinned text") + blind = [s for s in twin._raises_sites(case) if "statements" in s] + expect.num(len(blind), 0, "while the twin reports none") + +def test_the_mode_this_layer_only_narrows_is_still_listed_as_open(expect): + """`raises-catches-setup` must stay in VACUITY_MODES.md section 3. + + Property 1 CLOSES `raises-too-broad`. Property 2 only narrows + `raises-catches-setup`, because the statement rule counts TOP-LEVEL statements and + two shapes walk past it -- both asserted above. A document that quietly moved the + mode to section 2 would claim a closure this scan does not make, which is the + failure a map makes worse than a silence. + + Reading a document is the one thing this corpus and the shell harness may both do: + they are parallel in functionality and do not drive each other, and the docs are + where they are allowed to meet. + """ + import re + doc = pathlib.Path(__file__).with_name("VACUITY_MODES.md").read_text() + # SECTION 3 RUNS TO SECTION 4, not to 3.1. Splitting on "## 3." truncated at the + # first subsection heading and reported the mode missing from its own section -- + # my own arm failing for a reason that had nothing to do with the document. + start = re.search(r"^## 3\. ", doc, re.M) + end = re.search(r"^## 4\. ", doc, re.M) + expect.text("found" if start and end else "missing", "found", + "premise: section 3 and section 4 both have headings to bound by") + section3 = doc[start.end():end.start()] + expect.at_least(doc.count("raises-catches-setup"), 1, + "the document still names the mode this layer only narrows") + expect.text("open" if "raises-catches-setup" in section3 else "moved", "open", + "and names it in the section for what is NOT refused") + expect.text("absent" if "raises-too-broad" not in section3.split("is now closed")[0] + else "present", "absent", + "premise: and the mode this layer DOES close is not loose in section 3")