diff --git a/osprey_worker/conftest.py b/osprey_worker/conftest.py index e9f0d59c..f1c52071 100644 --- a/osprey_worker/conftest.py +++ b/osprey_worker/conftest.py @@ -17,6 +17,10 @@ from _pytest.config.argparsing import Parser +# capture launch state before test imports can patch gevent +_STARTED_PREPATCHED = monkey.is_module_patched('socket') + + def pytest_addoption(parser: 'Parser') -> None: """Register custom pytest options. @@ -69,13 +73,14 @@ def pytest_sessionfinish(session: object, exitstatus: int) -> None: the process can stall inside gevent's interpreter finalization and never exit, so the integration-tests CI job hangs until its 30-minute timeout. Exit immediately here (trylast, so this runs after the junit/terminal - hooks) to skip that teardown. Guarded on gevent being active so a plain - ``pytest`` run finalizes normally. + hooks) to skip that teardown. Guarded on pytest having *started* under the + gevent-monkey runner (see ``_STARTED_PREPATCHED``) so a plain ``pytest`` run + finalizes normally even if a test module monkey-patched during collection. """ import os import sys - if not monkey.is_module_patched('socket'): + if not _STARTED_PREPATCHED: return sys.stdout.flush() sys.stderr.flush() diff --git a/osprey_worker/src/osprey/worker/lib/discovery/tests/test_discovery.py b/osprey_worker/src/osprey/worker/lib/discovery/tests/test_discovery.py index 7279a5da..86aa79f6 100644 --- a/osprey_worker/src/osprey/worker/lib/discovery/tests/test_discovery.py +++ b/osprey_worker/src/osprey/worker/lib/discovery/tests/test_discovery.py @@ -1,6 +1,8 @@ from gevent import monkey -monkey.patch_all(aggressive=True) # noqa: E402 +# late monkey-patching corrupts locks during plain pytest collection +if monkey.is_module_patched('socket'): + monkey.patch_all(aggressive=True) import gevent # noqa: E402 import pytest # noqa: E402 diff --git a/osprey_worker/src/osprey/worker/lib/etcd/tests/conftest.py b/osprey_worker/src/osprey/worker/lib/etcd/tests/conftest.py index fdb28562..8892c227 100644 --- a/osprey_worker/src/osprey/worker/lib/etcd/tests/conftest.py +++ b/osprey_worker/src/osprey/worker/lib/etcd/tests/conftest.py @@ -1,8 +1,10 @@ # flake8: noqa E402 # ruff: noqa: E402 -from gevent.monkey import patch_all +from gevent import monkey -patch_all() +# late monkey-patching corrupts locks during plain pytest collection +if monkey.is_module_patched('socket'): + monkey.patch_all() import json diff --git a/osprey_worker/src/osprey/worker/lib/instruments/tests/test_concurrency.py b/osprey_worker/src/osprey/worker/lib/instruments/tests/test_concurrency.py index 426d0867..2af2249b 100644 --- a/osprey_worker/src/osprey/worker/lib/instruments/tests/test_concurrency.py +++ b/osprey_worker/src/osprey/worker/lib/instruments/tests/test_concurrency.py @@ -1,8 +1,9 @@ -# Patch so we can test concurrency +# late monkey-patching corrupts locks during plain pytest collection import gevent import gevent.monkey -gevent.monkey.patch_all() +if gevent.monkey.is_module_patched('socket'): + gevent.monkey.patch_all() from unittest.mock import call # noqa: E402 diff --git a/osprey_worker/tests/test_gevent_collection.py b/osprey_worker/tests/test_gevent_collection.py new file mode 100644 index 00000000..bb006f2f --- /dev/null +++ b/osprey_worker/tests/test_gevent_collection.py @@ -0,0 +1,65 @@ +"""Regression tests for plain pytest collection of gevent-patched test modules. + +Some test modules under ``osprey_worker`` used to call ``monkey.patch_all`` at +import time. Under plain ``pytest`` (not the pre-patched integration runner) +that late patch mutates global gevent state after threading/ssl are already +imported, raising ``RuntimeError: cannot release un-acquired lock`` during +collection. The ``pytest_sessionfinish`` force-exit hook then saw socket as +patched and called ``os._exit``, hiding the terminal traceback and leaving a +bare exit code 2. + +These tests run plain ``pytest --collect-only`` in a child process (so gevent +is NOT pre-patched) and assert collection succeeds with a normal summary. +""" + +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +# repo root is three parents up from this file +_REPO_ROOT = Path(__file__).resolve().parents[2] + +# one representative target per import-time gevent patch site: +# - discovery/tests/test_discovery.py patches in the module body +# - instruments/tests/test_concurrency.py patches in the module body +# - etcd/tests/conftest.py patches in the conftest, so collecting any etcd test +# exercises it +_GEVENT_TEST_MODULES = [ + 'osprey_worker/src/osprey/worker/lib/discovery/tests/test_discovery.py', + 'osprey_worker/src/osprey/worker/lib/instruments/tests/test_concurrency.py', + 'osprey_worker/src/osprey/worker/lib/etcd/tests/test_dict.py', +] + + +def _collect_only_plain(target: str) -> subprocess.CompletedProcess[str]: + """Run ``pytest --collect-only`` on ``target`` in a plain (non pre-patched) child.""" + return subprocess.run( + [sys.executable, '-m', 'pytest', '--collect-only', '-q', '-p', 'no:cacheprovider', target], + cwd=_REPO_ROOT, + capture_output=True, + text=True, + timeout=30, + ) + + +@pytest.mark.parametrize('target', _GEVENT_TEST_MODULES) +def test_plain_collection_succeeds(target: str) -> None: + module_path = _REPO_ROOT / target + assert module_path.exists(), f'{target} does not exist' + + result = _collect_only_plain(target) + + combined = result.stdout + result.stderr + assert result.returncode == 0, ( + f'plain `pytest --collect-only` on {target} exited {result.returncode}\n' + f'stdout:\n{result.stdout}\nstderr:\n{result.stderr}' + ) + # the specific late-monkey-patch failure and pytest's collection-error + # markers must be absent, and a healthy run reports a nonzero collected + # count ("no tests collected in ..." must not match) + assert 'cannot release un-acquired lock' not in combined, combined + assert 'error during collection' not in combined, combined + assert re.search(r'\d+ tests? collected in', result.stdout), result.stdout