fix: a subset run cannot judge what it did not collect (#1204)

[jdatcmd]

Closes #1204.

Running one pytest file on a major with rows in expected_unrunnable.txt exited 67 while reporting its own file passed, naming six tests it never collected as proof that a tracked list was stale.

$ pytest test_native_reclaim_cycles.py --pg-config .../pg17
  1 passed, 15 checks, 0 fail        exit 67

Green on PG 18 and 19, which have no rows, so it was invisible from the majors CI gates pull requests with. Reported by @OffgridwithJD.

The check had an unstated premise

unexpected, ran_anyway = unrunnable_offences(actual, allowed)

ran_anyway is allowed - actual: listed as declining, did not decline. That is only answerable for a test the run COLLECTED. A subset collects none of the others, so the question was not stale, it was unasked.

unexpected is actual - allowed and is answerable from any run: a test that declined, declined, whoever else was collected. It is untouched.

The instruction was worse than the noise, measured

                                     pytest test_temporal.py --pg-config .../pg17
list intact         exit 67   "expected to be unrunnable on PG17 and RAN"
PG17 rows deleted   exit 67   "unrunnable on PG17, and not expected to be"

test_temporal.py is one of the six. Running exactly the file that is supposed to decline still failed, because the other five were uncollected. And doing what the message said moved the red to the opposite direction, where a full run fires against rows that were correct.

A guard that refuses correct work gets switched off and takes its rule with it. A false positive that tells you how to break the thing is a different class from one that cries wolf.

The fix states the premise per ROW, not per run

allowed &= {item.nodeid for item in session.items}

@OffgridwithJD's diagnosis was the premise; their remedy was to gate the whole direction on a full run, which --pgc-expect-tests establishes. Intersecting with what was collected says the same thing per row, so a subset keeps whatever the subset can answer instead of switching the direction off.

Verified on the real corpus, not only in the layer

case	exit
a stale row for a test the subset does collect	67	names it
an unexpected decline on a one-file run	67	names it and its reason
the listed decliner alone, list intact	0	was 67

A full run is unchanged, because every listed row is collected there -- which is exactly the condition the first row tests.

TDD, in that order

RED    a listed test the run never collected is not judged: got 67 want 0
GREEN  test_layer.py  50 passed, 103 checks, 0 fail

The control beside it is the existing test_a_listed_test_that_runs_makes_the_list_stale, which lists a test that IS in the run: it must still fire, and it does.

And @OffgridwithJD tried to break it

They had a specific objection worth recording: under xdist the workers collect, not the controller, so session.items on the controller might be empty and the stale-list direction would switch itself off under -n -- a silent regression of the half #1192 bought.

They applied the diff to main with a deliberately stale row planted and measured:

SERIAL     rc=67   1 stale-message    467 passed
XDIST -n2  rc=67   2 stale-messages   467 passed

Caught in both. The doubled message under -n is cosmetic. Their objection did not hold, and the check is verified by someone who is not the author of the remedy.

Bookkeeping

guard_tests 402 -> 403, derived by collection over the 24 NO_CLUSTER files, not by adding one. cluster_tests re-derived in the same run and unmoved at 467, which is the expected answer for a change touching only the layer and its own guard.

The arm lives in test_layer.py because that is where the vacuity layer's own guards live and it drives pytest inside pytest, which is the only way to observe a subset run's exit status. There is no shell twin: pgc_vacuity.py has no shell counterpart, and #1192 set that precedent for the same reason.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n

[OffgridwithJD]

Verified at 887252c, behind_main=0. Approving, with one departure from my usual bar stated openly at the end.

The issue's own repro, fixed

one file on PG 17            rc=0   1 passed   stale-msgs=0     (was exit 67)
exactly the declining file   rc=0   1 passed   stale-msgs=0     (was exit 67)

Both halves of #1204 are closed, including the one neither of us expected — running precisely the file that is supposed to decline used to fail because the other five were not collected.

And it still has its teeth, tested against your code rather than my transcription

I stress-tested this design before it existed, against my own reading of your proposal. That is not the same as testing what you wrote, so I ran it again on 887252c with a deliberately stale row — a test that certainly runs on 18, listed as expected-to-decline — plus this box's ICU row so only the planted row was in play:

SERIAL    rc=67   stale-msgs=1   467 passed
XDIST -n2 rc=67   stale-msgs=2   467 passed

Caught in both. So the per-row intersection keeps everything #1192 bought while dropping only the unanswerable half, which is exactly the claim.

My objection to this design was wrong and I want that on the record here rather than only in our messages. I argued the controller's session.items would be empty under xdist, citing your own pytest_xdist_node_collection_finished docstring, and that the stale-list direction would therefore switch itself off under -n. It does not. Your per-ROW premise beats the per-RUN gate I proposed, and it survives the case I raised against it.

The single red is inherited

pytest (cluster tests, PG 18):

got 'differs by 12 (on=218 off=230 over 1000 groups)' want 'within 5'

That is #1203's arm, in test_sorted_pathkeys.py, which this PR does not touch. It is now the fourth occurrence and the second major — I have added it to #1203 with the data. Note the direction flipped: #1201 read on=255 off=216 (39 more), this reads on=218 off=230 (12 fewer). A systematic cost has a sign; this does not.

The departure, stated

My standing rule is to approve only on a fully green gate, and this gate is 14 of 15. I am approving anyway because I can show the fifteenth is not yours: the same arm fails on main, it lives in a file this PR does not modify, and it has now failed on two majors in two directions across three unrelated branches.

If that arm turns out to be a real regression rather than a noisy measurement, this approval is void and I will say so on this PR. Holding a third PR hostage to an arm that fails on main is a worse error than the one the rule exists to prevent.

Merging remains @jdatcmd's call.