Running a single pytest file on PG 15, 16 or 17 exits 67 while reporting 1 passed, and prints an instruction that, if followed, deletes correct rows and blinds the real check.
Reported by @OffgridwithJD, who hit it while reviewing #1201. Reproduced and measured below.
Reproduce
$ pytest test_native_reclaim_cycles.py --pg-config /usr/local/pg17/bin/pg_config
expected to be unrunnable on PG17 and RAN, so the list is stale:
test_analyze_differential.py::test_analyze_differential
test_analyze_function.py::test_analyze_function
test_analyze_function.py::test_histogram_bounds_are_a_positional_stride
test_analyze_function.py::test_null_frac_counts_live_rows_not_ones_a_delete_left
test_analyze_function.py::test_the_documented_statistics_are_the_ones_written
test_temporal.py::test_temporal
-- edit test/pytest/expected_unrunnable.txt, with the reason.
checks run: 15
accounting: 15 pass + 0 fail + 0 unrun = 15
1 passed in 0.77s
$ echo $?
67
The file under test passed every check. Six tests it never collected are reported as proof that a tracked list is stale.
It is green on PG 18 and 19, which have no rows in expected_unrunnable.txt, so it is invisible to anyone working on the majors CI gates pull requests with.
The check has an unstated premise
pytest_sessionfinish computes both directions from pgc_vacuity.py:1506:
actual = {nodeid for nodeid, _, _ in collector.items}
unexpected, ran_anyway = unrunnable_offences(actual, allowed)
ran_anyway is allowed - actual: listed as expected-to-decline, did not decline. That is only answerable if the run collected the test. A subset run collects none of them, so "did not decline" is not a stale row, it is a question nobody asked.
The other direction, unexpected = actual - allowed, is answerable from any run: a test that declined, declined, whoever else was collected.
Following the message makes it worse, measured
pytest test_temporal.py --pg-config .../pg17
list intact exit 67 expected-to-be-unrunnable ... and RAN (the other five)
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 fails, because the other five were not collected. And doing what the message says moves the red to the opposite direction, where on a full run it would fire against rows that were correct.
So this is not noise. It is a false positive that tells the reader how to break the thing, and this project's own rule is that a guard which refuses correct work gets switched off and takes its rule with it.
What I think the fix is
Judge only the listed tests this run actually collected. In pytest_sessionfinish:
collected = {item.nodeid for item in session.items}
ran_anyway = (allowed & collected) - actual
@OffgridwithJD suggested gating the ran_anyway direction on the run having collected the whole corpus, which --pgc-expect-tests establishes when it is passed and matched. That is the same premise; intersecting with session.items states it per row rather than per run, so a subset keeps whatever the subset can answer instead of switching the direction off.
Done when
A single-file run on PG 15, 16 and 17 exits 0 when its file passes, a listed decliner that stops declining still reddens a run that collected it, and an unexpected decline still reddens a one-file run.
The arm for it is a subset run: today's guards cannot see this because they only ever run the whole corpus.
Running a single pytest file on PG 15, 16 or 17 exits 67 while reporting
1 passed, and prints an instruction that, if followed, deletes correct rows and blinds the real check.Reported by @OffgridwithJD, who hit it while reviewing #1201. Reproduced and measured below.
Reproduce
The file under test passed every check. Six tests it never collected are reported as proof that a tracked list is stale.
It is green on PG 18 and 19, which have no rows in
expected_unrunnable.txt, so it is invisible to anyone working on the majors CI gates pull requests with.The check has an unstated premise
pytest_sessionfinishcomputes both directions frompgc_vacuity.py:1506:ran_anywayisallowed - actual: listed as expected-to-decline, did not decline. That is only answerable if the run collected the test. A subset run collects none of them, so "did not decline" is not a stale row, it is a question nobody asked.The other direction,
unexpected = actual - allowed, is answerable from any run: a test that declined, declined, whoever else was collected.Following the message makes it worse, measured
test_temporal.pyis one of the six. Running exactly the file that is supposed to decline still fails, because the other five were not collected. And doing what the message says moves the red to the opposite direction, where on a full run it would fire against rows that were correct.So this is not noise. It is a false positive that tells the reader how to break the thing, and this project's own rule is that a guard which refuses correct work gets switched off and takes its rule with it.
What I think the fix is
Judge only the listed tests this run actually collected. In
pytest_sessionfinish:allowedis a subset ofcollected, so nothing changes and build: the pytest corpus runs on more than one major (#1163) #1192's stale-list check keeps its teeth.unexpectedis untouched, so an unexpected decline still reddens a one-file run, which is the half build: the pytest corpus runs on more than one major (#1163) #1192 bought that matters most while porting.@OffgridwithJD suggested gating the
ran_anywaydirection on the run having collected the whole corpus, which--pgc-expect-testsestablishes when it is passed and matched. That is the same premise; intersecting withsession.itemsstates it per row rather than per run, so a subset keeps whatever the subset can answer instead of switching the direction off.Done when
A single-file run on PG 15, 16 and 17 exits 0 when its file passes, a listed decliner that stops declining still reddens a run that collected it, and an unexpected decline still reddens a one-file run.
The arm for it is a subset run: today's guards cannot see this because they only ever run the whole corpus.