Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions test/pytest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
52 changes: 47 additions & 5 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.

Expand All @@ -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
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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.
Expand Down
227 changes: 227 additions & 0 deletions test/pytest/VACUITY_MODES.md
Original file line number Diff line number Diff line change
@@ -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.<seq>`; 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.
Loading
Loading