diff --git a/tests/conftest.py b/tests/conftest.py index e1d5f107c..3bf13614a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -418,6 +418,19 @@ def pytest_collection_modifyitems(session: 'pytest.Session', config: 'pytest.Con #: loads this conftest but never collects ``test_tier_guard_xdist.py`` at all. PROBE_MODULE_NAME = 'test_zzz_xdist_guard_probe_unit.py' +#: Keeps the probe out of every session that does not name it on the command +#: line (GitHub issue #1756). A worker that dies mid-probe never runs the +#: probe's cleanup, and xdist's replacement worker then collects ``tests/`` +#: afresh: without this, its collection-time audit finds the stale probe, sets +#: ``session.shouldfail``, and the controller interrupts the whole session. +#: pytest consults ``collect_ignore`` only while recursing into a directory, +#: never for an initial path (``Dir.collect`` skips the check for +#: ``session.isinitpath``), so the probe's own nested run, which passes the +#: probe's path explicitly, still collects and audits it. Only this one +#: reserved path is ignored, and :class:`tests.test_tier_guard.SuiteIsCleanTests` +#: still scans it on disk like any other module. +collect_ignore = [f'protocols/{PROBE_MODULE_NAME}'] + #: Module names whose own tests manage :func:`_tier_guard_lock` themselves and #: must not also take the shared hold :func:`_tier_guard_reader_lock` puts on #: every other unit-tier test. diff --git a/tests/test_tier_guard_xdist.py b/tests/test_tier_guard_xdist.py index 2b78cbce1..966ec6ee1 100644 --- a/tests/test_tier_guard_xdist.py +++ b/tests/test_tier_guard_xdist.py @@ -37,6 +37,11 @@ ``SuiteIsCleanTests``, without that module's own source changing at all. :class:`ProbeWriteLockTests` pins the mutex itself, independently of xdist. +A third (#1756): a worker that dies mid-probe never runs its cleanup, and the +replacement worker xdist starts collects the stale probe. ``collect_ignore`` in +:mod:`tests.conftest` keeps the probe out of every recursive collection, and +:class:`ProbeWorkerCrashTests` kills a real worker mid-probe to pin that. + Kept separate from :mod:`tests.test_tier_guard`, which another change is already modifying: this module touches none of its code, only :mod:`tests.conftest`. @@ -45,8 +50,10 @@ import importlib.util import os +import shutil import subprocess import sys +import tempfile import textwrap import threading import unittest @@ -237,6 +244,98 @@ def test_xdist_run_reports_the_diagnostic_and_fails(self) -> None: self.assertNotIn('crashitem', output) +#: Module name of the ``-p`` plugin :class:`ProbeWorkerCrashTests` loads into +#: its throwaway session; written to a temporary directory, never to the tree. +_CRASH_PLUGIN_NAME = 'pcapkit_tier_guard_probe_crash' + +#: Kills a worker mid-probe, as CI's GraalPy leg lost one (job 116215824320): +#: after :class:`XdistSubprocessTests` has written the probe and before its +#: cleanup can remove it. The lock becomes a no-op because the test +#: that launched the session holds the exclusive hold on the probe's behalf. +_CRASH_PLUGIN_SOURCE = textwrap.dedent(f""" + import contextlib + import os + import subprocess + + _run = subprocess.run + + + def _crashing_run(args, *posargs, **kwargs): + if any(str(arg).endswith({_conftest.PROBE_MODULE_NAME!r}) for arg in args): + os._exit(1) + return _run(args, *posargs, **kwargs) + + + def pytest_runtest_setup(item): + if hasattr(item.config, 'workerinput'): + from tests import conftest + conftest._tier_guard_lock = lambda exclusive: contextlib.nullcontext() + subprocess.run = _crashing_run + """).lstrip('\n') + + +@unittest.skipUnless(importlib.util.find_spec('xdist') is not None, + 'pytest-xdist not installed') +class ProbeWorkerCrashTests(unittest.TestCase): + """A worker dying mid-probe fails that one test, not the session (#1756). + + xdist replaces the dead worker, and the replacement collects ``tests/`` + afresh while the probe's cleanup never ran. Before ``collect_ignore`` in + :mod:`tests.conftest`, that collection audited the stale probe and the + controller ended the session with ``xdist.dsession.Interrupted``, exit 2. + + """ + + def setUp(self) -> None: + reason = _tiers.guard_unavailable_reason() + if reason is not None: + self.skipTest(f'git cannot answer here: {reason}') + + self.probe_path = _tiers.TESTS_ROOT / 'protocols' / _conftest.PROBE_MODULE_NAME + # Exclusive for the whole throwaway session and until the stale probe is + # gone, so a concurrent SuiteIsCleanTests never sees it; LIFO cleanups + # unlink while still holding the lock, as in XdistSubprocessTests. + lock = _conftest._tier_guard_lock(exclusive=True) + lock.__enter__() + self.addCleanup(lock.__exit__, None, None, None) + self.assertFalse( + self.probe_path.exists(), + f'{self.probe_path} already exists -- a previous run did not clean up', + ) + self.addCleanup(self.probe_path.unlink, missing_ok=True) + + self.plugin_dir = tempfile.mkdtemp(prefix='pcapkit-1756-') + self.addCleanup(shutil.rmtree, self.plugin_dir, ignore_errors=True) + with open(os.path.join(self.plugin_dir, f'{_CRASH_PLUGIN_NAME}.py'), 'w', + encoding='utf-8') as handle: + handle.write(_CRASH_PLUGIN_SOURCE) + + def test_a_worker_crash_mid_probe_does_not_interrupt_the_session(self) -> None: + """Fails on main: exit 2 and the probe's diagnostic as the interrupt reason.""" + protocols = self.probe_path.parent + environ = _child_environ() + environ['PYTHONPATH'] = os.pathsep.join([self.plugin_dir, environ['PYTHONPATH']]) + probe_test = ('tests/test_tier_guard_xdist.py::XdistSubprocessTests' + '::test_xdist_run_reports_the_diagnostic_and_fails') + completed = subprocess.run( + [sys.executable, '-m', 'pytest', '-p', 'no:cacheprovider', '-p', _CRASH_PLUGIN_NAME, + '-q', '-n', '2', '--dist', 'load', '--max-worker-restart=2', probe_test, + # The directory the probe lives in, as CI collects it, minus every + # module but test_z*: only the probe could be left there. + 'tests/protocols', f'--ignore-glob={protocols}/[!t]*', + f'--ignore-glob={protocols}/t[!e]*', f'--ignore-glob={protocols}/test_[!z]*'], + cwd=str(_tiers.ROOT), env=environ, capture_output=True, text=True, + timeout=300, check=False, + ) + output = completed.stdout + completed.stderr + + self.assertIn(f"crashed while running '{probe_test}'", output) + self.assertTrue(self.probe_path.exists(), 'the crash did not leave the probe behind') + self.assertNotIn('Interrupted', output) + self.assertNotIn(_DIAGNOSTIC_MARKER, output) + self.assertEqual(completed.returncode, 1, output) + + class ProbeWriteLockTests(unittest.TestCase): """:func:`tests.conftest._tier_guard_lock` actually excludes a concurrent reader.