fix: an unguarded pyarrow import is refused under pytest -n too (#1305)

test/pytest/TESTS.md

@@ -6486,6 +6486,7 @@ Written red first: all seven failed before `pgc_vacuity` had the methods. The fo
 | `test_a_broken_install_fails_rather_than_declines` | a `pyarrow` that raises `ImportError` from inside fails with its own error |
 | `test_a_missing_dependency_of_pyarrow_fails_rather_than_declines` | a `pyarrow` that cannot import its own dependency raises `ModuleNotFoundError` naming that dependency, and fails rather than declines; shown red with `require_module`'s `.name` check deleted (OffgridwithJD, review of #1286) |
 | `test_every_unguarded_import_spelling_is_refused` | each of six spellings, `__import__` among them, is named and refused; a file using `require_module` is not |
+| `test_an_unguarded_import_is_refused_under_xdist_too` | under `-n 2` the controller names the unguarded import once and the run is refused, as the serial run is: workers ship their sites back through `workeroutput` (#1305) |
 
 ## 92. test_tid_range_scan.py: a ctid range reads only the row groups that hold it
 

test/pytest/expected_tests.txt

@@ -267,7 +267,8 @@
 # NO_CLUSTER. Re-derived by collection on 73d9663: `412 tests collected`.
 # guard_tests 412 -> 413 for test_a_missing_dependency_of_pyarrow_fails_rather_than_declines
 # (#1286 review), collected over the NO_CLUSTER files on this tree.
-guard_tests 413
+# guard_tests 413 -> 414 for #1305's test_an_unguarded_import_is_refused_under_xdist_too, by collection.
+guard_tests 414
 
 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran
 # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and

test/pytest/pgc_vacuity.py

@@ -1554,7 +1554,16 @@ def pytest_sessionfinish(session, exitstatus):
     # A test file that imports a declinable capability outside require_module is
     # refused (#1286 review): it would fail on a box without the module, or pass
     # having caught its own ImportError, and either way the decline is not named.
-    if getattr(session.config, "pgc_capability_imports", None) and session.exitstatus == 0:
+    #
+    # UNDER XDIST THE WORKERS COLLECT, so the sites they found are shipped back
+    # through workeroutput and merged on the controller in pytest_testnodedown
+    # (#1305). Measured before that: the serial run said REFUSED and exited 1, the
+    # -n 2 run said `2 passed` and exited 0.
+    if hasattr(session.config, "workerinput"):
+        sites = getattr(session.config, "pgc_capability_imports", None)
+        if sites:
+            session.config.workeroutput["pgc_capability_imports"] = list(sites)
+    elif getattr(session.config, "pgc_capability_imports", None) and session.exitstatus == 0:
         session.exitstatus = 1
 
     # THE RECONCILIATION, FIRST, because it is a statement about whether the run
@@ -2027,6 +2036,26 @@ def _capability_import_sites(path, capabilities=DECLINABLE_CAPABILITIES):
     return sites
 
 
+@pytest.hookimpl(optionalhook=True)
+def pytest_testnodedown(node, error):
+    """Merge a worker's unguarded-import sites into the controller's list (#1305).
+
+    Every worker collects every file, so each site arrives once per worker; the
+    list keeps the first of each. Optional, because the hook exists only when
+    xdist is loaded.
+    """
+    sites = getattr(node, "workeroutput", {}).get("pgc_capability_imports") or []
+    if not sites:
+        return
+    store = getattr(node.config, "pgc_capability_imports", None)
+    if store is None:
+        store = []
+        node.config.pgc_capability_imports = store
+    for site in sites:
+        if site not in store:
+            store.append(site)
+
+
 def pytest_collect_file(file_path, parent):
     """Scan each test file for direct imports of a declinable capability.
 

test/pytest/test_capability_decline.py

@@ -174,3 +174,29 @@ def test_guarded(expect):
     expect.num(1 if result.ret != 0 else 0, 1, "and a run carrying them is refused")
     expect.num(out.count("test_guarded.py imports pyarrow"), 0,
                "while a file that goes through require_module is not named")
+
+
+def test_an_unguarded_import_is_refused_under_xdist_too(pytester, monkeypatch, expect):
+    """The refusal reaches the controller when workers do the collecting (#1305).
+
+    Under `-n`, pytest_collect_file runs in the workers, so the sites they found
+    never reached the controller, which prints the summary and sets the exit status:
+    measured by @OffgridwithJD, the serial run said REFUSED and exited 1, the `-n 2`
+    run said `2 passed` and exited 0. A subprocess, because xdist's workers are.
+    """
+    import pathlib
+    monkeypatch.delenv("PGC_REQUIRE", raising=False)
+    pkg = pytester.mkpydir("pyarrow")
+    (pkg / "parquet.py").write_text("")
+    here = pathlib.Path(__file__).resolve().parent
+    monkeypatch.setenv("PYTHONPATH", f"{pytester.path}:{here}")
+    pytester.makepyfile(test_bad="import pyarrow.parquet as pq\n\ndef test_x(expect):\n    expect.num(1, 1, 'x')\n",
+                        test_ok="def test_y(expect):\n    expect.num(1, 1, 'y')\n")
+    serial = pytester.runpytest_subprocess("-p", "pgc_vacuity", "-p", "no:cacheprovider")
+    xdist = pytester.runpytest_subprocess("-p", "pgc_vacuity", "-p", "no:cacheprovider", "-n", "2")
+    expect.num(1 if serial.ret != 0 and "REFUSED  test_bad.py" in serial.stdout.str() else 0, 1,
+               "premise: the serial run refuses the unguarded import")
+    out = xdist.stdout.str()
+    expect.num(out.count("REFUSED  test_bad.py imports pyarrow outside require_module"), 1,
+               "under -n 2 the controller names the unguarded import, once")
+    expect.num(1 if xdist.ret != 0 else 0, 1, "and the -n 2 run is refused, not green")