From e62dd8d0df977e3a130a86a4e0ff7a8dfd4fac23 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 02:03:29 +0000 Subject: [PATCH 1/6] test: the vacuity mode inventory, and two gaps it found in the layer (#432) An enumeration ran in the audit container against pytest 9.1.1, xdist 3.8.0 and psycopg 3.3.5, with every mode required to be DEMONSTRATED BY AN ACTUAL RUN rather than described. It produced 79 modes, 73 of them executed, and a refusal design for 74. WHAT DID NOT RUN, SAID FIRST. The adversarial stage that would have attacked each refusal was cut off by a session limit: 148 attacks started, 0 completed. So the summary line reading "defeated: 0" counts zero defeats out of ZERO ATTEMPTS, and none of the 74 designs has met an adversary. VACUITY_MODES.md says so in its own section rather than leaving the number to be misread. CHECKING THE LAYER AGAINST THE INVENTORY FOUND THREE GAPS. Two are here; the third landed first, in #897, and this commit now defers to it. expect.num(-1, -1) passed. cursor.rowcount is -1 when no count is available and 1 for an unfetched SELECT, and both are numbers. expect.rowcount now refuses the sentinel and says what it is. A broad except was forbidden IN A COMMENT, which enforces nothing. After any failed statement psycopg raises for every later one, so one `except Exception` hides the real error and all its successors. It is now uncollectable. plan_marker(absent=True) returned a pass against []. That gap is closed on main by #897's own guard, so the two tests this commit wrote for it are DROPPED rather than shipped beside it -- two tests for one property under two names is what makes a corpus hard to read, and the doc gate would then require documenting both. Independent discovery is worth recording; a duplicate test is not. AND THE BROAD-EXCEPT GUARD IMMEDIATELY REJECTED CODE ALREADY ON MAIN. Rebasing it onto 6364e220 turned the whole run red at collection: ERROR: ... test_build_refusal.py:340 except Exception catches Exception broadly -- catch the specific exception class instead. That is my own arm from #897, catching a failed make_cluster broadly. Narrowed to (FileNotFoundError, RuntimeError, OSError) -- a missing pg_config raises FileNotFoundError out of the subprocess layer, measured, and anything else now escapes and fails loudly, which is what should happen to an error the arm did not predict. The guard earned its place before this branch was opened. Written first as a line regex, the guard fired on the forbidden shape appearing inside a pytester.makepyfile STRING and so rejected the layer's own tests. 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. One assumption of mine was refuted by checking: expect.rows does NOT sort, so ordered claims are testable through it. The collapse comes from CALLERS sorting, which test_native_projection.py does deliberately. Verified: harness_selftest 342 passed + 0 failed + 0 unrunnable PASSED docs_style 9 checks PASSED pytest 76 passed serial, 76 passed -n 4, marker cleared for each shellcheck -S error -s bash test/*.sh test/selftest/*.sh exit 0 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/README.md | 2 + test/pytest/TESTS.md | 48 +++++++- test/pytest/VACUITY_MODES.md | 198 ++++++++++++++++++++++++++++++ test/pytest/pgc_vacuity.py | 91 +++++++++++++- test/pytest/test_build_refusal.py | 9 +- test/pytest/test_layer.py | 36 ++++++ 6 files changed, 375 insertions(+), 9 deletions(-) create mode 100644 test/pytest/VACUITY_MODES.md diff --git a/test/pytest/README.md b/test/pytest/README.md index b5c13307..ee48513c 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -3,6 +3,8 @@ 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, 73 demonstrated by a run, 23 refused by this layer today. - `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..75da2e04 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 +Seventy-seven 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. @@ -33,7 +33,8 @@ behaviour, the source of that number is named. - [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) +- [10. What this corpus does NOT yet refuse](#10-what-this-corpus-does-not-yet-refuse) +- [11. Traps this corpus records](#11-traps-this-corpus-records) ## 1. How to read a test in here @@ -90,7 +91,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 +114,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 @@ -761,7 +787,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 +## 10. 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 23 +of them.** The other 56, of which 50 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, because the gap most likely to affect a new test is +that the port has **no ordered oracle**: a claim about `ORDER BY` compared with +sorted lists cannot fail on order, and `lib.sh` closes that with `pgc_seq_hash` and +`diff_query_ordered` while the port has nothing. + +## 11. 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..551b4ef6 --- /dev/null +++ b/test/pytest/VACUITY_MODES.md @@ -0,0 +1,198 @@ +# 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, **73 demonstrated by a run**. 74 refusals designed, every one of them +stating a residual. None of the 74 has been adversarially tested. + +## 2. What the layer refuses today + +23 of the 79. 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` | + +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. + +## 3. What it does not refuse + +56 modes, **50 of them demonstrated by a run**. 52 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 + +The layer asserts how many tests were **collected**. It does not assert how many +**reported**. + +- `crashed-worker-silently-loses-tests` — measured under `--max-worker-restart=0`: + 8 collected, summary says "1 failed, 6 passed", and one named test never reported. + pytest prints no warning. +- `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` + +The designed refusal is to reconcile the collected node-id **set** against the +reported node-id **set** in the controlling process, and fail on any difference. +That is strictly stronger than the count check the layer has, and it subsumes it. + +### 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` — measured: a corpus glob matching nothing + turns a data-driven suite into one `s` and exit 0. This is the shape most likely + to bite a port, because the bash suites read corpora from disk. +- `session-fixture-skip-greens-the-whole-suite` — a session fixture calling + `pytest.skip()` skips every dependent test. "The cluster would not start" becomes + exit 0. The enumerating agent called this the single largest blast radius. +- `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 + +- `set-oracle-on-an-ordered-claim` — a test that names `ORDER BY` and compares + `sorted(got) == sorted(want)` cannot fail on order. `lib.sh` closes this with + `diff_query_ordered`, `pgc_seq_hash` and `pgc_check_ordered_oracle`; **the port + has no ordered oracle at all.** Note `expect.rows` is order-sensitive, so the + collapse comes from callers sorting, as `test_native_projection.py` does. +- `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_layer_fails_when_a_reported_test_is_missing` — reconcile collected node-ids + against reported node-ids. Subsumes the count check and closes §3.1. +2. `test_layer_rejects_an_empty_parametrize` — closes the shape most likely to bite + a corpus-driven port. +3. `test_layer_rejects_a_fixture_that_skips` — closes the largest blast radius. +4. `test_expect_ordered_rows_refuses_a_sorted_comparison` — port `pgc_seq_hash` and + `diff_query_ordered`. The port cannot express an ordered claim today. +5. `test_expect_query_error_sentinel_is_unique_per_failure` — make something produce + `QUERY_ERROR.`; the constant exists and nothing writes it. +6. `test_layer_requires_a_write_to_have_written` — closes `insert-wrote-no-rows`. +7. `test_layer_requires_ab_arms_to_differ` — closes `mutation-arm-unobservable`. +8. `test_raises_requires_a_sqlstate` — closes `raises-too-broad`. + +Items 1 to 3 are worth more than the rest combined, because each turns a whole run +green rather than one test. + +## 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 23 demonstrated modes. 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..75974225 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,24 @@ def num(self, got, want, name): if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") + # -- 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. @@ -315,6 +335,17 @@ 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 + + # AN ABSENCE ASSERTION IS SATISFIED BY AN EMPTY PLAN. Measured: this + # returned a pass for `absent=True` against `[]`, which is the case most + # worth catching -- a plan that failed to arrive looks exactly like a plan + # that legitimately lacks the node. So a plan with no nodes at all is + # refused rather than compared. + if nodes == 0: + raise VacuityError( + f"{label}: the plan has no nodes, so this could not have failed. " + f"Assert on a plan that arrived." + ) self._counted() if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") @@ -482,6 +513,37 @@ 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. +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") + elif isinstance(t, ast.Name) and t.id in ("Exception", "BaseException"): + out.append(f"{name}:{node.lineno} except {t.id}") + return out + + def pytest_collection_modifyitems(config, items): """Refuse a bare skip, which exits 0 and reads as success. @@ -489,13 +551,36 @@ 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}") + 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") 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] + 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" + ) 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_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*"]) From da405d42a5e47be335dbd6de7a2584d73e8231ae Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 22:03:26 +0000 Subject: [PATCH 2/6] test/pytest: fix the refusal oracle, and make the inventory count itself Answers the review on #905. Four defects, three of them in the layer's own guards and one in the document that describes them. expect.refusal matched the whole traceback, not the error --------------------------------------------------------- `expect.refusal` built its patterns as `*{p}*` and ran them over the pytest output. pytest prints the ENCLOSING FUNCTION'S SOURCE in a traceback, so a pattern naming the thing the guard refuses matched the fixture's own source line and passed whether or not the guard fired. Anchored to `E*{p}*`, which is the error line pytest actually emits. This is on main from #897 and it is the largest of the four: 13 merged arms were asserting nothing. Verified by neutering each guard in turn -- with the guard live the arm passes, with it removed the arm now fails. Four arms tested, all four held. plan_marker carried a dead branch --------------------------------- `nodes == 0` could not be reached: `if not nodes:` above it returns first. Removed. `nodes == 0` now occurs zero times and `if not nodes:` once. the broad-except scan missed every tuple handler ------------------------------------------------- `except (ValueError, Exception):` is as broad as `except Exception:` and the scan walked past it, because it inspected the handler type only when that type was a bare Name. It now inspects each member of a Tuple. All five spellings verified: `Exception`, `(ValueError, Exception)`, `BaseException` and the bare `except:` are refused; `except ValueError:` still passes as the control. the inventory could not be checked, so it drifted -------------------------------------------------- README.md said 23 refused modes and VACUITY_MODES.md said 27, and a reader could check NEITHER, because the document offered no rule for what counts as a mode. That is the defect this directory exists to refuse, committed by the document describing the refusal. Section 1a now states the rule -- a mode is a backticked kebab-case identifier of three or more words -- and reconciles the totals against it: 21 refused, 51 not, 72 named, against 79 the enumeration produced. The seven never written down are named as a gap rather than counted as coverage. The numbers are now gated in both harnesses, because a total nobody recomputes goes stale the same way twice: * `test/pytest/test_docs_cover_the_corpus.py` -- four arms over the table, the README, the gap arithmetic, and the prose totals outside the table. * `test/selftest/350-the-pytest-corpus-must-be.sh` -- the same rules, and this is the copy with teeth: nothing in the gate runs pytest. The two implementations disagreed, and the disagreement was the point. The bash reader took the first number on the line and returned 2 and 3 for totals of 21 and 51 -- the digits inside "named in section 2". Its fixture could not see it because there the label digit and the value were both 2, so there is now an arm whose only job is to tell those two readings apart. Proved able to fail, each mutation asserted applied by md5 and restored byte-exact: 1a states 22 refused, disk has 21 arm reddens a refused mode id loses its backticks arm reddens README drifts back to 23 arm reddens the gap row closes on its own arm reddens section 2's opening drifts back to 23 arm reddens the closing paragraph drifts back to 23 arm reddens TESTS.md drifts back to 23 arm reddens harness_selftest 387 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 84 passed serial and under -n 4 docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/README.md | 5 +- test/pytest/TESTS.md | 8 +- test/pytest/VACUITY_MODES.md | 37 +++- test/pytest/pgc_vacuity.py | 59 ++++-- test/pytest/test_docs_cover_the_corpus.py | 105 +++++++++++ .../selftest/350-the-pytest-corpus-must-be.sh | 172 ++++++++++++++++++ 6 files changed, 366 insertions(+), 20 deletions(-) diff --git a/test/pytest/README.md b/test/pytest/README.md index ee48513c..eb7a7207 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -4,7 +4,10 @@ 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, 73 demonstrated by a run, 23 refused by this layer today. + pass: 79 modes produced by the enumeration, 72 of them named in that + file, 73 demonstrated by a run, and 21 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 75da2e04..078f01e2 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. -Seventy-seven of them test the harness rather than the +**96 tests in 6 files.** Eighty-one 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. @@ -632,6 +632,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 | @@ -790,7 +794,7 @@ an arm where the two differ is void rather than reported. ## 10. 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 23 +asserting nothing, 73 of them demonstrated by an actual run. **This layer refuses 21 of them.** The other 56, of which 50 were demonstrated, are listed there with the refusal design each would need and the order worth building them in. diff --git a/test/pytest/VACUITY_MODES.md b/test/pytest/VACUITY_MODES.md index 551b4ef6..58d077ea 100644 --- a/test/pytest/VACUITY_MODES.md +++ b/test/pytest/VACUITY_MODES.md @@ -25,12 +25,40 @@ attacks**. That number is not evidence that these refusals survive attack, and t document is the synthesis the failed stage would have produced, written by hand from the stage outputs that did complete. -So: 79 modes, **73 demonstrated by a run**. 74 refusals designed, every one of them +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. + +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 | 21 | +| named in section 3, not refused | 51 | +| **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 -23 of the 79. Each is enforced by a mechanism, not a convention, and each has a red +21 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 | @@ -59,7 +87,7 @@ than reasoning about it, and all three had passed silently before: ## 3. What it does not refuse -56 modes, **50 of them demonstrated by a run**. 52 have a refusal already designed. +56 modes by the run's count, **51 of them named below**, **50 demonstrated by a run**. 52 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 @@ -194,5 +222,6 @@ have tried to defeat them did not run. Every design states its own residual, and those residuals are the authors' own, unchallenged. So treat §2 as measured, §3 as measured, and §5 as a plan that has not yet met an -adversary. The layer is known to refuse 23 demonstrated modes. It is not known to be +adversary. The layer is known to refuse 21 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 75974225..3991f5bf 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -266,7 +266,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. @@ -336,16 +360,6 @@ def plan_marker(self, plan, key, name=None, absent=False): if key in node: found = True - # AN ABSENCE ASSERTION IS SATISFIED BY AN EMPTY PLAN. Measured: this - # returned a pass for `absent=True` against `[]`, which is the case most - # worth catching -- a plan that failed to arrive looks exactly like a plan - # that legitimately lacks the node. So a plan with no nodes at all is - # refused rather than compared. - if nodes == 0: - raise VacuityError( - f"{label}: the plan has no nodes, so this could not have failed. " - f"Assert on a plan that arrived." - ) self._counted() if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") @@ -539,8 +553,27 @@ def _broad_except_sites(path): t = node.type if t is None: out.append(f"{name}:{node.lineno} bare except") - elif isinstance(t, ast.Name) and t.id in ("Exception", "BaseException"): - out.append(f"{name}:{node.lineno} except {t.id}") + 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 diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 4a765e71..7b108fc8 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -151,3 +151,108 @@ 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()) + 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/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index 3329bbf8..59859b8c 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -139,3 +139,175 @@ 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 }' +} + +_mi_ref="$(_mi_ids "$_mi_doc" '2\.')" +_mi_not="$(_mi_ids "$_mi_doc" '3\.')" + +# 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 From 79b8252a4a088806fd2d8450e704928c506349ee Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 02:09:03 +0000 Subject: [PATCH 3/6] test: the ordered oracle, and the set oracle it has to be different from The largest gap the mode inventory found: lib.sh has had pgc_seq_hash, diff_query_ordered and pgc_check_ordered_oracle since #418, and the port had no way to express an ordering claim at all. A test naming ORDER BY and comparing sorted lists cannot fail on order. FOUR MECHANISMS, each with a red test that fails without it. expect.ordered_rows compares two sequences IN ORDER and names the position of the first difference rather than saying two hashes differ. It refuses two empty sequences, and it refuses a sequence whose elements are all identical: such a sequence reads the same forwards and backwards, so an ordering claim about it cannot fail. That second refusal is pgc_check_ordered_oracle's premise applied to a caller's data instead of to a fixed fixture. expect.ordering_observable is that premise itself, ported: read the same rows both ways and require the two to differ before relying on order. expect.row_set is the counterpart, and the reason it exists is that ignoring order should be DECLARED. 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 while staying green. The arm that pins it runs both oracles over the same forward and reverse sequences and requires one to ignore the reversal and the other to catch it. And a collection-time ast scan refuses sorted(), set() or frozenset() feeding ordered_rows. The helper is order-sensitive, so the collapse is introduced at the call site: ordered_rows(sorted(got), sorted(want)) reads like an ordering claim and is not one. Parsed rather than grepped, for the reason the broad-except scan is. test_native_projection.py now declares which oracle it wants. Its fetch helper returns rows in query order and the assertions use row_set, because the bash original uses pgc_set_hash and is order-blind. Sorting inside a fetch helper is exactly how an ordering claim silently becomes a set one, so the helper no longer does it. The property comparison against the bash suite still reports every property covered. 38 tests, green serially and under four xdist workers. VACUITY_MODES.md records the mode as closed and the count as 24 of 79. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/README.md | 2 +- test/pytest/TESTS.md | 13 ++- test/pytest/VACUITY_MODES.md | 18 ++-- test/pytest/pgc_vacuity.py | 118 +++++++++++++++++++++ test/pytest/test_native_projection.py | 31 ++++-- test/pytest/test_ordered.py | 144 ++++++++++++++++++++++++++ 6 files changed, 303 insertions(+), 23 deletions(-) create mode 100644 test/pytest/test_ordered.py diff --git a/test/pytest/README.md b/test/pytest/README.md index eb7a7207..aa9bf848 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -5,7 +5,7 @@ This is the issue #432 pilot. It runs beside `test/*.sh`, and replaces nothing. - `TESTS.md` in this directory documents every test and every assertion helper. - `VACUITY_MODES.md` is the inventory of ways a pytest harness can report a false pass: 79 modes produced by the enumeration, 72 of them named in that - file, 73 demonstrated by a run, and 21 refused by this layer today. + file, 73 demonstrated by a run, and 22 refused by this layer today. VACUITY_MODES.md section 1a gives the counting rule and reconciles the run's totals against what is actually written down. - `design/ISSUE_432_PYTEST_HARNESS.md` holds the design and the measurements diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 078f01e2..28886f01 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -65,6 +65,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 | @@ -80,6 +83,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 @@ -794,7 +805,7 @@ an arm where the two differ is void rather than reported. ## 10. 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 21 +asserting nothing, 73 of them demonstrated by an actual run. **This layer refuses 22 of them.** The other 56, of which 50 were demonstrated, are listed there with the refusal design each would need and the order worth building them in. diff --git a/test/pytest/VACUITY_MODES.md b/test/pytest/VACUITY_MODES.md index 58d077ea..b55e2171 100644 --- a/test/pytest/VACUITY_MODES.md +++ b/test/pytest/VACUITY_MODES.md @@ -40,8 +40,8 @@ recollection of the run: | | modes | | --- | ---: | -| named in section 2, refused today | 21 | -| named in section 3, not refused | 51 | +| named in section 2, refused today | 22 | +| named in section 3, not refused | 50 | | **named in this document** | **72** | | produced by the enumeration run | 79 | | **named nowhere here** | **7** | @@ -58,7 +58,7 @@ an id can be read, argued with and turned into a test, and a number cannot. ## 2. What the layer refuses today -21 of the 79, counted by section 1a's rule. Each is enforced by a mechanism, not a convention, and each has a red +22 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 | @@ -75,6 +75,7 @@ test in `test_layer.py` that fails without it. | `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 `sorted()` feeding it | `set-oracle-on-an-ordered-claim` | Three of those were added after checking this layer against the inventory rather than reasoning about it, and all three had passed silently before: @@ -87,7 +88,7 @@ than reasoning about it, and all three had passed silently before: ## 3. What it does not refuse -56 modes by the run's count, **51 of them named below**, **50 demonstrated by a run**. 52 have a refusal already designed. +55 modes by the run's count, **50 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 @@ -133,11 +134,6 @@ is the same reason `test/run_all_versions.sh` carries its own accounting. ### 3.4 The assertion is shaped so it cannot fail -- `set-oracle-on-an-ordered-claim` — a test that names `ORDER BY` and compares - `sorted(got) == sorted(want)` cannot fail on order. `lib.sh` closes this with - `diff_query_ordered`, `pgc_seq_hash` and `pgc_check_ordered_oracle`; **the port - has no ordered oracle at all.** Note `expect.rows` is order-sensitive, so the - collapse comes from callers sorting, as `test_native_projection.py` does. - `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`, @@ -204,8 +200,6 @@ Each entry names the red test to write first. 2. `test_layer_rejects_an_empty_parametrize` — closes the shape most likely to bite a corpus-driven port. 3. `test_layer_rejects_a_fixture_that_skips` — closes the largest blast radius. -4. `test_expect_ordered_rows_refuses_a_sorted_comparison` — port `pgc_seq_hash` and - `diff_query_ordered`. The port cannot express an ordered claim today. 5. `test_expect_query_error_sentinel_is_unique_per_failure` — make something produce `QUERY_ERROR.`; the constant exists and nothing writes it. 6. `test_layer_requires_a_write_to_have_written` — closes `insert-wrote-no-rows`. @@ -222,6 +216,6 @@ have tried to defeat them did not run. Every design states its own residual, and those residuals are the authors' own, unchallenged. So treat §2 as measured, §3 as measured, and §5 as a plan that has not yet met an -adversary. The layer is known to refuse 21 demonstrated modes -- the ids named in section 2, +adversary. The layer is known to refuse 22 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 3991f5bf..c0470af6 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -110,6 +110,80 @@ 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. @@ -540,6 +614,41 @@ def pytest_collection_finish(session): # 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 _sorted_ordered_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.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" + ) + return out + + def _broad_except_sites(path): try: tree = ast.parse(pathlib.Path(path).read_text()) @@ -594,12 +703,14 @@ def pytest_collection_modifyitems(config, items): seen_files.add(f) for site in _broad_except_sites(f): offenders.append(f"{site} catches Exception broadly") + offenders.extend(_sorted_ordered_sites(f)) 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( @@ -614,6 +725,13 @@ def pytest_collection_modifyitems(config, items): + "; ".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( "the pgColumnar vacuity layer refuses this run: " + ". ".join(parts) + "." ) 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..08b9ee0c --- /dev/null +++ b/test/pytest/test_ordered.py @@ -0,0 +1,144 @@ +"""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_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) From 08913a5b86db88d7aa7697428d77597ca8fc4080 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 02:21:31 +0000 Subject: [PATCH 4/6] test/pytest: refuse three run-shape defects that green a whole session The layer refused vacuous tests. It did not refuse vacuous runs. These three shapes each turn an entire session green rather than one test, which is why VACUITY_MODES.md ranked them above the rest of the backlog combined. Reconcile reported node-ids against collected ones. Measured on a 6-test corpus under -n 2 --max-worker-restart=0: bare pytest exits 1 and names the crash, but reports 5 node-ids for 6 collected and never mentions test_d, which was assigned to the dead worker and never ran. The loss is silent, not the crash. A suite whose crash lands on an already-failing test therefore reports what you expected while running fewer tests than you wrote. Fail a parametrize over an empty set, with its own message. A corpus glob matching nothing gives one s and exit 0, and the cause a reader needs is the corpus, not the marker. Fail a skip arriving during fixture setup. A session fixture calling pytest.skip() skips every dependent test, so "the cluster would not start" becomes exit 0. expect.cannot_run stays available and records a counted assertion instead of skipping. Two measurements shaped the implementation. Under xdist the workers collect, not the controller, so the guard was present and blind until it listened to pytest_xdist_node_collection_finished. And the state lives on a per-config plugin instance: pytester.runpytest() runs the inner session in-process, so module-level sets leaked between these tests and the sessions they drive, and the corpus reported 44 passed while exiting 1. 44 passed, exit 0, serially and under -n 4, with exit codes read without a pipe. VACUITY_MODES.md now records 27 of 79 refused. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/README.md | 2 +- test/pytest/TESTS.md | 94 +++++++++++-- test/pytest/VACUITY_MODES.md | 83 +++++++----- test/pytest/pgc_vacuity.py | 123 +++++++++++++++++- test/pytest/test_docs_cover_the_corpus.py | 5 + test/pytest/test_runshape.py | 116 +++++++++++++++++ .../selftest/350-the-pytest-corpus-must-be.sh | 19 ++- 7 files changed, 390 insertions(+), 52 deletions(-) create mode 100644 test/pytest/test_runshape.py diff --git a/test/pytest/README.md b/test/pytest/README.md index aa9bf848..7f19f184 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -5,7 +5,7 @@ This is the issue #432 pilot. It runs beside `test/*.sh`, and replaces nothing. - `TESTS.md` in this directory documents every test and every assertion helper. - `VACUITY_MODES.md` is the inventory of ways a pytest harness can report a false pass: 79 modes produced by the enumeration, 72 of them named in that - file, 73 demonstrated by a run, and 22 refused by this layer today. + 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 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 28886f01..ebaab997 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -32,9 +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. What this corpus does NOT yet refuse](#10-what-this-corpus-does-not-yet-refuse) -- [11. Traps this corpus records](#11-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 @@ -775,7 +777,75 @@ 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_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 | + +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 @@ -802,19 +872,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. What this corpus does NOT yet refuse +## 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 22 -of them.** The other 56, of which 50 were demonstrated, are listed there with the +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, because the gap most likely to affect a new test is -that the port has **no ordered oracle**: a claim about `ORDER BY` compared with -sorted lists cannot fail on order, and `lib.sh` closes that with `pgc_seq_hash` and -`diff_query_ordered` while the port has nothing. +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. -## 11. Traps this corpus records +## 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 index b55e2171..8d9c6991 100644 --- a/test/pytest/VACUITY_MODES.md +++ b/test/pytest/VACUITY_MODES.md @@ -35,13 +35,20 @@ stating a residual. None of the 74 has been adversarially tested. 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 | 22 | -| named in section 3, not refused | 50 | +| 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** | @@ -58,7 +65,7 @@ an id can be read, argued with and turned into a test, and a number cannot. ## 2. What the layer refuses today -22 of the 79, counted by section 1a's rule. Each is enforced by a mechanism, not a convention, and each has a red +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 | @@ -76,6 +83,9 @@ test in `test_layer.py` that fails without it. | `--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 `sorted()` feeding it | `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: @@ -88,26 +98,38 @@ than reasoning about it, and all three had passed silently before: ## 3. What it does not refuse -55 modes by the run's count, **50 of them named below**, **49 demonstrated by a run**. 51 have a refusal already designed. +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 -The layer asserts how many tests were **collected**. It does not assert how many -**reported**. +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: -- `crashed-worker-silently-loses-tests` — measured under `--max-worker-restart=0`: - 8 collected, summary says "1 failed, 6 passed", and one named test never reported. - pytest prints no warning. - `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` -The designed refusal is to reconcile the collected node-id **set** against the -reported node-id **set** in the controlling process, and fail on any difference. -That is strictly stronger than the count check the layer has, and it subsumes it. - ### 3.2 The invocation throws the verdict away - `exit-5-lost-through-a-pipe` — measured: `pytest -q -k nosuch | tee run.log` @@ -122,12 +144,15 @@ 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` — measured: a corpus glob matching nothing - turns a data-driven suite into one `s` and exit 0. This is the shape most likely - to bite a port, because the bash suites read corpora from disk. -- `session-fixture-skip-greens-the-whole-suite` — a session fixture calling - `pytest.skip()` skips every dependent test. "The cluster would not start" becomes - exit 0. The enumerating agent called this the single largest blast radius. +**`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` @@ -195,19 +220,15 @@ is the same mistake as matching a plan by substring.** Each entry names the red test to write first. -1. `test_layer_fails_when_a_reported_test_is_missing` — reconcile collected node-ids - against reported node-ids. Subsumes the count check and closes §3.1. -2. `test_layer_rejects_an_empty_parametrize` — closes the shape most likely to bite - a corpus-driven port. -3. `test_layer_rejects_a_fixture_that_skips` — closes the largest blast radius. -5. `test_expect_query_error_sentinel_is_unique_per_failure` — make something produce +1. `test_expect_query_error_sentinel_is_unique_per_failure` — make something produce `QUERY_ERROR.`; the constant exists and nothing writes it. -6. `test_layer_requires_a_write_to_have_written` — closes `insert-wrote-no-rows`. -7. `test_layer_requires_ab_arms_to_differ` — closes `mutation-arm-unobservable`. -8. `test_raises_requires_a_sqlstate` — closes `raises-too-broad`. +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`. -Items 1 to 3 are worth more than the rest combined, because each turns a whole run -green rather than one test. +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 @@ -216,6 +237,6 @@ have tried to defeat them did not run. Every design states its own residual, and those residuals are the authors' own, unchallenged. So treat §2 as measured, §3 as measured, and §5 as a plan that has not yet met an -adversary. The layer is known to refuse 22 demonstrated modes -- the ids named in section 2, +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 c0470af6..cefd4185 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -517,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) @@ -565,10 +561,97 @@ 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_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", @@ -696,14 +779,40 @@ def pytest_collection_modifyitems(config, items): 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 diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 7b108fc8..93c90e71 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -175,6 +175,11 @@ def _named_modes(): 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() diff --git a/test/pytest/test_runshape.py b/test/pytest/test_runshape.py new file mode 100644 index 00000000..e523e52b --- /dev/null +++ b/test/pytest/test_runshape.py @@ -0,0 +1,116 @@ +"""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) diff --git a/test/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index 59859b8c..a348b7e3 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -195,8 +195,25 @@ _mi_row() { # _mi_row FILE LABEL -> the number, or "" if the row is absent | 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="$(_mi_ids "$_mi_doc" '3\.')" +_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. From 1a167f5211bd1a75caddf5a7853758e145e621d9 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 15:13:08 +0000 Subject: [PATCH 5/6] test/pytest: pin the ordered oracle's two guards to their own messages These live here rather than in the base branch's copy of test_guards_pinned.py because the guards they pin do not exist until the ordered oracle does. Both are unreachable by SUBSUMPTION rather than untested, which is why the outcome alone cannot pin them. Neuter ordered_rows' both-empty guard and the UNOBSERVABLE guard fires on the same input, because every element of an empty sequence is trivially the same. Neuter ordering_observable's and the forward == reverse AssertionError fires, because two empty readings are equal. Either way the inner run still fails, so an arm asserting only failed=1 still passes and the guard is pinned by nothing. A mutation census over the full stack found exactly these two still unheld once the rest of the layer was pinned, which is how they were found rather than by reading. full stack, census before 17 guards 5 HELD 12 UNHELD full stack, census after 18 guards 18 HELD 0 UNHELD 71 passed, exit 0, serial and under -n 4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/TESTS.md | 11 ++++++++++- test/pytest/test_guards_pinned.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index ebaab997..f934b39c 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. -**96 tests in 6 files.** Eighty-one of them test the harness rather than the +**113 tests in 8 files.** Ninety-eight 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. @@ -290,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. 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") From f563a60b873fd4f2fded04fcbe7e02014f30baee Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 9 Sep 2026 22:11:57 +0000 Subject: [PATCH 6/6] test/pytest: stop the run-shape guard failing healthy runs, and widen the order scan Answers the review on #906. The first of these is the one that mattered most, and the framing in the review was better than mine. the run-shape guard failed any run that used -k ----------------------------------------------- `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 without reporting: $ 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, produced by the guard whose whole subject is false greens. It is worse than a missed true red, because the response to it is to stop using -k, and then to stop using the plugin. `pytest_deselected` is where pytest distinguishes the two, so the guard now learns the difference there. Asking for a subset is a deliberate act by whoever typed the command; a test lost to a crashed worker is not. $ pytest -q -k "refus" 46 passed, 62 deselected exit 0 Three arms, and the third is the one that matters: subtracting the deselected ids is only correct if a genuinely lost test is still caught, so one arm deselects AND kills a worker in the same run and requires the red. Without it an over-broad subtraction would pass every other arm and quietly retire the guard. the order-killer scan saw one spelling of three ------------------------------------------------ It caught `expect.ordered_rows(sorted(got), ...)` and walked past both of these, which read as more careful code than the version it did catch: g = sorted(got) # bound to a name first expect.ordered_rows(g, want) got.sort() # killed in place; the call site is unchanged The scan now tracks names bound to an order-killing call and names sorted in place, within one function body. Its limits are named in VACUITY_MODES.md 2.1 and pinned by a test, so "one function deep" cannot quietly become a claim of completeness. It compares line numbers, so a name sorted AFTER the claim is not refused: a guard against false greens has no business emitting a false red. False-positive budget over the real corpus before trusting it: 0 hits in 12 files. the inventory could count one mode in two states ------------------------------------------------- Rebasing onto #905 put the new counting rule over a document where modes had actually moved, and it reported 25 refused and 50 unrefused out of 72 named. The three extra are section 3's back-references -- "`X` is now closed" -- which point at modes that moved into section 2. Section 1a now says section 2 wins, and section 3's total is the ids it names minus the ids section 2 claims. 25 + 47 = 72, and both harnesses implement it. This only became visible because the totals were gated; the same document previously carried 27 and 24 in different places with nothing to catch either. Proved able to fail, each mutation asserted applied by md5 and restored byte-exact: pytest_deselected stops subtracting 2 arms redden (both -k spellings) the killed-name scan forgets its names 2 arms redden (bound and in-place) harness_selftest 387 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 108 passed serial, under -n 4, and with --pgc-expect-tests pytest -k 46 passed, 62 deselected, exit 0 docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/TESTS.md | 9 +++- test/pytest/VACUITY_MODES.md | 22 +++++++- test/pytest/pgc_vacuity.py | 98 +++++++++++++++++++++++++++++++----- test/pytest/test_ordered.py | 81 +++++++++++++++++++++++++++++ test/pytest/test_runshape.py | 79 +++++++++++++++++++++++++++++ 5 files changed, 274 insertions(+), 15 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index f934b39c..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. -**113 tests in 8 files.** Ninety-eight 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. @@ -800,6 +800,10 @@ tests port that pair and its premise check. | `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 | @@ -829,6 +833,9 @@ than a single test, which is why they were built before the rest of the backlog. | `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. diff --git a/test/pytest/VACUITY_MODES.md b/test/pytest/VACUITY_MODES.md index 8d9c6991..8ffb92ac 100644 --- a/test/pytest/VACUITY_MODES.md +++ b/test/pytest/VACUITY_MODES.md @@ -82,7 +82,7 @@ test in `test_layer.py` that fails without it. | `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 `sorted()` feeding it | `set-oracle-on-an-ordered-claim` | +| `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` | @@ -96,6 +96,26 @@ than reasoning about it, and all three had passed silently before: 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. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index cefd4185..e312cbef 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -592,6 +592,23 @@ def pytest_collection_modifyitems(self, items): # 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. @@ -709,6 +726,45 @@ def pytest_collection_finish(session): _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()) @@ -716,19 +772,35 @@ def _sorted_ordered_sites(path): return [] out = [] name = pathlib.Path(path).name - for node in ast.walk(tree): - 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" - ) + # 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 diff --git a/test/pytest/test_ordered.py b/test/pytest/test_ordered.py index 08b9ee0c..196ae9ec 100644 --- a/test/pytest/test_ordered.py +++ b/test/pytest/test_ordered.py @@ -78,6 +78,87 @@ def test_sorted_into_an_ordered_claim(expect): 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.""" diff --git a/test/pytest/test_runshape.py b/test/pytest/test_runshape.py index e523e52b..2249da2d 100644 --- a/test/pytest/test_runshape.py +++ b/test/pytest/test_runshape.py @@ -114,3 +114,82 @@ def test_declares_itself(expect): ) 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*"])