From 992175e027ed2d95ac6ac812a9bfca8188305cf5 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:53:25 +0000 Subject: [PATCH] test: a collection-time vacuity refusal keeps its reason under xdist (#963) UsageError inside a worker was rewritten as INTERNALERROR rc 1, because collection_finish still sent the ids. The worker now records the sentence and the controller re-raises UsageError, which is the process serial already used. --- CHANGELOG.md | 15 +++++++ test/pytest/TESTS.md | 29 ++++++++++++++ test/pytest/pgc_vacuity.py | 79 +++++++++++++++++++++++++++++++----- test/pytest/test_layer.py | 82 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 720ff606..019056fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1816,6 +1816,21 @@ true until the next version shipped. the extracted block holds one and the premise passes. The static caller sweep catches it -- injected, both the premise and the arm report `got [4] want [3]`. +- A collection-time vacuity refusal keeps its reason under pytest-xdist (#963). + + `pytest_collection_modifyitems` raises `UsageError`. Serial, that is rc 4 and + the sentence on stderr. Under `-n` pytest still runs `pytest_collection_finish` + in a `finally`, so the worker tells the controller it collected the tests and + then exits. xdist's `worker_workerfinished` asserts a worker that collected + tests must not finish with them pending: a 35-line INTERNALERROR, rc 1, and + the sentence is gone. Measured on the pinned runner (pytest 9.1.1, + pytest-xdist 3.8.0). + + A worker now records the sentence on `workeroutput` and clears the items so + no ids cross. The controller re-raises `UsageError` from `pytest_testnodedown`, + which is the process serial already used. The in-test control (a body that + concludes nothing) is unchanged in both modes. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8104a84e..5853ef5e 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -244,6 +244,35 @@ reaches the exit status, that the override is conditional, and that the two harnesses agree on 67. Against the pre-fix layer it reddens six arms. +### A collection-time refusal must keep its reason under xdist + +#963. `pytest_collection_modifyitems` raises `pytest.UsageError`. Serial, that is +rc 4 and the sentence on stderr. Under `-n` pytest still runs +`pytest_collection_finish` in a `finally`, so the worker tells the controller it +collected the tests and then exits; xdist's `worker_workerfinished` asserts a +worker that collected tests must not finish with them pending, and the reader +gets a 35-line INTERNALERROR, rc 1, and no sentence. + +The table is the test. A VacuityError raised inside a test body is the control: +it is a normal failure and must stay rc 1 with the sentence, in both modes, so +a fix that moved the wrong hook reddens here. + +| test | asserts | +| --- | --- | +| `test_a_bare_skip_refusal_keeps_its_reason_under_xdist` | collection skip: rc 4, sentence, no INTERNALERROR, serial and `-n 2` | +| `test_a_broad_except_refusal_keeps_its_reason_under_xdist` | the same for a second collection-time rule, so the defect is the hook | +| `test_an_in_test_vacuity_refusal_is_unchanged_under_xdist` | **control**: a body that concludes nothing stays rc 1 with the sentence | + +A worker records the sentence on `workeroutput` and clears the items so no ids +cross. The controller re-raises `UsageError` from `pytest_testnodedown`, which +is the process serial already used. Refusing `-n` would drop a runner this +layer already registers an xdist hook for. Turning the refusal into a test +failure would keep rc 1, which is the same code a failing test gives. + +**The shell harness needs no equivalent.** The subject is the collection hook +in `pgc_vacuity.py`. A shell part that greps or inspects that module is the +coupling selftest 350 and 360 deleted. + ### An A/B whose arms agree measures nothing `expect.differ(a, b, name)` is the assertion `mutation-arm-unobservable` says nobody diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 10ded547..b5ec8158 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -1396,6 +1396,17 @@ def pytest_xdist_node_collection_finished(self, node, ids): """ self.collected.update(ids) + def pytest_testnodedown(self, node, error): + """Re-raise a collection refusal the worker could not (#963). + + Per-config, because pytester inner runs share this interpreter and a + module-level hook would fire for the outer session's nodes too. + """ + wo = getattr(node, "workeroutput", None) or {} + msg = wo.get("pgc_vacuity_refusal") + if msg: + raise pytest.UsageError(msg) + def pytest_runtest_logreport(self, report): """Record that a test produced an outcome, and catch a skip during SETUP. @@ -1458,26 +1469,66 @@ def pytest_addoption(parser): ) +def _collection_usage_error(session, config, items, msg): + """Refuse collection as a UsageError, including under xdist (#963). + + Serial: raise UsageError. wrap_session sets rc 4 and pytest.main prints + `ERROR: ` on stderr. + + Under xdist the same raise happens inside a worker. pytest still runs + `pytest_collection_finish` in a `finally`, so the worker tells the + controller it collected the tests, then exits. The controller's + `worker_workerfinished` then asserts that a worker which collected tests + must not finish with them still pending -- a 35-line INTERNALERROR, rc 1, + and the sentence is gone. Measured. + + So a worker does not raise. It records the sentence on workeroutput, + clears the items so collection_finish sends no ids, and sets shouldfail + so worker_workerfinished does not take the crashitem branch even if a + race leaves one. The controller re-raises UsageError from + pytest_testnodedown, which is the process wrap_session already knows + how to print. + """ + if hasattr(config, "workerinput"): + wo = getattr(config, "workeroutput", None) + if wo is None: + config.workeroutput = {} + wo = config.workeroutput + wo["pgc_vacuity_refusal"] = msg + items[:] = [] + session.shouldfail = msg + return + raise pytest.UsageError(msg) + + +@pytest.hookimpl(tryfirst=True) def pytest_collection_finish(session): """Assert the run's own shape, so a filtered or truncated run cannot be green. lib.sh does the equivalent in pgc_summary, which reconciles passed plus failed plus unrunnable against the total and fails when the arithmetic does not close. + + tryfirst so a refusal clears session.items before xdist's collection_finish + sends the ids. Sending first is the INTERNALERROR in `_collection_usage_error`. """ want = session.config.getoption("--pgc-expect-tests") if want is None: return if want <= 0: - raise pytest.UsageError( + _collection_usage_error( + session, session.config, session.items, f"--pgc-expect-tests {want} would be satisfied by a run that collected " - f"nothing, so it asserts nothing. Give the real number." + f"nothing, so it asserts nothing. Give the real number.", ) + return got = len(session.items) if got != want: - raise pytest.UsageError( + _collection_usage_error( + session, session.config, session.items, f"collected {got} test(s) but expected {want}. A run that quietly " - f"collects fewer tests than it should is a green that means nothing." + f"collects fewer tests than it should is a green that means nothing.", ) + return # A broad except in a test swallows the failure the test exists to find. @@ -2023,7 +2074,7 @@ def restore(ns): _arm_bindings, _changed_bindings, _restore_bindings = _binding_guard() -def pytest_collection_modifyitems(config, items, +def pytest_collection_modifyitems(session, config, items, _changed=_changed_bindings, _restore=_restore_bindings): """Refuse a bare skip, which exits 0 and reads as success. @@ -2037,7 +2088,8 @@ def pytest_collection_modifyitems(config, items, _rebound = _changed(globals()) if _rebound: _restore(globals()) - raise pytest.UsageError( + _collection_usage_error( + session, config, items, "the pgColumnar vacuity layer refuses this run: a conftest or plugin " "rebound the layer's own " + ("names " if len(_rebound) > 1 else "name ") @@ -2046,8 +2098,9 @@ def pytest_collection_modifyitems(config, items, "run would have reported on rules that were switched off. The bindings " "have been restored. If a check is wrong for your case, say so where the " "run records it: expect.cannot_run(REASON, detail), which names a reason " - "from a closed list, or fix the test the rule is objecting to." + "from a closed list, or fix the test the rule is objecting to.", ) + return offenders = [] seen_files = set() @@ -2081,13 +2134,15 @@ def pytest_collection_modifyitems(config, items, if "empty parameter set" in reason: empty_params.append(f"{item.name}: {reason}") if empty_params: - raise pytest.UsageError( + _collection_usage_error( + session, config, items, "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." + + " -- assert the corpus is non-empty before parametrizing over it.", ) + return 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 @@ -2143,9 +2198,11 @@ def pytest_collection_modifyitems(config, items, + " -- move the setup above the block, leaving the statement under " "test alone inside it" ) - raise pytest.UsageError( - "the pgColumnar vacuity layer refuses this run: " + ". ".join(parts) + "." + _collection_usage_error( + session, config, items, + "the pgColumnar vacuity layer refuses this run: " + ". ".join(parts) + ".", ) + return # ARMED HERE, AT THE BOTTOM, because a snapshot taken earlier would miss every name diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index f153f4fe..9a580cf2 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -6,6 +6,8 @@ its own arithmetic. """ +import pytest + def test_layer_rejects_a_test_with_no_assertion(pytester, expect): """A test body that concludes nothing must fail, not pass. @@ -270,6 +272,86 @@ def test_rowcount_sentinel(expect): result.stdout.fnmatch_lines(["*no row count available*"]) +def _collection_refusal_row(result, expect, *, rc, reason_glob, name): + """One row of the #963 table: exit status, the sentence, no INTERNALERROR.""" + blob = result.stdout.str() + result.stderr.str() + expect.num(result.ret, rc, f"{name}: exit status") + expect.num(blob.count("INTERNALERROR"), 0, f"{name}: INTERNALERROR lines") + if rc == 4: + # UsageError is written to stderr. The stream is the claim: any non-zero + # exit would satisfy run_failed, including the INTERNALERROR this issue + # exists to close. + result.stderr.fnmatch_lines([reason_glob]) + else: + result.stdout.fnmatch_lines([reason_glob]) + + +@pytest.mark.parametrize("mode", ["serial", "xdist"]) +def test_a_bare_skip_refusal_keeps_its_reason_under_xdist(pytester, expect, mode): + """#963. A collection-time UsageError must stay rc 4 with the sentence, + including under `-n 2`. Measured on main: serial printed the reason on + stderr and exited 4; `-n 2` replaced it with a 35-line INTERNALERROR and + exited 1. + """ + pytester.makepyfile( + """ + import pytest + @pytest.mark.skip(reason="not today") + def test_quietly_gone(): + assert False + """ + ) + extra = ("-n", "2") if mode == "xdist" else () + result = pytester.runpytest("-p", "pgc_vacuity", *extra) + _collection_refusal_row( + result, expect, rc=4, reason_glob="*bare skip*", + name=f"bare skip {mode}", + ) + + +@pytest.mark.parametrize("mode", ["serial", "xdist"]) +def test_a_broad_except_refusal_keeps_its_reason_under_xdist(pytester, expect, mode): + """#963. Same table, second collection-time row. The defect is the hook, + not the skip marker, so a second offence has to keep the sentence too. + """ + pytester.makepyfile( + """ + def test_swallows(expect): + try: + raise RuntimeError("the real failure") + except Exception: + pass + expect.num(1, 1, "and then asserts something harmless") + """ + ) + extra = ("-n", "2") if mode == "xdist" else () + result = pytester.runpytest("-p", "pgc_vacuity", *extra) + _collection_refusal_row( + result, expect, rc=4, reason_glob="*catches Exception broadly*", + name=f"broad except {mode}", + ) + + +@pytest.mark.parametrize("mode", ["serial", "xdist"]) +def test_an_in_test_vacuity_refusal_is_unchanged_under_xdist(pytester, expect, mode): + """#963 control. A VacuityError raised inside a test body is a normal + failure and reports identically under serial and `-n 2`. If this arm + started needing rc 4, the fix would have moved the wrong hook. + """ + pytester.makepyfile( + """ + def test_asserts_nothing(): + x = 1 + 1 + """ + ) + extra = ("-n", "2") if mode == "xdist" else () + result = pytester.runpytest("-p", "pgc_vacuity", *extra) + _collection_refusal_row( + result, expect, rc=1, reason_glob="*made no counted assertion*", + name=f"in-test vacuity {mode}", + ) + + def test_layer_rejects_a_broad_except_in_a_test_file(pytester, expect): """The layer forbade this in a comment, which enforces nothing.