diff --git a/test/pytest/README.md b/test/pytest/README.md index b5c13307..7f19f184 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -3,6 +3,11 @@ 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. + 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 behind each guard. diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index a3a4f811..576c3c88 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -4,7 +4,7 @@ Reference for anyone reading, running, or adding to `test/pytest/`. The design a the decisions behind the harness are in `design/ISSUE_432_PYTEST_HARNESS.md`. This file covers the tests themselves. -**90 tests in 6 files.** Seventy-five of them test the harness rather than the +**120 tests in 8 files.** One hundred and five of them test the harness rather than the product, and they come first, because a harness that can report a false green makes every other result in this directory worthless. @@ -32,8 +32,11 @@ behaviour, the source of that number is named. - [6. test_docs_cover_the_corpus.py: this document, checked](#6-test_docs_cover_the_corpuspy-this-document-checked) - [7. test_connection.py: the cluster and the direct connection](#7-test_connectionpy-the-cluster-and-the-direct-connection) - [8. test_native_projection.py: the ported suite](#8-test_native_projectionpy-the-ported-suite) -- [9. Adding a test](#9-adding-a-test) -- [10. Traps this corpus records](#10-traps-this-corpus-records) +- [9. test_ordered.py: the ordered oracle](#9-test_orderedpy-the-ordered-oracle) +- [10. test_runshape.py: the shape of the run itself](#10-test_runshapepy-the-shape-of-the-run-itself) +- [11. Adding a test](#11-adding-a-test) +- [12. What this corpus does NOT yet refuse](#12-what-this-corpus-does-not-yet-refuse) +- [13. Traps this corpus records](#13-traps-this-corpus-records) ## 1. How to read a test in here @@ -64,6 +67,9 @@ assertion, so the two read differently in output. | `num(got, want, name)` | two numbers are equal | anything that is not a number, including `bool`, and including the string `"100"` that `psql -At` would have given | | `at_least(got, floor, name)` | `got >= floor` | non-numbers, and a floor of zero or less, which any count satisfies | | `rows(got, want, name, allow_empty=None)` | two result sets are equal | both sides empty, unless `allow_empty` gives a reason | +| `row_set(got, want, name, allow_empty=None)` | two result sets are equal **ignoring order** | what `rows` refuses | +| `ordered_rows(got, want, name)` | two sequences are equal **in order** | two empty sequences, and a sequence whose elements are all identical, where order cannot be observed | +| `ordering_observable(forward, reverse, name)` | this fixture can distinguish order at all | a fixture that reads identically both ways | | `hash(got, want, name)` | two oracle hashes are equal | comparing an object against itself, either side being a `QUERY_ERROR` sentinel, both sides empty | | `text(got, want, name)` | two strings are equal | an empty expectation, which anything empty satisfies | | `plan_marker(plan, key, name, absent=False)` | some plan node carries a `Columnar` property key | nothing; `absent=True` inverts it | @@ -79,6 +85,14 @@ subsumed by a neighbouring one, so the inner run fails either way and an outcome-only assertion cannot tell which. Requiring the message is the same move as asserting on a SQLSTATE rather than on prose -- name the contract, not the symptom. +**Which oracle you pick is an assertion, not a formatting choice.** `row_set` +ignores order by declaration; `ordered_rows` asserts it. The collection scan refuses +`sorted()` or `set()` feeding `ordered_rows`, because that reads as an ordering claim +and is not one. This is `pgc_seq_hash`, `diff_query_ordered` and +`pgc_check_ordered_oracle` ported, including that third one's control: the set oracle +must be order-blind BY DESIGN, or an ordered oracle could quietly be implemented as a +set one and every ordering test would go silent while staying green. + `rows` compares row sets rather than `md5(string_agg(...))`. That asserts the same property as the bash oracle by a stronger means: a hash mismatch says two hashes differ, a row-set mismatch says which row. It also avoids recomputing the hash in @@ -90,7 +104,7 @@ Python, where encoding or collation could make identical rows hash differently. ## 3. test_layer.py: the guards, testing themselves -These fourteen run pytest inside pytest through the `pytester` fixture. Each +These eighteen run pytest inside pytest through the `pytester` fixture. Each writes a small test file, runs it with the plugin loaded, and asserts on the INNER run's outcome. That is what proves a guard REFUSES, rather than assuming it. @@ -113,13 +127,38 @@ one of those eight measurements exited 0. | `test_an_unrunnable_test_names_its_reason_and_its_detail` | the `UNRUN` line carries reason and detail | nothing was printed at all | | `test_a_real_failure_outranks_an_unrunnable_test` | a run with both exits 1, not 67 | — | | `test_a_run_with_nothing_unrunnable_still_exits_zero` | **control**: a green run is untouched | — | +| `test_layer_rejects_an_absence_assertion_over_an_empty_plan` | an absence claim over `[]` is refused | it passes: nothing is there to find | +| `test_layer_allows_an_absence_assertion_over_a_real_plan` | **control**: `absent=True` still works on a plan that arrived | — | +| `test_layer_rejects_psycopgs_no_count_sentinel` | `rowcount` of `-1` is refused | `-1` and `1` are both numbers, so `num` compares them happily | +| `test_layer_rejects_a_broad_except_in_a_test_file` | a broad `except` is uncollectable | it was forbidden in a COMMENT, which enforces nothing | -Five of the fourteen are controls rather than guards. They are not decoration. A guard +Six of the eighteen are controls rather than guards. They are not decoration. A guard with a bad false-positive rate gets switched off, and then the guard it replaced is gone too. `test_a_counted_assertion_passes` and `test_layer_matches_the_exact_provider` exist so that a guard which starts rejecting good tests reddens here first. +The last four came from checking the layer against the 79-mode inventory in +`VACUITY_MODES.md` rather than from reasoning about it, and **all three guards they +added had been passing silently**. Two are worth stating in full because the shape +recurs. + +**`plan_marker(absent=True)` returned a pass against `[]`.** An absence assertion is +satisfied by nothing being there at all, which is the case most worth catching: a +plan that failed to arrive looks exactly like a plan that legitimately lacks the +node. Absence claims need a premise that the thing which could carry the marker +exists — the same reason `at_least` refuses a floor of zero. + +**A broad `except` was forbidden in a comment, which enforces nothing.** After any +failed statement psycopg raises `InFailedSqlTransaction` for every later one, so a +single `except Exception` hides the real error and all its successors. Written first +as a line regex, the guard immediately rejected this layer's own tests, because the +forbidden shape appears inside a `pytester.makepyfile` string. It 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** — the same mistake as +matching a plan by substring, and a guard that rejects legitimate tests is a guard +somebody switches off. + The escape hatches are deliberately more expensive to type than the honest form. `allow_empty` takes a reason, not `True`. `--pgc-expect-tests` takes the real number. `cannot_run` takes a reason from a closed list. None of them can become the @@ -251,6 +290,15 @@ restored 38c951eb7dda byte-exact **Each mutation reddens exactly one test, and it is that test's own.** That is the property worth having: it proves the three are distinguishable rather than subsumed, which "something went red" cannot. +| `test_ordered_rows_both_empty_names_its_own_refusal` | the sequence oracle's both-empty refusal, pinned to ITS message | +| `test_ordering_observable_both_empty_names_its_own_refusal` | the premise check's both-empty refusal, pinned to ITS message | +| `test_refusal_itself_refuses_an_empty_pattern_list` | the new helper must not become the defect it removes | + +The two ordered-oracle rows are the subsumption case in its purest form. Both +guards refuse a both-empty comparison, and so does `rows()` underneath them, so an +arm asserting only "the inner run failed" passes with any one of the three deleted. +Each is pinned to its own message, which is the only way the three stay +distinguishable. Three of these carry reasoning that is easy to lose. @@ -606,6 +654,10 @@ many times. | `test_the_stated_totals_are_the_totals_on_disk` | the bold totals line matches the corpus | | `test_a_fully_documented_corpus_reports_nothing_missing` | **control**: no false positive on a complete document | | `test_an_undocumented_test_is_named_rather_than_passed_over` | the exact shape that shipped: file named, one test inside it not | +| `test_the_mode_inventory_states_its_own_totals_correctly` | the totals in VACUITY_MODES.md section 1a are the modes on disk | +| `test_the_readme_and_the_inventory_agree_on_what_is_refused` | README.md quotes the inventory's number, so the two cannot drift apart again | +| `test_the_inventory_accounts_for_every_mode_the_run_found` | the admitted gap row is the run's total minus what is written down | +| `test_the_prose_totals_match_the_counted_modes` | every sentence stating what the layer refuses today carries the counted number, not just the table | | `test_an_undocumented_file_is_caught_with_the_tests_inside_it` | how 29 tests went missing at once | | `test_a_document_with_no_totals_line_states_none` | absent totals report `None`, which must not read as "they match" | | `test_a_stated_total_that_disagrees_with_disk_is_visible` | the count arm's own red | @@ -734,7 +786,82 @@ The mutation makes `PgColumnarProjectionFanoutRow` return without writing. Each builds and installs once, and both harnesses print the `.so` md5 they measured, so an arm where the two differ is void rather than reported. -## 9. Adding a test +## 9. test_ordered.py: the ordered oracle + +`lib.sh` has two oracles and this port had one. `pgc_set_hash` sorts before hashing, +so a bash test naming `ORDER BY` and comparing with `diff_query` cannot fail on +order; `pgc_seq_hash` and `diff_query_ordered` are the ones that can. These nine +tests port that pair and its premise check. + +| test | the guard | what it stops | +| --- | --- | --- | +| `test_ordered_rows_refuses_a_sequence_whose_order_is_unobservable` | an ordered claim over a constant sequence is refused | forward equals reverse, so order asserts nothing | +| `test_ordered_rows_refuses_two_empty_sequences` | two empty sides are refused here too | inherits `rows()`'s refusal instead of losing it | +| `test_ordered_rows_accepts_a_real_ordering` | **positive control** | a genuine ordered claim still passes | +| `test_ordered_rows_fails_on_the_wrong_order` | the oracle detects order | proves it can fail, not merely that it permits | +| `test_layer_refuses_sorting_the_input_to_an_ordered_claim` | `sorted()` feeding `ordered_rows` is uncollectable, found by AST | `ordered_rows(sorted(got), sorted(want))` cannot fail on order | +| `test_layer_refuses_a_name_bound_to_a_sorted_call` | `g = sorted(got)` one line above the claim is the same collapse | the inline spelling was the only one caught, so the guard was blind to the version least likely to be noticed | +| `test_layer_refuses_a_list_sorted_in_place` | `got.sort()` kills the order and leaves the name spelled the same | nothing at the call site says anything happened | +| `test_layer_allows_a_name_sorted_after_the_claim` | **control** | a name sorted AFTER the claim did not affect it; refusing that would be a false red | +| `test_the_order_killer_scan_is_one_function_deep` | **pinned limit** | a sort behind a helper is not caught, and this arm reddens if that documented limit ever moves | +| `test_ordering_observable_requires_the_two_directions_to_differ` | a fixture reading the same forwards and backwards is refused | the premise `pgc_check_ordered_oracle` asserts in bash | +| `test_ordering_observable_passes_when_the_directions_differ` | **positive control** | a real fixture is untouched | +| `test_the_two_oracles_are_different_instruments` | the set oracle and the sequence oracle must disagree on a permutation | if they agree, one of them is not the instrument it claims to be | +| `test_row_set_still_refuses_two_empty_sides` | **regression control** | adding the sequence oracle must not weaken the set one | + +The third property is the one worth reading twice. Two oracles that always agree are +one oracle with two names, and a suite built on them would pass every ordering claim +by construction. The test feeds both a permutation and requires the set oracle to +accept while the sequence oracle rejects. + +The AST scan matters for the same reason the broad-`except` scan does. A line regex +for `sorted(` fired inside the `pytester.makepyfile` string of the test that tests +it, so the guard rejected its own corpus. Walking the tree and looking at real call +nodes is the only version that distinguishes code from a string holding code. + +## 10. test_runshape.py: the shape of the run itself + +The other guards ask whether a test asserted anything. These six ask whether the +**run** did. Each of the three failure shapes turns a whole session green rather +than a single test, which is why they were built before the rest of the backlog. + +| test | the guard | bare pytest, measured | +| --- | --- | --- | +| `test_layer_fails_when_a_collected_test_never_reports` | the reported node-id set is reconciled against the collected one | 6 collected, 5 reported; the crash is named, the lost test is not | +| `test_layer_accepts_a_run_where_every_test_reports` | **positive control** | an honest parallel run is untouched | +| `test_layer_rejects_a_parametrize_over_an_empty_list` | an empty parameter set fails the run | `1 skipped`, exit 0 | +| `test_layer_accepts_a_parametrize_with_cases` | **positive control** | a real parameter set is untouched | +| `test_layer_rejects_a_fixture_that_skips` | a skip arriving during setup fails the run | every dependent test skips, exit 0 | +| `test_layer_allows_a_declared_unrunnable_test` | **escape hatch and control** | `expect.cannot_run` records a counted assertion instead of skipping | +| `test_layer_allows_a_deliberately_selected_subset` | `-k` is a deliberate act, not tests lost | the guard reported 15 deselected tests as never reported and failed a healthy run | +| `test_layer_allows_an_explicitly_deselected_test` | `--deselect` reaches the same hook by another route | pinned separately so one fix cannot cover only one spelling | +| `test_a_run_that_both_deselects_and_loses_a_test_still_fails` | **the distinguishing arm** | subtracting the deselected ids is right only if a genuinely lost test is still caught | + +Half of these are controls, and deliberately so: a run-shape guard fires on the whole +session, so a false positive costs the entire suite rather than one test. + +Read that first row precisely, because bare pytest is not silent here: it exits 1 +and prints `worker 'gw1' crashed while running 'test_loss.py::test_kills'`. What it +never mentions is `test_loss.py::test_d`, which was collected, assigned to the dead +worker, and never ran. Measured on a 6-test corpus under `-n 2 +--max-worker-restart=0`: 6 collected, 5 node-ids reported, and the missing one +appears in no line of the output. A suite whose crash happens to land on a test +already expected to fail therefore reports exactly what you expected while running +fewer tests than you wrote. + +Two things about the reconciliation took a measurement to get right. Under `xdist` +the **workers** collect, not the controller, so the controller's collected set stayed +empty and the guard was present and blind until it also listened to +`pytest_xdist_node_collection_finished`. And the state has to live on a per-config +plugin instance rather than module globals: `pytester.runpytest()` runs the inner +session in-process, so module-level sets leaked between these tests and the sessions +they drive. The corpus reported 44 passed and exited 1. + +The empty-parametrize refusal carries its own message rather than folding into the +bare-skip refusal. When a corpus glob matches nothing, the cause the reader needs to +see is the corpus, not the marker. + +## 11. Adding a test 0. **Write it twice.** Every test in this tree ships as a `.sh` suite and a pytest test **in the same change** (jd, 2026-09-09). Not ported later, not one or the @@ -761,7 +888,19 @@ an arm where the two differ is void rather than reported. failed the selftest on both majors of the matrix, which is how it was found. A new directory under `test/` inherits every rule the old ones follow. -## 10. Traps this corpus records +## 12. 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 +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. + +## 13. Traps this corpus records Recorded because each one produced a confident wrong result before it was caught, and all are the same family as the defect the layer exists to prevent. diff --git a/test/pytest/VACUITY_MODES.md b/test/pytest/VACUITY_MODES.md new file mode 100644 index 00000000..8ffb92ac --- /dev/null +++ b/test/pytest/VACUITY_MODES.md @@ -0,0 +1,262 @@ +# The vacuity modes: what is refused, what is not, and what nobody attacked + +A vacuity defect is a test that reports PASS while asserting nothing. This file is +the inventory: every way a pytest harness can do that which anyone here has +demonstrated, which of them `pgc_vacuity.py` refuses today, and which it does not. + +`TESTS.md` documents the tests that exist. This documents the ones that should. + +## 1. Where these numbers come from, and what did not run + +A five-angle enumeration ran in the audit container against pytest 9.1.1, +pytest-xdist 3.8.0 and psycopg 3.3.5. Each mode had to be demonstrated by an actual +run rather than described. + +| stage | started | completed | failed | +| --- | ---: | ---: | ---: | +| enumerate the modes | 5 | 5 | 0 | +| design a refusal per mode | 79 | 74 | 5 | +| **attack each refusal** | **148** | **0** | **148** | +| synthesize the layer | 1 | 0 | 1 | + +**The adversarial stage did not run.** It was cut off by a session limit, so the +summary line reading `defeated: 0` counts zero defeats out of **zero completed +attacks**. That number is not evidence that these refusals survive attack, and this +document is the synthesis the failed stage would have produced, written by hand from +the stage outputs that did complete. + +So: 79 modes produced by the run, of which **72 are named here** (see 1a), and **73 demonstrated by a run**. 74 refusals designed, every one of them +stating a residual. None of the 74 has been adversarially tested. + +## 1a. How to count a mode in this document + +**A mode is a backticked kebab-case identifier of three or more words**, such as +`collected-but-nothing-asserted`. That is the counting rule, stated because the +document had none and its numbers therefore could not be checked — which is a +poor property for a document about claims that cannot be checked. + +**A mode named in section 2 is refused, even where section 3 also names it.** +Section 3 keeps a back-reference to every mode that moved — "`X` is now closed" +— so a reader who knew a mode as unrefused finds out where it went. Counting +those references as unrefused lists one mode in both states, which is how the +first version of this rule reported 25 refused and 50 unrefused out of 72 named. +Section 3's total is therefore the ids it names MINUS the ids section 2 claims. + +Counted that way, and this is a measurement of the file rather than a +recollection of the run: + +| | modes | +| --- | ---: | +| named in section 2, refused today | 25 | +| named in section 3, not refused | 47 | +| **named in this document** | **72** | +| produced by the enumeration run | 79 | +| **named nowhere here** | **7** | + +**The enumeration produced 79; this document names 72 of them.** The other seven +were counted by the run and never transcribed, so they cannot be cited, checked +or built against. They are not a secret reserve of coverage — they are a gap in +this file. + +The run's own split was 23 refused and 56 not, against the 21 and 51 named here. +Those differ by exactly the seven that were never written down. Where the two +disagree, **the named ids are the record** and the run's totals are history: +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 +test in `test_layer.py` that fails without it. + +| mechanism | modes it closes | +| --- | --- | +| a test must make a counted assertion | `collected-but-nothing-asserted`, `return-instead-of-assert`, `assert-hidden-in-an-uncalled-helper` | +| `expect.rows` refuses two empty sides | `empty-equals-empty`, `empty-rows-equal-empty-rows`, `empty-vs-empty-set` | +| `expect.hash` refuses self-comparison, error sentinels, two empties | `oracle-against-itself`, `md5-of-the-empty-oracle` | +| `expect.at_least` refuses a floor of zero | `tautological-bound` | +| `expect.plan_marker` matches a typed key, never a substring | `substring-superstring`, `plan-substring-matches-property-or-prefix` | +| `plan_marker` refuses an absence claim over an empty plan | `absence-assertion-over-empty-plan` | +| `expect.rowcount` refuses psycopg's `-1` | `rowcount-minus-one-is-truthy-and-numeric` | +| a broad `except` is uncollectable, found by AST | `aborted-transaction-swallowed-into-one-fallback` | +| a bare skip fails the run | `skip-family-exit-0`, `all-tests-skipped-exit-zero`, `all-skipped-exits-zero` | +| `xfail_strict = true` | `xfail-xpass-and-the-wrong-exception`, `xfail-and-xpass-are-green` | +| `--pgc-expect-tests` asserts the run's own shape | `zero-collected-exit-5`, `filters-select-nothing`, `partial-selection-exits-zero` | +| the connection fixture is autocommit | `uncommitted-fixture-measures-an-empty-table` | +| `ordered_rows` refuses an unobservable ordering, and the scan refuses an order-killed value feeding it, in three spellings | `set-oracle-on-an-ordered-claim` | +| 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` | + +Three of those were added after checking this layer against the inventory rather +than reasoning about it, and all three had passed silently before: + +- `plan_marker(absent=True)` returned a pass against `[]`. An absence assertion is + satisfied by nothing being there at all, which is the case most worth catching. +- `expect.num(-1, -1)` passed. `cursor.rowcount` is `-1` when no count is available + and `1` for an unfetched `SELECT`; both are numbers. +- A broad `except` was forbidden **in a comment**, which enforces nothing. + +### 2.1 What the order-killer scan sees, and what it does not + +The scan first caught only `expect.ordered_rows(sorted(got), ...)` — the killer +written inside the argument. Two spellings of the same collapse walked past it, +and both read as more careful code than the one that was caught: + + g = sorted(got) # bound to a name first + expect.ordered_rows(g, want) + + got.sort() # killed in place; the call site is unchanged + expect.ordered_rows(got, want) + +All three are refused now. The scan is **one function deep**: a helper that sorts +and returns is invisible to it, as are aliases, attributes and branches. That is a +floor rather than a proof of order-sensitivity, and it is pinned by a test so the +limit cannot quietly turn into a claim of completeness. + +It compares line numbers, so a name sorted *after* the claim is not refused. A +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. +Grouped by what a reader needs to decide about them. + +### 3.1 The run can lose tests and still exit 0 + +A run that starts N tests and finishes fewer can still exit 0. Losing a test looks +exactly like never having written it. + +**`crashed-worker-silently-loses-tests` is now closed.** The layer reconciles the +collected node-id set against the reported one in the controlling process. + +The measurement is narrower than the mode name suggests. On a 6-test corpus under +`-n 2 --max-worker-restart=0`, bare pytest exits 1 and names the crash, so the run +is not green. But 5 node-ids reported against 6 collected, and `test_d` appears +nowhere in the output: it was assigned to the dead worker and never ran. The loss is +what is silent, not the crash. The guard names the missing node-ids and fails the +run on the difference. + +Two things that took a measurement to get right. Under xdist the **workers** collect, +not the controller, so the controller's set stayed empty and the guard was present +and blind until it also listened to `pytest_xdist_node_collection_finished`. And the +state has to live on a per-config plugin instance: `pytester.runpytest()` runs the +inner session in-process, so module-level sets leaked between the layer's own tests +and the sessions they drive, and 44 passing tests exited 1. + +Still open in this family: + +- `xdist-drops-the-deselected-count`, `env-deselect-passes-xdist-divergence-guard`, + `session-fixture-runs-once-per-worker`, `xdist-split-makes-a-loop-assert-vacuous` +- `process-exits-0-mid-run`, `retry-wrapper-greens-a-lossy-run`, + `junit-records-a-crash-as-error-failures-zero` + +### 3.2 The invocation throws the verdict away + +- `exit-5-lost-through-a-pipe` — measured: `pytest -q -k nosuch | tee run.log` + exits **0** without `pipefail` and **5** with it. The tee-the-log habit discards + the only vacuity guard pytest ships. +- `ci-step-swallows-the-exit-code`, `n-zero-silently-serial` — measured: `-n 0` runs + in-process with no workers, no warning, exit 0. So `-n "$PGC_JOBS"` with the + variable empty turns the parallel gate serial in silence. + +These are not fixable inside the plugin. They belong to whatever invokes it, which +is the same reason `test/run_all_versions.sh` carries its own accounting. + +### 3.3 Collection can go quiet + +**`empty-parametrize-is-a-silent-skip` and +`session-fixture-skip-greens-the-whole-suite` are now closed.** The first has its own +message rather than being folded into the bare-skip refusal, because the cause a +reader needs to see is the corpus, not the marker. The second catches any skip +arriving during setup, since `expect.cannot_run` records a counted assertion instead +of skipping. + +Still open in this family: + +- `conftest-import-failure`, `collection-error-and-continue-flag`, + `collection-error-loses-a-module-silently`, `collect-only-and-collect_ignore`, + `mark-typos-and-bare-marks` + +### 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. +- `same-broken-helper-both-sides`, `truthy-error-string`, `assert-not-unset-error`, + `zero-on-both-arms`, `tuple-assert-always-true`, `approx-of-nothing` + +### 3.5 The fixture built the wrong situation + +- `insert-wrote-no-rows` — `INSERT ... SELECT ... WHERE false` writes nothing and + raises nothing; `rowcount` is 0 and nobody reads it. +- `mutation-arm-unobservable` — both arms of an A/B produce the identical answer and + both are green. The assertion that would catch it, that the arms must **differ**, + is the one nobody writes. +- `error-swallowed-to-empty` — two queries raise, a helper turns each into the same + falsy value, and they compare equal. `lib.sh` closed this deliberately with a + unique `QUERY_ERROR.$seq` per failure. **The port has the sentinel constant but + nothing produces it.** +- `guc-set-but-path-never-engaged`, `aggregate-masks-empty-fixture`, + `db-derived-empty-parametrize`, `null-filter-matches-nothing`, + `loop-over-zero-rows`, `assert-inside-a-loop-over-zero-rows` + +### 3.6 psycopg's typed results introduce their own + +- `sql-null-to-python-none`, `none-conflates-null-no-row-and-missing-column` +- `dict-row-collapses-duplicate-columns`, `decimal-scale-and-null-aggregate` +- `truthy-cursor-from-execute`, `lossy-row-render` +- `only-first-result-set-fetched`, `multistatement-execute-positions-on-the-first-result` +- `executemany-returning-fetchall-sees-only-the-first-batch`, + `server-cursor-rowcount-is-not-a-row-count`, `empty-query-string-succeeds` + +The enumerating agent's own summary is worth keeping: psycopg **fixes** the half of +issue #418 where an error read as empty, because a failed statement raises and `[]` +can only mean zero rows. That improvement is exactly what will tempt a port to drop +the sentinels that close the other half, empty compared with empty. + +### 3.7 The guard itself goes quiet + +- `guard-as-teardown-fixture-still-reports-passed` — a guard implemented as a + teardown fixture leaves the test reporting PASSED. +- `session-accounting-guard`, `session-exit-rewrite-masks-a-real-failure`, + `description-guard-reopens-psycopg-raise`, + `mitigations-measured-and-the-one-that-does-not-work` + +## 4. The false-positive budget + +A guard that rejects legitimate tests gets switched off, and then the thing it +replaced is gone too. The layer has four escape hatches, each costing more to type +than the honest form: `allow_empty` takes a reason, `--pgc-expect-tests` takes the +real number, `cannot_run` takes a reason from a closed list, and `absent=True` +requires a plan that arrived. + +One false positive has already been hit and fixed. The broad-`except` refusal was +first written as a line regex and immediately rejected this layer's own tests, +because they contain the forbidden shape inside a `pytester.makepyfile` string. It +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.** + +## 5. What to add next, in order + +Each entry names the red test to write first. + +1. `test_expect_query_error_sentinel_is_unique_per_failure` — make something produce + `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`. + +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 +the constant already exists and nothing writes it. + +## 6. What this document cannot tell you + +None of the 74 refusal designs has been adversarially tested: the stage that would +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, +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 af398d4e..e312cbef 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -19,7 +19,9 @@ `check_num` refuses two identical md5 hashes for the same reason. """ +import ast import numbers +import pathlib import pytest @@ -108,6 +110,98 @@ def num(self, got, want, name): if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") + def row_set(self, got, want, name, allow_empty=None): + """Compare two result sets as SETS, order deliberately ignored. + + The counterpart to ordered_rows, and the port of pgc_set_hash. It exists so + that ignoring order is DECLARED rather than smuggled in by sorting at the + call site: `ordered_rows(sorted(x), ...)` reads like an ordering claim and is + not one, which is why the collection scan refuses it. + + pgc_check_ordered_oracle asserts three things, and this is the third: the set + oracle must be order-blind BY DESIGN. Without a control proving the two + instruments differ, an ordered oracle could quietly be implemented as a set + one and every ordering test in the tree would go silent. + """ + self.rows(sorted(map(repr, got)), sorted(map(repr, want)), name, + allow_empty=allow_empty) + + # -- ordered sequences --------------------------------------------------- + def ordered_rows(self, got, want, name): + """Compare two sequences IN ORDER, refusing the cases where order says nothing. + + This is the port of `pgc_seq_hash` and `diff_query_ordered`, which the harness + has had since #418 and this layer did not. It compares the sequences rather + than hashing them, for the same reason `rows` does: a mismatch names the + position, where a hash mismatch only says two hashes differ. + + THE REFUSAL THAT MATTERS IS THE SECOND ONE. A sequence whose elements are all + equal reads the same forwards and backwards, so an ordering claim about it + cannot fail. That is `pgc_check_ordered_oracle`'s premise inverted: the bash + version proves its oracle order-sensitive by requiring forward != reverse on a + known fixture, and the same requirement applied to a caller's data is what + stops an ordered assertion being decorative. + """ + g, w = list(got), list(want) + if not g and not w: + raise VacuityError( + f"{name}: both sequences are empty, so this comparison could not " + f"have failed. Use rows(..., allow_empty='why') if empty is the point." + ) + if len(set(map(repr, g))) < 2 and len(set(map(repr, w))) < 2: + raise VacuityError( + f"{name}: order cannot be observed in these sequences. Every element " + f"is the same, so the reverse ordering is identical and the claim " + f"asserts nothing beyond what rows() already asserts." + ) + self._counted() + if g != w: + for i, (a, b) in enumerate(zip(g, w)): + if a != b: + raise AssertionError( + f"{name}: first difference at position {i}: got {a!r} want {b!r}" + ) + raise AssertionError( + f"{name}: same prefix, different length: got {len(g)} rows want {len(w)}" + ) + + def ordering_observable(self, forward, reverse, name): + """Assert this fixture can distinguish order at all, before relying on it. + + `pgc_check_ordered_oracle` ported. Read the same rows both ways and require + the two to differ: a fixture that reads identically forwards and backwards + supports no ordering claim, and every ordered assertion over it is vacuous + however carefully it is written. + """ + f, r = list(forward), list(reverse) + if not f and not r: + raise VacuityError(f"{name}: both directions are empty.") + self._counted() + if f == r: + raise AssertionError( + f"{name}: the forward and reverse readings are identical, so nothing " + f"in this fixture can detect an ordering error. Give it rows whose " + f"order is observable before asserting order." + ) + + # -- row counts --------------------------------------------------------- + def rowcount(self, got, want, name): + """Compare a row count, refusing psycopg's "no count available" sentinel. + + cursor.rowcount is -1 when the statement produced no count, and measured on + a live server it is 1 for an unfetched SELECT -- neither is a number of + rows. Both are numbers, so expect.num compares them happily: num(-1, -1) + passes. A count that matters should come from count(*) or from len() of the + rows actually fetched. + """ + for side, v in (("left", got), ("right", want)): + if v == -1: + raise VacuityError( + f"{name}: the {side} side is -1, which is psycopg's " + f"\"no row count available\" and not a number of rows." + ) + self.num(got, want, name) + # -- row sets ---------------------------------------------------------- def rows(self, got, want, name, allow_empty=None): """Compare two result sets. Refuses two empty sides unless declared. @@ -246,7 +340,31 @@ def refusal(self, result, name, *patterns): ) self._counted() result.assert_outcomes(failed=1, passed=0) - result.stdout.fnmatch_lines([f"*{p}*" for p in patterns]) + # ANCHORED TO pytest's ERROR-LINE PREFIX, and that is the whole point. + # + # This was `f"*{p}*"`, which searches the inner run's WHOLE stdout -- + # and pytest prints the enclosing function's SOURCE in a traceback, + # including lines that never executed. So the pattern matched the + # guard's own string literal in the traceback rather than anything the + # guard produced. Measured: with `hash()`'s left-sentinel guard + # neutered, the inner output still contains + # + # raise VacuityError(f"{name}: the left side is a failed query: ...") + # E AssertionError: a failed query on the left: got ... want ... + # + # and `*the left side is a failed query*` matched the first line. Every + # message in a function is printed whenever anything in it fails. + # + # THAT IS THE DEFECT THIS HELPER EXISTS TO PREVENT, IN THIS HELPER. + # `outcomes(failed=1)` is satisfied by any refusal; requiring the message + # was meant to fix it, and matching printed source meant it did not -- + # it was satisfied by any failure in a function whose source contains the + # phrase. A census over the layer found SEVEN guards unheld this way. + # + # `E` is the prefix pytest puts on the raised-exception lines of a + # traceback, so the phrase must now appear in a message rather than + # anywhere in the file. + result.stdout.fnmatch_lines([f"E*{p}*" for p in patterns]) def outcomes(self, result, name, **want): """Assert on an INNER pytest run's outcomes, and count it. @@ -315,6 +433,7 @@ def plan_marker(self, plan, key, name=None, absent=False): seen.update(k for k in node if k.startswith("Columnar")) if key in node: found = True + self._counted() if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") @@ -398,10 +517,6 @@ def pytest_runtest_logreport(self, report): self.items.append((report.nodeid, reason, detail)) -def pytest_configure(config): - collector = _UnrunnableCollector() - config.pluginmanager.register(collector, "pgc_unrunnable_collector") - config.pgc_unrunnable = collector @pytest.hookimpl(wrapper=True) @@ -446,10 +561,114 @@ def pytest_sessionfinish(session, exitstatus): collector = getattr(session.config, "pgc_unrunnable", None) if collector is None or not collector.items: return - if exitstatus == 0: + # Both, not just the argument: _RunShape may already have escalated this run + # to 1 for a lost test, and `exitstatus` is the value from before that. + if exitstatus == 0 and session.exitstatus == 0: session.exitstatus = EXIT_INCOMPLETE +# THE RUN'S OWN SHAPE, HELD PER SESSION. +# +# Three modes remove many tests at once while the run reads green, so they are worth +# more than any per-assertion guard. Counting collected tests cannot see them: a +# crashed xdist worker loses its remaining tests and the collected count is still +# right. Measured under --max-worker-restart=0: 8 collected, summary "1 failed, +# 6 passed", one named test never reported, and pytest printed no warning. +# +# AN INSTANCE PER CONFIG, NOT MODULE GLOBALS. pytester.runpytest() runs the inner +# session IN-PROCESS, so module-level sets are shared between the layer's own tests +# and the sessions they drive. Measured before this was fixed: 44 tests passed and +# the run exited 1, because the outer session had inherited every inner run's +# collected ids and setup skips. State that belongs to a session has to live on the +# session. +class _RunShape: + def __init__(self): + self.collected = set() + self.reported = set() + self.setup_skips = [] + + def pytest_collection_modifyitems(self, items): + # Fires in the controller when running serially, and in each worker under + # xdist. Harmless in a worker: the worker's own sessionfinish returns early. + self.collected.update(i.nodeid for i in items) + + def pytest_deselected(self, items): + """Deselection is not loss, and the difference is the whole guard. + + `pytest_collection_modifyitems` above fires before pytest's own -k and -m + filtering has removed anything, so without this hook every deselected test + looks like a test that vanished without reporting. Measured before the fix: + `pytest -q test_layer.py -k refus` gave "1 passed, 15 deselected" and then + exit 1 with "15 collected test(s) never reported an outcome". That is a + false red on a healthy run, produced by the guard whose subject is false + greens -- and the first thing anyone does about it is stop using -k. + + Asking for a subset is a deliberate act by whoever typed the command. A + test lost to a crashed worker is not. This hook is where pytest tells the + difference, so it is where the guard has to learn it. + """ + self.collected.difference_update(i.nodeid for i in items) + + def pytest_xdist_node_collection_finished(self, node, ids): + """Under xdist the WORKERS collect, not the controller. + + Measured: with -n 2 the controller's collected set stayed empty, so the + reconciliation had nothing to compare and a crashed worker's lost tests went + unreported -- the guard was there and blind. xdist hands the controller each + node's collected ids through this hook, which is the only place the + controller learns what was found. + """ + self.collected.update(ids) + + def pytest_runtest_logreport(self, report): + """Record that a test produced an outcome, and catch a skip during SETUP. + + A skip in setup is how one fixture removes every test that depends on it: a + session fixture calling pytest.skip() turns "the cluster would not start" + into exit 0. expect.cannot_run does not skip, it records a counted + assertion, so any skip arriving here came from somewhere else. + """ + if report.when == "call" or (report.when == "setup" + and report.outcome != "passed"): + self.reported.add(report.nodeid) + if report.when == "setup" and report.skipped: + self.setup_skips.append(report.nodeid) + + def pytest_sessionfinish(self, session, exitstatus): + # Only the process holding the whole picture can reconcile: an xdist worker + # sees a slice, and the controller receives every worker's reports. + if hasattr(session.config, "workerinput"): + return + problems = [] + missing = sorted(self.collected - self.reported) + if missing: + problems.append( + f"{len(missing)} collected test(s) never reported an outcome, so the " + f"run lost them silently: " + ", ".join(missing[:5]) + + (" ..." if len(missing) > 5 else "") + ) + if self.setup_skips: + problems.append( + f"{len(self.setup_skips)} test(s) were skipped during setup, which is " + f"how one fixture removes every test that depends on it: " + + ", ".join(sorted(self.setup_skips)[:5]) + + (" ..." if len(self.setup_skips) > 5 else "") + + " -- use expect.cannot_run(REASON, detail) in the test instead" + ) + if problems: + print("\nVACUITY: " + " AND ".join(problems)) + session.exitstatus = 1 + + +def pytest_configure(config): + # Two independent per-session mechanisms, both registered here because a + # plugin module may define pytest_configure only once. + collector = _UnrunnableCollector() + config.pluginmanager.register(collector, "pgc_unrunnable_collector") + config.pgc_unrunnable = collector + config.pluginmanager.register(_RunShape(), f"pgc_runshape_{id(config)}") + + def pytest_addoption(parser): parser.addoption( "--pgc-expect-tests", @@ -482,6 +701,146 @@ def pytest_collection_finish(session): ) +# A broad except in a test swallows the failure the test exists to find. +# +# After ANY failed statement psycopg raises InFailedSqlTransaction for every later +# one, so a single `except Exception` around a test body hides the real error AND +# every error after it. The layer used to forbid this in a comment, which enforces +# nothing: measured, a test using the forbidden shape passed with no complaint. +# +# PARSED, NOT GREPPED. The first version matched lines with a regex and immediately +# fired on this file's own tests, because they contain the forbidden shape inside a +# `pytester.makepyfile` string. A guard that rejects a legitimate test is a guard +# somebody switches off, and a line regex over source cannot tell code from a string +# literal -- the same mistake as matching a plan by substring. ast can: a handler +# inside a string is not an ExceptHandler node. +# An ordered claim whose inputs were SORTED cannot fail on order. +# +# ordered_rows is order-sensitive, so the vacuity is introduced at the call site: +# `expect.ordered_rows(sorted(got), sorted(want))` compares two sequences that were +# just put in the same order. This is the collapse VACUITY_MODES.md records as +# set-oracle-on-an-ordered-claim, and lib.sh has no equivalent because bash has no +# sorted() to reach for. +# +# Parsed, not grepped, for the same reason as the except scan below. +_ORDER_KILLERS = ("sorted", "set", "frozenset") + + +def _order_killed_names(fn): + """Names bound to an order-killing value earlier in one function body. + + -> {name: (lineno, how)} + + The inline spelling is only the shortest way to write the collapse. These two + are the same defect and read as more careful code, which is worse: + + g = sorted(got) # bound to an order-killing call + expect.ordered_rows(g, want) + + got.sort() # killed in place + expect.ordered_rows(got, want) + + WHAT THIS DOES NOT SEE, stated because a guard's blind spots are part of its + meaning: it is one function deep, so a helper that sorts and returns is invisible; + it does not follow aliases (`h = g`), attributes (`self.rows.sort()`), branches, + or a name re-bound to something honest after being killed. It is a floor, not a + proof of order-sensitivity. The suite's own removal proofs are what establish + that an ordered claim can actually fail on order. + """ + killed = {} + for node in ast.walk(fn): + # X = sorted(...) / set(...) / frozenset(...) + if isinstance(node, ast.Assign) and isinstance(node.value, ast.Call): + f = node.value.func + if isinstance(f, ast.Name) and f.id in _ORDER_KILLERS: + for t in node.targets: + if isinstance(t, ast.Name): + killed.setdefault(t.id, (node.lineno, f"{f.id}()")) + # X.sort() -- in place, and the name keeps its spelling at the call site + elif isinstance(node, ast.Expr) and isinstance(node.value, ast.Call): + f = node.value.func + if (isinstance(f, ast.Attribute) and f.attr == "sort" + and isinstance(f.value, ast.Name)): + killed.setdefault(f.value.id, (node.lineno, ".sort()")) + return killed + + +def _sorted_ordered_sites(path): + try: + tree = ast.parse(pathlib.Path(path).read_text()) + except (OSError, SyntaxError): + return [] + out = [] + name = pathlib.Path(path).name + # Per function, because a killed name means nothing outside the body that + # killed it, and a module-level walk would carry one test's `g` into the next. + fns = [n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))] + for fn in fns: + killed = _order_killed_names(fn) + for node in ast.walk(fn): + if not isinstance(node, ast.Call): + continue + f = node.func + if not (isinstance(f, ast.Attribute) and f.attr in ("ordered_rows", + "ordering_observable")): + continue + for arg in node.args: + if (isinstance(arg, ast.Call) and isinstance(arg.func, ast.Name) + and arg.func.id in _ORDER_KILLERS): + out.append( + f"{name}:{node.lineno} {arg.func.id}() feeds an ordered claim" + ) + elif isinstance(arg, ast.Name) and arg.id in killed: + where, how = killed[arg.id] + # Only a kill that already happened. A name sorted AFTER the + # claim was made did not affect it, and flagging that would be + # a false red -- the thing this whole layer exists to refuse. + if where < node.lineno: + out.append( + f"{name}:{node.lineno} {arg.id} was order-killed by " + f"{how} at line {where} and feeds an ordered claim" + ) + return out + + +def _broad_except_sites(path): + try: + tree = ast.parse(pathlib.Path(path).read_text()) + except (OSError, SyntaxError): + return [] + out = [] + name = pathlib.Path(path).name + for node in ast.walk(tree): + if not isinstance(node, ast.ExceptHandler): + continue + t = node.type + if t is None: + out.append(f"{name}:{node.lineno} bare except") + continue + # A TUPLE HANDLER IS THE SHAPE PEOPLE ACTUALLY WRITE. + # + # This looked only at a bare `ast.Name`, so `except Exception:` was + # refused and `except (ValueError, Exception):` passed (@jdatcmd, #905 + # review). Measured against the real layer, three spellings of one + # swallow: + # + # except Exception: -> refused + # except (ValueError, Exception): -> PASSED <- the hole + # except BaseException: -> refused + # + # A tuple is how this gets written when someone starts with a specific + # exception and widens it under pressure, which is the exact moment the + # guard is for -- so the hole was in the case the guard most needed to + # cover. Any member of the tuple being broad makes the handler broad. + members = t.elts if isinstance(t, ast.Tuple) else [t] + for m in members: + if isinstance(m, ast.Name) and m.id in ("Exception", "BaseException"): + out.append(f"{name}:{node.lineno} except {m.id}") + break + return out + + def pytest_collection_modifyitems(config, items): """Refuse a bare skip, which exits 0 and reads as success. @@ -489,13 +848,71 @@ def pytest_collection_modifyitems(config, items): only through expect.cannot_run(), which names a reason from a closed list. """ offenders = [] + seen_files = set() for item in items: for marker in ("skip", "skipif"): - if item.get_closest_marker(marker) is not None: - offenders.append(f"{item.name} carries a bare @pytest.mark.{marker}") + mk = item.get_closest_marker(marker) + if mk is None: + continue + why = str(mk.kwargs.get("reason", "")) or (str(mk.args[0]) if mk.args else "") + if "empty parameter set" in why: + continue # reported below, with a message about the real cause + offenders.append(f"{item.name} carries a bare @pytest.mark.{marker}") + f = str(getattr(item, "fspath", "") or "") + if f and f not in seen_files: + seen_files.add(f) + for site in _broad_except_sites(f): + offenders.append(f"{site} catches Exception broadly") + offenders.extend(_sorted_ordered_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 + # that matched nothing turns a data-driven suite into a single "s" and exit 0. + empty_params = [] + for item in items: + m = item.get_closest_marker("skip") + reason = "" + if m is not None: + reason = str(m.kwargs.get("reason", "")) or ( + str(m.args[0]) if m.args else "") + if "empty parameter set" in reason: + empty_params.append(f"{item.name}: {reason}") + if empty_params: + raise pytest.UsageError( + "the pgColumnar vacuity layer refuses this run: a parametrize over an " + "empty parameter set produces one skipped placeholder and exits 0, so a " + "corpus that matched nothing reads as a suite that ran: " + + "; ".join(empty_params) + + " -- assert the corpus is non-empty before parametrizing over it." + ) if offenders: + # One hook, two offences, so the message must say which. An earlier version + # reused the skip wording and told a reader with a broad `except` to call + # expect.cannot_run, which would not have helped them. + 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] + parts = [] + if skips: + parts.append( + "a bare skip is refused, because it exits 0 and reads as success: " + + "; ".join(skips) + + " -- use expect.cannot_run(REASON, detail) so the run cannot go quiet" + ) + if excepts: + parts.append( + "a broad except swallows the failure the test exists to find, and " + "after one failed statement psycopg raises for every later one: " + + "; ".join(excepts) + + " -- catch the specific exception class instead" + ) + if ordered: + parts.append( + "sorted() or set() feeding an ordered claim removes the very " + "ordering it asserts: " + + "; ".join(ordered) + + " -- pass the rows in the order the query returned them" + ) raise pytest.UsageError( - "bare skip is refused by the pgColumnar vacuity layer: " - + "; ".join(offenders) - + ". Use expect.cannot_run(REASON, detail) so the run cannot go quiet." + "the pgColumnar vacuity layer refuses this run: " + ". ".join(parts) + "." ) diff --git a/test/pytest/test_build_refusal.py b/test/pytest/test_build_refusal.py index f0d976b5..813c6137 100644 --- a/test/pytest/test_build_refusal.py +++ b/test/pytest/test_build_refusal.py @@ -336,10 +336,17 @@ def test_make_cluster_leaves_nothing_behind_when_setup_fails(tmp_path, expect): import glob before = set(glob.glob("/tmp/pgc-pytest-*")) + # NARROW, and named rather than caught broadly. The broad-except guard that + # arrives with the vacuity inventory refuses `except Exception` here and is + # right to: after one failed statement psycopg raises for every later one, so + # a broad catch hides the real error and all its successors. A missing + # pg_config raises FileNotFoundError out of the subprocess layer; anything + # else escapes and fails the test loudly, which is what should happen to an + # error this arm did not predict. raised = "no" try: make_cluster("/nonexistent/bin/pg_config", "gw77") - except Exception: + except (FileNotFoundError, RuntimeError, OSError): raised = "yes" expect.text(raised, "yes", "premise: setup really failed, so the arm is not vacuous") diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 4a765e71..93c90e71 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -151,3 +151,113 @@ def test_a_stated_total_that_disagrees_with_disk_is_visible(tmp_path, expect): "while the fixture on disk holds 2 in 1") expect.num(int(stated_totals(doc) == (sum(len(v) for v in found.values()), len(found))), 0, "a stated total that disagrees with disk does not compare equal") + + +# --------------------------------------------------------------------------- +# VACUITY_MODES.md counts itself, and the count is checked. +# +# @jdatcmd found README.md and VACUITY_MODES.md disagreeing about how many modes +# the layer refuses, and could not check either because the document offered NO +# COUNTING RULE. A document whose subject is claims that cannot be checked should +# not make one. Section 1a now defines a mode as a backticked kebab id of three +# or more words; this asserts the numbers 1a states are the numbers on disk. + +MODE_ID = re.compile(r"`([a-z0-9]+(?:-[a-z0-9]+){2,})`") +MODES_DOC = HERE / "VACUITY_MODES.md" + + +def _named_modes(): + """-> (refused, not_refused, all) per section 1a's rule.""" + text = MODES_DOC.read_text() + chunks = {} + for chunk in re.split(r"^## ", text, flags=re.M): + head = chunk.splitlines()[0] if chunk.strip() else "" + chunks[head] = set(MODE_ID.findall(chunk)) + refused = next((v for k, v in chunks.items() if k.startswith("2.")), set()) + not_refused = next((v for k, v in chunks.items() if k.startswith("3.")), set()) + # Section 3 keeps a back-reference to every mode that moved into section 2 + # ("`X` is now closed"), so a mode can be named in both. Section 2 wins: a + # refused mode is refused. Without this the same id is counted in two states + # and the totals stop adding up -- measured at 25 + 50 against 72 named. + not_refused = not_refused - refused + return refused, not_refused, set().union(*chunks.values()) if chunks else set() + + +def test_the_mode_inventory_states_its_own_totals_correctly(expect): + """The numbers in section 1a must be the numbers on disk. + + Not a tidiness check: these totals are how a reader decides whether a gap is + covered, and they were wrong in two files at once with no way to tell. + """ + refused, not_refused, allm = _named_modes() + expect.at_least(len(allm), 20, "premise: the counting rule finds modes at all") + + doc = MODES_DOC.read_text() + for label, got in (("refused today", len(refused)), + ("not refused", len(not_refused)), + ("named in this document", len(allm))): + row = re.search(rf"\|[^|\n]*{re.escape(label)}[^|\n]*\|\s*\**(\d+)", doc) + expect.text(repr(row is not None), "True", + f"section 1a states a total for {label!r}") + expect.num(int(row.group(1)), got, + f"the stated total for {label!r} is the number on disk") + + +def test_the_readme_and_the_inventory_agree_on_what_is_refused(expect): + """They did not, and neither could be checked against anything. + + README.md said 23 refused while the inventory named 21 — the run's number + against the document's, with nothing to distinguish them. + """ + refused, _, _ = _named_modes() + readme = (HERE / "README.md").read_text() + expect.at_least(readme.count(f"{len(refused)} refused"), 1, + "README.md quotes the number of modes actually named as refused") + + +def test_the_inventory_accounts_for_every_mode_the_run_found(expect): + """The 'named nowhere here' row is the gap this document admits to. + + It is arithmetic between numbers the document states, so it can go stale on + its own: an editor who transcribes a missing mode updates the named total and + leaves the gap row claiming a gap that has closed. The enumeration's own 79 is + history -- it is not on disk, and this does not pretend to check it. + """ + doc = MODES_DOC.read_text() + + def row(label): + m = re.search(rf"\|[^|\n]*{re.escape(label)}[^|\n]*\|\s*\**(\d+)", doc) + expect.text(repr(m is not None), "True", f"section 1a states {label!r}") + return int(m.group(1)) + + refused, not_refused, _ = _named_modes() + expect.num(len(refused) + len(not_refused), row("named in this document"), + "the two section totals sum to the document total") + expect.num(row("produced by the enumeration run") - row("named in this document"), + row("named nowhere here"), + "the admitted gap is the run's total minus what is written down") + + +def test_the_prose_totals_match_the_counted_modes(expect): + """Section 1a's table was not the only place a total lived. + + Three sentences outside it still asserted the run's 23 after the table said 21 -- + section 2's opening, the closing paragraph, and TESTS.md. A table that is checked + and prose that is not means the drift simply moves into the prose, which is where + it was in the first place. + + The run's own 23 appears once on purpose, as history, and is not touched here: + what is gated is every sentence that states what the layer refuses TODAY. + """ + refused, _, _ = _named_modes() + n = len(refused) + for path, pattern in ( + (MODES_DOC, r"(\d+) of the 79"), + (MODES_DOC, r"known to refuse (\d+) demonstrated modes"), + (HERE / "TESTS.md", r"This layer refuses (\d+)"), + ): + m = re.search(pattern, path.read_text()) + expect.text(repr(m is not None), "True", + f"{path.name} states a refused total matching {pattern!r}") + expect.num(int(m.group(1)), n, + f"{path.name}: the prose total is the number of ids named") diff --git a/test/pytest/test_guards_pinned.py b/test/pytest/test_guards_pinned.py index 981d498d..e69655d6 100644 --- a/test/pytest/test_guards_pinned.py +++ b/test/pytest/test_guards_pinned.py @@ -261,3 +261,34 @@ def test_absent_on_nothing(expect): expect.plan_marker([], "Columnar Projected Columns", absent=True) '''), "plan_marker refuses an absence claim over an empty plan", "plan has no nodes") + + +# The ordered oracle's own guards. These live here rather than in the base +# branch's copy of this file because the guards they pin do not exist until the +# ordered oracle does. +# +# BOTH ARE UNREACHABLE BY SUBSUMPTION, which is why they need the message and +# not just the outcome. Neuter either one and a NEIGHBOURING refusal fires on +# the same input, so the inner run still fails and an arm asserting only +# `failed=1` still passes. A census over the full stack found exactly these two +# unheld after the rest of the layer was pinned. + + +def test_ordered_rows_both_empty_names_its_own_refusal(pytester, expect): + """Neutered, the UNOBSERVABLE guard fires on `[], []` instead: every element + of an empty sequence is trivially the same, so that guard also matches.""" + expect.refusal(_inner(pytester, ''' + def test_two_empty(expect): + expect.ordered_rows([], [], "two empty sequences") + '''), "ordered_rows names its own both-empty refusal", + "both sequences are empty") + + +def test_ordering_observable_both_empty_names_its_own_refusal(pytester, expect): + """Neutered, the `forward == reverse` AssertionError fires instead, because + two empty readings are equal.""" + expect.refusal(_inner(pytester, ''' + def test_empty_directions(expect): + expect.ordering_observable([], [], "no rows either way") + '''), "ordering_observable names its own both-empty refusal", + "both directions are empty") diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index 831bd15c..8f1ae4b9 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -254,3 +254,39 @@ def test_ok(expect): ) result = pytester.runpytest("-p", "pgc_vacuity") expect.num(result.ret, 0, "an ordinary green run is untouched") + + +def test_layer_rejects_psycopgs_no_count_sentinel(pytester, expect): + """cursor.rowcount is -1 when no count is available, and 1 for an unfetched + SELECT. Both are numbers, so expect.num compares them happily.""" + pytester.makepyfile( + """ + def test_rowcount_sentinel(expect): + expect.rowcount(-1, -1, "a count that is not a count") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "the -1 sentinel is refused", failed=1, passed=0) + result.stdout.fnmatch_lines(["*no row count available*"]) + + +def test_layer_rejects_a_broad_except_in_a_test_file(pytester, expect): + """The layer forbade this in a comment, which enforces nothing. + + After any failed statement psycopg raises InFailedSqlTransaction for every + later one, so one `except Exception` hides the real error and all its + successors. Measured: a test using the forbidden shape passed with no complaint. + """ + pytester.makepyfile( + """ + def test_swallows(expect): + try: + raise RuntimeError("the real failure") + except Exception: + pass + expect.num(1, 1, "and then asserts something harmless") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a broad except must not be collectable") + result.stderr.fnmatch_lines(["*catches Exception broadly*"]) diff --git a/test/pytest/test_native_projection.py b/test/pytest/test_native_projection.py index 535d963e..6e05bcf2 100644 --- a/test/pytest/test_native_projection.py +++ b/test/pytest/test_native_projection.py @@ -5,11 +5,16 @@ original appears here with the same name, so the two can be compared mechanically. ONE DELIBERATE DIFFERENCE IN MECHANISM. The bash suite compares -`md5(string_agg(t ORDER BY t))`. This port compares the row sets themselves, as -sorted Python lists. That asserts the same property by a stronger means: an md5 -mismatch tells you two hashes differ, a row-set mismatch tells you which row. It -also avoids recomputing the hash in Python, where encoding and collation could -make the same rows hash differently. +`md5(string_agg(t ORDER BY t))`, which is order-blind by construction. This port +compares the result sets themselves through `expect.row_set`, which is order-blind +by declaration. That asserts the same property by a stronger means: an md5 mismatch +tells you two hashes differ, a row-set mismatch tells you which row. It also avoids +recomputing the hash in Python, where encoding and collation could make the same +rows hash differently. + +The fetch helper returns rows in query order and the assertion chooses the +comparison, because sorting inside the helper is how an ordering claim silently +becomes a set one. """ import pytest @@ -35,9 +40,17 @@ def fo(pgc_conn): def _rows(conn, sql, params=None): + """Rows in the order the query returned them. + + NOT sorted here. Which comparison is wanted is the assertion's business, and + sorting in the fetch helper is how an ordered claim silently becomes a set one: + the caller says expect.row_set for an order-blind comparison and + expect.ordered_rows for an ordering claim, and the collection scan refuses + sorted() feeding the latter. + """ with conn.cursor() as cur: cur.execute(sql, params) - return sorted(r[0] for r in cur.fetchall()) + return [r[0] for r in cur.fetchall()] def _scalar(conn, sql, params=None): @@ -59,14 +72,14 @@ def test_fp_fanout_matches_base(fo, expect): """bash: 'fp fan-out matches base (a,c)'""" got = _rows(fo, "SELECT pgcolumnar.read_projection('fo','fp')") want = _rows(fo, "SELECT a::text || '|' || c::text FROM fo") - expect.rows(got, want, "fp fan-out matches base (a,c)") + expect.row_set(got, want, "fp fan-out matches base (a,c)") def test_fq_fanout_matches_base(fo, expect): """bash: 'fq fan-out matches base (b)'""" got = _rows(fo, "SELECT pgcolumnar.read_projection('fo','fq')") want = _rows(fo, "SELECT b FROM fo") - expect.rows(got, want, "fq fan-out matches base (b)") + expect.row_set(got, want, "fq fan-out matches base (b)") def test_fp_row_count_matches_base(fo, expect): @@ -105,7 +118,7 @@ def test_fp_reflects_deletes(fo, expect): got = _rows(fo, "SELECT pgcolumnar.read_projection('fo','fp')") want = _rows(fo, "SELECT a::text || '|' || c::text FROM fo") - expect.rows(got, want, "fp reflects deletes (a,c)") + expect.row_set(got, want, "fp reflects deletes (a,c)") pcount = _scalar(fo, "SELECT count(*) FROM pgcolumnar.read_projection('fo','fp')") bcount = _scalar(fo, "SELECT count(*) FROM fo") diff --git a/test/pytest/test_ordered.py b/test/pytest/test_ordered.py new file mode 100644 index 00000000..196ae9ec --- /dev/null +++ b/test/pytest/test_ordered.py @@ -0,0 +1,225 @@ +"""The ordered oracle, tested before it refuses anything. + +lib.sh has pgc_seq_hash, diff_query_ordered and pgc_check_ordered_oracle; the port +had no way to express an ordered claim at all, so a test naming ORDER BY compared +with sorted lists could not fail on order (VACUITY_MODES.md, set-oracle-on-an- +ordered-claim). +""" + + +def test_ordered_rows_refuses_a_sequence_whose_order_is_unobservable(pytester, expect): + """If every element is the same, forward equals reverse and order asserts nothing. + + This is pgc_check_ordered_oracle's premise ported: the bash version proves the + oracle is order-sensitive by requiring forward != reverse on a real fixture. + """ + pytester.makepyfile( + """ + def test_all_the_same(expect): + expect.ordered_rows(['x', 'x', 'x'], ['x', 'x', 'x'], "order of identicals") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "an unobservable ordering is refused", failed=1, passed=0) + result.stdout.fnmatch_lines(["*order cannot be observed*"]) + + +def test_ordered_rows_refuses_two_empty_sequences(pytester, expect): + pytester.makepyfile( + """ + def test_both_empty(expect): + expect.ordered_rows([], [], "two empty sequences") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "two empty sequences are refused", failed=1, passed=0) + + +def test_ordered_rows_accepts_a_real_ordering(pytester, expect): + """The positive control: a genuine ordered claim must still pass.""" + pytester.makepyfile( + """ + def test_real_order(expect): + expect.ordered_rows([1, 2, 3], [1, 2, 3], "a real ordering") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a real ordering passes", passed=1, failed=0) + + +def test_ordered_rows_fails_on_the_wrong_order(pytester, expect): + """And it must actually detect order, not merely permit the claim.""" + pytester.makepyfile( + """ + def test_wrong_order(expect): + expect.ordered_rows([3, 1, 2], [1, 2, 3], "the wrong order") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "the wrong order is caught", failed=1, passed=0) + + +def test_layer_refuses_sorting_the_input_to_an_ordered_claim(pytester, expect): + """`ordered_rows(sorted(got), sorted(want))` cannot fail on order. + + This is the collapse itself: the helper is order-sensitive, so the vacuity is + introduced at the CALL SITE. Found by ast, for the same reason the broad-except + scan parses rather than greps. + """ + pytester.makepyfile( + """ + def test_sorted_into_an_ordered_claim(expect): + got = [3, 1, 2] + expect.ordered_rows(sorted(got), sorted([1, 2, 3]), "sorted away") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "sorting the input to an ordered claim is refused") + result.stderr.fnmatch_lines(["*sorted*ordered*"]) + + +def test_layer_refuses_a_name_bound_to_a_sorted_call(pytester, expect): + """The same collapse, one line apart, and it read as more careful code. + + The inline spelling was the only one caught. Splitting it across two statements + is what a reader does when the line gets long, so the guard was strictest on the + version most likely to be noticed by a human and blind to the version least + likely to be. + """ + pytester.makepyfile( + """ + def test_sorted_via_a_name(expect): + got = [3, 1, 2] + g = sorted(got) + expect.ordered_rows(g, [1, 2, 3], "sorted away, one line earlier") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a name bound to sorted() is refused") + result.stderr.fnmatch_lines(["*order-killed*"]) + + +def test_layer_refuses_a_list_sorted_in_place(pytester, expect): + """`got.sort()` kills the order and leaves the name spelled exactly as before. + + Nothing at the call site says anything happened, which makes this the hardest + of the three to see in review and the one worth catching most. + """ + pytester.makepyfile( + """ + def test_sorted_in_place(expect): + got = [3, 1, 2] + got.sort() + expect.ordered_rows(got, [1, 2, 3], "sorted in place") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a list sorted in place is refused") + result.stderr.fnmatch_lines(["*order-killed*"]) + + +def test_layer_allows_a_name_sorted_after_the_claim(pytester, expect): + """The control, and the reason the guard compares line numbers. + + A name sorted AFTER the ordered claim did not affect it. Refusing that would be + a false red, which is the defect this layer exists to refuse rather than commit. + """ + pytester.makepyfile( + """ + def test_sorted_afterwards(expect): + got = [1, 2, 3] + expect.ordered_rows(got, [1, 2, 3], "a real ordering claim") + got.sort() # after the claim; it changed nothing about it + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "sorting after the claim is not the collapse", + passed=1, failed=0) + + +def test_the_order_killer_scan_is_one_function_deep(pytester, expect): + """A named limit, pinned so it cannot quietly become a claim of completeness. + + A helper that sorts and returns is invisible to this scan. That is a real gap and + it is recorded here rather than in prose alone: if someone widens the scan later, + this arm reddens and tells them the documented limit has moved. + """ + pytester.makepyfile( + """ + def _tidy(rows): + return sorted(rows) + + def test_sorted_behind_a_helper(expect): + got = [3, 1, 2] + expect.ordered_rows(_tidy(got), [1, 2, 3], "sorted behind a helper") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a sort behind a helper is NOT caught, by design", + passed=1, failed=0) + + +def test_ordering_observable_requires_the_two_directions_to_differ(pytester, expect): + """The ported premise: a fixture that reads the same forwards and backwards + cannot support an ordering claim at all.""" + pytester.makepyfile( + """ + def test_premise_fails(expect): + expect.ordering_observable(['a', 'a'], ['a', 'a'], "a flat fixture") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a flat fixture is refused as a premise", failed=1, passed=0) + + +def test_ordering_observable_passes_when_the_directions_differ(pytester, expect): + pytester.makepyfile( + """ + def test_premise_holds(expect): + expect.ordering_observable([1, 2, 3], [3, 2, 1], "a real fixture") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a real fixture passes the premise", passed=1, failed=0) + + +def test_the_two_oracles_are_different_instruments(pytester, expect): + """pgc_check_ordered_oracle's third property, ported. + + The ordered oracle must be order-SENSITIVE and the set oracle order-BLIND. Without + this control an ordered oracle could be implemented as a set one and every + ordering test in the tree would go silent while staying green. + """ + pytester.makepyfile( + """ + FWD = [1, 2, 3] + REV = [3, 2, 1] + + def test_the_set_oracle_is_order_blind(expect): + # Order-blind BY DESIGN: the same rows in any order compare equal. + expect.row_set(FWD, REV, "set oracle ignores order") + + def test_the_ordered_oracle_is_order_sensitive(expect): + # And the ordered one must NOT: this comparison has to fail. + try: + expect.ordered_rows(FWD, REV, "ordered oracle sees order") + except AssertionError: + expect.num(1, 1, "the ordered oracle refused the reversed sequence") + else: + raise AssertionError("the ordered oracle did not detect the reversal") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "the two oracles behave differently", passed=2, failed=0) + + +def test_row_set_still_refuses_two_empty_sides(pytester, expect): + """The set oracle inherits rows()'s refusal rather than losing it.""" + pytester.makepyfile( + """ + def test_both_empty(expect): + expect.row_set([], [], "two empty sets") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "an empty set comparison is refused", failed=1, passed=0) diff --git a/test/pytest/test_runshape.py b/test/pytest/test_runshape.py new file mode 100644 index 00000000..2249da2d --- /dev/null +++ b/test/pytest/test_runshape.py @@ -0,0 +1,195 @@ +"""The three modes that turn a WHOLE RUN green rather than one test. + +Each is measured in VACUITY_MODES.md and each is worth more than the per-assertion +guards, because a single occurrence silently removes many tests at once. +""" + + +def test_layer_fails_when_a_collected_test_never_reports(pytester, expect): + """A crashed xdist worker loses its remaining tests and pytest says nothing. + + Measured with --max-worker-restart=0: 8 collected, summary "1 failed, 6 passed", + and one named test never reported. Counting collected tests cannot see it; only + reconciling the reported node-ids against the collected ones can. + """ + pytester.makepyfile( + """ + import os, signal + def test_a(expect): expect.num(1, 1, "a") + def test_b(expect): expect.num(1, 1, "b") + def test_kills_its_worker(expect): + os.kill(os.getpid(), signal.SIGKILL) + def test_d(expect): expect.num(1, 1, "d") + def test_e(expect): expect.num(1, 1, "e") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity", "-n", "2", + "--max-worker-restart=0") + expect.run_failed(result, "a lost test must fail the run") + result.stdout.fnmatch_lines(["*never reported*"]) + + +def test_layer_accepts_a_run_where_every_test_reports(pytester, expect): + """The control: reconciliation must not fire on an honest parallel run.""" + pytester.makepyfile( + """ + def test_a(expect): expect.num(1, 1, "a") + def test_b(expect): expect.num(1, 1, "b") + def test_c(expect): expect.num(1, 1, "c") + def test_d(expect): expect.num(1, 1, "d") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity", "-n", "2") + expect.outcomes(result, "an honest parallel run is untouched", passed=4, failed=0) + + +def test_layer_rejects_a_parametrize_over_an_empty_list(pytester, expect): + """A corpus glob that matches nothing becomes one 's' and exit 0. + + Measured: pytest generates a single SKIPPED placeholder for an empty argvalues + list. The suite reads as run and asserted nothing. + """ + pytester.makepyfile( + """ + import pytest + CORPUS = [] # a glob that matched nothing + @pytest.mark.parametrize("path", CORPUS) + def test_decode_roundtrip(path, expect): + expect.num(1, 1, "would run if the corpus were not empty") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "an empty parameter set must fail the run") + result.stderr.fnmatch_lines(["*empty parameter set*"]) + + +def test_layer_accepts_a_parametrize_with_cases(pytester, expect): + """The control: a real parameter set is untouched.""" + pytester.makepyfile( + """ + import pytest + @pytest.mark.parametrize("n", [1, 2, 3]) + def test_cases(n, expect): + expect.num(n, n, "a real case") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a real parameter set runs", passed=3, failed=0) + + +def test_layer_rejects_a_fixture_that_skips(pytester, expect): + """A session fixture calling pytest.skip() skips every dependent test. + + "The cluster would not start" becomes exit 0. This is the single largest blast + radius in the inventory: one skip removes an entire suite while it reads green. + """ + pytester.makepyfile( + conftest=""" + import pytest + @pytest.fixture(scope="session") + def cluster(): + pytest.skip("the cluster would not start") + """, + test_uses_cluster=""" + def test_one(cluster, expect): expect.num(1, 1, "one") + def test_two(cluster, expect): expect.num(2, 2, "two") + """, + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a fixture that skips must fail the run") + result.stdout.fnmatch_lines(["*skipped during setup*"]) + + +def test_layer_allows_a_declared_unrunnable_test(pytester, expect): + """The control and the escape hatch: expect.cannot_run stays available. + + A test that declares itself unrunnable with a reason from the closed list is the + supported way to not run, and it must not be caught by the fixture-skip guard. + """ + pytester.makepyfile( + """ + def test_declares_itself(expect): + expect.cannot_run("MISSING_DEPENDENCY", "no iceberg endpoint here") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a declared unrunnable test is allowed", passed=1, failed=0) + + +# --------------------------------------------------------------------------- +# DESELECTION IS NOT LOSS. +# +# The reconciliation above compares reported node-ids against collected ones, and +# `pytest_collection_modifyitems` fires BEFORE pytest's own -k and -m filtering has +# removed anything. So every deselected test looked like a test that vanished: +# +# $ pytest -q test_layer.py -k "refus" +# 1 passed, 15 deselected +# VACUITY: 15 collected test(s) never reported an outcome ... +# exit 1 +# +# A false red on a healthy run, from the guard whose subject is false greens. It is +# worse than a missed true red: the response to it is to stop using the plugin. +# --------------------------------------------------------------------------- + +def test_layer_allows_a_deliberately_selected_subset(pytester, expect): + """`-k` must not be reported as tests lost. + + Asking for a subset is a deliberate act by whoever typed the command; a test + lost to a crashed worker is not. `pytest_deselected` is where pytest tells the + two apart, so it is where the guard learns the difference. + """ + pytester.makepyfile( + """ + def test_alpha(expect): expect.num(1, 1, "alpha") + def test_beta(expect): expect.num(2, 2, "beta") + def test_gamma(expect): expect.num(3, 3, "gamma") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity", "-k", "alpha") + expect.outcomes(result, "a -k subset is a healthy run", passed=1, failed=0) + expect.num(result.ret, 0, "and it exits 0 rather than on the run-shape guard") + + +def test_layer_allows_an_explicitly_deselected_test(pytester, expect): + """--deselect reaches the same hook by a different route, so it is pinned too.""" + pytester.makepyfile( + """ + def test_alpha(expect): expect.num(1, 1, "alpha") + def test_beta(expect): expect.num(2, 2, "beta") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity", + "--deselect", "test_layer_allows_an_explicitly" + "_deselected_test.py::test_beta") + expect.outcomes(result, "an explicit deselection is a healthy run", + passed=1, failed=0, deselected=1) + expect.num(result.ret, 0, "and it exits 0") + + +def test_a_run_that_both_deselects_and_loses_a_test_still_fails(pytester, expect): + """The two must stay distinguishable, or the fix is just a blindfold. + + Subtracting the deselected ids from the collected set is the right fix only if + the set still holds every test that was genuinely lost. An over-broad + subtraction would pass this file's other arms and silently retire the guard, so + this arm deselects AND kills a worker in one run and requires the red. + """ + pytester.makepyfile( + """ + import os, signal + def test_skipme_alpha(expect): expect.num(1, 1, "deselected by -k") + def test_skipme_beta(expect): expect.num(2, 2, "deselected by -k") + def test_keep_a(expect): expect.num(1, 1, "a") + def test_keep_b(expect): expect.num(1, 1, "b") + def test_keep_kills_its_worker(expect): + os.kill(os.getpid(), signal.SIGKILL) + def test_keep_d(expect): expect.num(1, 1, "d") + def test_keep_e(expect): expect.num(1, 1, "e") + def test_keep_f(expect): expect.num(1, 1, "f") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity", "-k", "keep", "-n", "2", + "--max-worker-restart=0") + expect.run_failed(result, "a lost test is still caught when others were deselected") + result.stdout.fnmatch_lines(["*never reported*"]) diff --git a/test/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index 3329bbf8..a348b7e3 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -139,3 +139,192 @@ check "a stated total that disagrees with disk is visible" \ unset _dcv_dir _dcv_doc _dcv_seen _dcv_stated _dcv_fix unset -f _dcv_missing _dcv_count + +# ---- the mode inventory must count itself, and the count must be checkable ---- +# +# WHY THIS EXISTS. `test/pytest/VACUITY_MODES.md` and `test/pytest/README.md` +# stated different numbers for how many vacuity modes the layer refuses -- 27 and +# 23 -- and a reviewer could check NEITHER, because the document offered no rule +# for what counts as a mode. That is the defect this whole directory exists to +# refuse, committed by the document that describes the refusal: a number nobody +# can recompute is an assertion, not a measurement. +# +# Section 1a now states the rule: a mode is a backticked kebab-case identifier of +# three or more words. These arms hold the document to its own rule. +# +# WHAT IS POLICED. Four properties: +# * section 1a's "refused today" total is the count of ids in section 2 +# * its "not refused" total is the count in section 3 +# * its document total is the sum of the two +# * README.md quotes the same refused number, so the two cannot drift again +# The enumeration run's own 79 is history, not a property of the tree: nothing +# here pretends to check it. What IS checked is that the gap the document admits +# to equals the run total minus what was actually written down. + +_mi_doc="$PGC_TESTDIR/pytest/VACUITY_MODES.md" +_mi_readme="$PGC_TESTDIR/pytest/README.md" + +check "premise: the mode inventory is where this part thinks it is" \ + "$([ -f "$_mi_doc" ] && echo yes || echo no)" "yes" + +# Distinct ids matching section 1a's rule, within one "## " section. +# A function over FILE and PREFIX so the fixture arms below run the same logic. +_mi_ids() { # _mi_ids FILE PREFIX -> count + local f="$1" pre="$2" + awk -v p="^## $pre" ' + $0 ~ p { inside = 1; next } + /^## / { inside = 0 } + inside { print } + ' "$f" 2>/dev/null \ + | grep -oE '`[a-z0-9]+(-[a-z0-9]+){2,}`' \ + | sort -u | wc -l | tr -d ' ' +} + +# A stated row from the section 1a table. The label is a substring of the cell, +# not the whole of it, so this matches the cell rather than anchoring to its start. +# +# Take the VALUE cell, not the first number on the line. The labels themselves +# contain digits -- "named in section 2, refused today" -- so the obvious +# `grep -oE '[0-9]+' | head -1` returns the 2 from "section 2" and never the +# total. It did, and it read 2 and 3 for totals of 21 and 51. The fixture below +# could not see it because there the label digit and the value were both 2, so +# there is now an arm whose whole job is to tell those two readings apart. +_mi_row() { # _mi_row FILE LABEL -> the number, or "" if the row is absent + local f="$1" label="$2" + grep -E "^\|[^|]*${label}[^|]*\|" "$f" 2>/dev/null | head -1 \ + | awk -F'|' 'NF > 2 { v = $(NF - 1); gsub(/[^0-9]/, "", v); print v }' +} + +# Section 3 keeps a back-reference to every mode that moved into section 2 +# ("`X` is now closed"), so a mode can be named in both. Section 2 wins, and +# section 3's total is what it names MINUS what section 2 claims. Counting the +# back-references as unrefused puts one mode in two states: measured at 25 + 50 +# against 72 named, which is three ids counted twice. +_mi_idlist() { # _mi_idlist FILE PREFIX -> the ids themselves, one per line, sorted + local f="$1" pre="$2" + awk -v p="^## $pre" ' + $0 ~ p { inside = 1; next } + /^## / { inside = 0 } + inside { print } + ' "$f" 2>/dev/null \ + | grep -oE '`[a-z0-9]+(-[a-z0-9]+){2,}`' \ + | sort -u +} + +_mi_ref="$(_mi_ids "$_mi_doc" '2\.')" +_mi_not="$(comm -23 <(_mi_idlist "$_mi_doc" '3\.') <(_mi_idlist "$_mi_doc" '2\.') \ + | grep -c . || true)" + +# The same trap as the sweep above: a counter that finds nothing agrees with a +# document that claims nothing, and both look like success. +check "premise: the counting rule finds modes at all" \ + "$([ "$_mi_ref" -ge 15 ] && [ "$_mi_not" -ge 30 ] && echo enough || echo "$_mi_ref/$_mi_not")" \ + "enough" + +check "section 1a's refused total is the count of ids in section 2" \ + "$(_mi_row "$_mi_doc" 'refused today')" "$_mi_ref" + +check "section 1a's not-refused total is the count of ids in section 3" \ + "$(_mi_row "$_mi_doc" 'not refused')" "$_mi_not" + +check "section 1a's document total is the sum of its two sections" \ + "$(_mi_row "$_mi_doc" 'named in this document')" "$((_mi_ref + _mi_not))" + +check "the admitted gap is the run total minus what is written down" \ + "$(_mi_row "$_mi_doc" 'named nowhere here')" \ + "$(( $(_mi_row "$_mi_doc" 'produced by the enumeration run') \ + - $(_mi_row "$_mi_doc" 'named in this document') ))" + +# README.md is the file that drifted. It must quote the inventory's number rather +# than carry one of its own. +check "README.md quotes the number of modes the inventory names as refused" \ + "$(grep -cE "$_mi_ref refused" "$_mi_readme")" "1" + +# ---- and these arms must be able to FAIL ------------------------------------- + +_mi_fix="$PGC_WORKDIR/modecount"; rm -rf "$_mi_fix"; mkdir -p "$_mi_fix" +{ + printf '## 2. refused\n' + printf 'text `alpha-beta-gamma` and `delta-epsilon-zeta` here\n' + printf '## 3. not refused\n' + printf '`eta-theta-iota`\n' + printf '## 4. table\n' + printf '| named in section 2, refused today | 2 |\n' + printf '| named in section 3, not refused | 1 |\n' + printf '| **named in this document** | **3** |\n' +} > "$_mi_fix/GOOD.md" + +check "the counter counts a fixture's section 2" "$(_mi_ids "$_mi_fix/GOOD.md" '2\.')" "2" +check "the counter counts a fixture's section 3" "$(_mi_ids "$_mi_fix/GOOD.md" '3\.')" "1" + +# Two-word and one-word ids are not modes under section 1a's rule, and a counter +# that took them would inflate every total in the document. +printf '## 2. refused\n`one-two` and `single` and `alpha-beta-gamma`\n' > "$_mi_fix/SHORT.md" +check "an id of fewer than three words is not counted as a mode" \ + "$(_mi_ids "$_mi_fix/SHORT.md" '2\.')" "1" + +# An id named twice is one mode. Section 2 names several ids in more than one row. +printf '## 2. refused\n`alpha-beta-gamma` again `alpha-beta-gamma`\n' > "$_mi_fix/DUP.md" +check "an id named twice counts once" "$(_mi_ids "$_mi_fix/DUP.md" '2\.')" "1" + +# The section boundary must hold: ids after the next heading belong to it. +printf '## 2. refused\n`alpha-beta-gamma`\n## 3. not\n`delta-epsilon-zeta`\n' > "$_mi_fix/BOUND.md" +check "the counter stops at the next heading" "$(_mi_ids "$_mi_fix/BOUND.md" '2\.')" "1" + +# A wrong stated total is visible. This is the shape that shipped in two files. +printf '## 2. refused\n`alpha-beta-gamma`\n## 4. t\n| named in section 2, refused today | 9 |\n' \ + > "$_mi_fix/WRONG.md" +check "a stated total that disagrees with the ids is visible" \ + "$([ "$(_mi_row "$_mi_fix/WRONG.md" 'refused today')" = "$(_mi_ids "$_mi_fix/WRONG.md" '2\.')" ] \ + && echo agrees || echo differs)" "differs" + +check "and the same comparison agrees on the fixture that is right" \ + "$([ "$(_mi_row "$_mi_fix/GOOD.md" 'refused today')" = "$(_mi_ids "$_mi_fix/GOOD.md" '2\.')" ] \ + && echo agrees || echo differs)" "agrees" + +# A missing row must not read as a passing comparison. +printf '## 2. refused\n`alpha-beta-gamma`\n' > "$_mi_fix/NOROW.md" +# The label contains a digit and the value is a different digit, so a reader that +# takes the first number on the line and one that takes the value cell give +# different answers. This is the arm that would have caught the helper's own bug. +printf '## 4. t\n| named in section 2, refused today | 7 |\n' > "$_mi_fix/LABELDIGIT.md" +check "the row's value is read, not a digit inside its label" \ + "$(_mi_row "$_mi_fix/LABELDIGIT.md" 'refused today')" "7" + +check "an absent total is empty rather than a number that happens to match" \ + "$([ -z "$(_mi_row "$_mi_fix/NOROW.md" 'refused today')" ] && echo absent || echo present)" \ + "absent" + +# The table is not the only place a total lives. Three sentences outside it still +# asserted the run's 23 after the table said 21: section 2's opening, the closing +# paragraph, and TESTS.md. Gating the table alone just moves the drift into prose. +# +# The run's own 23 appears once on purpose, as history, and is not gated. What is +# gated is every sentence stating what the layer refuses TODAY. +_mi_prose() { # _mi_prose FILE REGEX -> the captured number, or "" + local f="$1" re="$2" + grep -oE "$re" "$f" 2>/dev/null | head -1 | grep -oE '[0-9]+' | head -1 +} + +_mi_tests="$PGC_TESTDIR/pytest/TESTS.md" + +check "section 2's opening states the counted number of refused modes" \ + "$(_mi_prose "$_mi_doc" '[0-9]+ of the 79, counted')" "$_mi_ref" + +check "the closing paragraph states the counted number too" \ + "$(_mi_prose "$_mi_doc" 'known to refuse [0-9]+ demonstrated modes')" "$_mi_ref" + +check "TESTS.md states the counted number as well" \ + "$(_mi_prose "$_mi_tests" 'This layer refuses [0-9]+')" "$_mi_ref" + +# And the prose reader must be able to fail, on a fixture rather than on the tree. +printf 'the layer is known to refuse 99 demonstrated modes here\n' > "$_mi_fix/PROSE.md" +check "a prose total that disagrees with the ids is visible" \ + "$(_mi_prose "$_mi_fix/PROSE.md" 'known to refuse [0-9]+ demonstrated modes')" "99" + +check "an absent prose total is empty rather than a stray number" \ + "$([ -z "$(_mi_prose "$_mi_fix/GOOD.md" 'known to refuse [0-9]+ demonstrated modes')" ] \ + && echo absent || echo present)" "absent" + +unset _mi_doc _mi_readme _mi_ref _mi_not _mi_fix _mi_tests +unset -f _mi_ids _mi_row _mi_prose