diff --git a/test/pytest/README.md b/test/pytest/README.md index b5c13307..eb7a7207 100644 --- a/test/pytest/README.md +++ b/test/pytest/README.md @@ -3,6 +3,11 @@ This is the issue #432 pilot. It runs beside `test/*.sh`, and replaces nothing. - `TESTS.md` in this directory documents every test and every assertion helper. +- `VACUITY_MODES.md` is the inventory of ways a pytest harness can report a false + pass: 79 modes produced by the enumeration, 72 of them named in that + file, 73 demonstrated by a run, and 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 a3a4f811..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. -**90 tests in 6 files.** Seventy-five 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. @@ -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 @@ -606,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 | @@ -761,7 +791,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 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. + +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..58d077ea --- /dev/null +++ b/test/pytest/VACUITY_MODES.md @@ -0,0 +1,227 @@ +# The vacuity modes: what is refused, what is not, and what nobody attacked + +A vacuity defect is a test that reports PASS while asserting nothing. This file is +the inventory: every way a pytest harness can do that which anyone here has +demonstrated, which of them `pgc_vacuity.py` refuses today, and which it does not. + +`TESTS.md` documents the tests that exist. This documents the ones that should. + +## 1. Where these numbers come from, and what did not run + +A five-angle enumeration ran in the audit container against pytest 9.1.1, +pytest-xdist 3.8.0 and psycopg 3.3.5. Each mode had to be demonstrated by an actual +run rather than described. + +| stage | started | completed | failed | +| --- | ---: | ---: | ---: | +| enumerate the modes | 5 | 5 | 0 | +| design a refusal per mode | 79 | 74 | 5 | +| **attack each refusal** | **148** | **0** | **148** | +| synthesize the layer | 1 | 0 | 1 | + +**The adversarial stage did not run.** It was cut off by a session limit, so the +summary line reading `defeated: 0` counts zero defeats out of **zero completed +attacks**. That number is not evidence that these refusals survive attack, and this +document is the synthesis the failed stage would have produced, written by hand from +the stage outputs that did complete. + +So: 79 modes produced by the run, of which **72 are named here** (see 1a), and **73 demonstrated by a run**. 74 refusals designed, every one of them +stating a residual. None of the 74 has been adversarially tested. + +## 1a. How to count a mode in this document + +**A mode is a backticked kebab-case identifier of three or more words**, such as +`collected-but-nothing-asserted`. That is the counting rule, stated because the +document had none and its numbers therefore could not be checked — which is a +poor property for a document about claims that cannot be checked. + +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 + +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 | +| --- | --- | +| 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 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 + +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 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 af398d4e..3991f5bf 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. @@ -246,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. @@ -315,6 +359,7 @@ def plan_marker(self, plan, key, name=None, absent=False): seen.update(k for k in node if k.startswith("Columnar")) if key in node: found = True + self._counted() if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") @@ -482,6 +527,56 @@ 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") + continue + # A TUPLE HANDLER IS THE SHAPE PEOPLE ACTUALLY WRITE. + # + # This looked only at a bare `ast.Name`, so `except Exception:` was + # refused and `except (ValueError, Exception):` passed (@jdatcmd, #905 + # review). Measured against the real layer, three spellings of one + # swallow: + # + # except Exception: -> refused + # except (ValueError, Exception): -> PASSED <- the hole + # except BaseException: -> refused + # + # A tuple is how this gets written when someone starts with a specific + # exception and widens it under pressure, which is the exact moment the + # guard is for -- so the hole was in the case the guard most needed to + # cover. Any member of the tuple being broad makes the handler broad. + members = t.elts if isinstance(t, ast.Tuple) else [t] + for m in members: + if isinstance(m, ast.Name) and m.id in ("Exception", "BaseException"): + out.append(f"{name}:{node.lineno} except {m.id}") + break + return out + + def pytest_collection_modifyitems(config, items): """Refuse a bare skip, which exits 0 and reads as success. @@ -489,13 +584,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_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/pytest/test_layer.py b/test/pytest/test_layer.py index 831bd15c..8f1ae4b9 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -254,3 +254,39 @@ def test_ok(expect): ) result = pytester.runpytest("-p", "pgc_vacuity") expect.num(result.ret, 0, "an ordinary green run is untouched") + + +def test_layer_rejects_psycopgs_no_count_sentinel(pytester, expect): + """cursor.rowcount is -1 when no count is available, and 1 for an unfetched + SELECT. Both are numbers, so expect.num compares them happily.""" + pytester.makepyfile( + """ + def test_rowcount_sentinel(expect): + expect.rowcount(-1, -1, "a count that is not a count") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "the -1 sentinel is refused", failed=1, passed=0) + result.stdout.fnmatch_lines(["*no row count available*"]) + + +def test_layer_rejects_a_broad_except_in_a_test_file(pytester, expect): + """The layer forbade this in a comment, which enforces nothing. + + After any failed statement psycopg raises InFailedSqlTransaction for every + later one, so one `except Exception` hides the real error and all its + successors. Measured: a test using the forbidden shape passed with no complaint. + """ + pytester.makepyfile( + """ + def test_swallows(expect): + try: + raise RuntimeError("the real failure") + except Exception: + pass + expect.num(1, 1, "and then asserts something harmless") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + expect.run_failed(result, "a broad except must not be collectable") + result.stderr.fnmatch_lines(["*catches Exception broadly*"]) diff --git a/test/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