diff --git a/.github/actions/pytest-skip-summary/action.yml b/.github/actions/pytest-skip-summary/action.yml index 94e2359e..59bf8968 100644 --- a/.github/actions/pytest-skip-summary/action.yml +++ b/.github/actions/pytest-skip-summary/action.yml @@ -17,6 +17,14 @@ runs: echo '```' if [ ! -f pytest-output.log ]; then echo "pytest did not run -- see the failed step above" + elif ! grep -qE ' in [0-9]+\.[0-9]+s' pytest-output.log; then + # pytest always prints a final "N passed/failed... in X.XXs" line + # on a normal exit, pass or fail. Its absence means the process + # was killed mid-run (e.g. pytest-timeout's `thread` method + # force-exiting via os._exit()) rather than that nothing was + # skipped -- distinguish the two so a hung run doesn't get a + # falsely reassuring summary. + echo "pytest did not finish normally (killed, crashed, or timed out) -- see the failed step above" elif grep -q "short test summary info" pytest-output.log; then awk '/short test summary info/,0' pytest-output.log else diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8460ce9a..69198691 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,13 @@ jobs: test: name: Test - Python ${{ matrix.python-version }} runs-on: ubuntu-latest + # Hard backstop behind pytest-timeout's per-test enforcement (pyproject.toml's + # [tool.pytest.ini_options] `timeout`): a hang during collection, before any + # test has started, or inside the Rust/maturin build in the install step + # below is outside pytest's control entirely. PR #308 hung here for ~55 + # minutes (a fork-safety deadlock) with nothing to stop it until a human + # noticed and cancelled the run. + timeout-minutes: 20 strategy: fail-fast: false matrix: diff --git a/changelog.d/309.changed.md b/changelog.d/309.changed.md new file mode 100644 index 00000000..fc53700a --- /dev/null +++ b/changelog.d/309.changed.md @@ -0,0 +1 @@ +**CI now fails fast on a hung test run instead of burning an hour of compute.** `pytest-timeout` caps every test at 120s, using the `thread` method rather than the platform-default `signal` one — a watchdog thread dumps every stack and force-exits the process, which also catches a hang stuck in a C-level lock that a `SIGALRM` can't interrupt. `.github/workflows/ci.yml`'s test job additionally carries a 20-minute job-level `timeout-minutes` backstop for hangs outside pytest's control (test collection, the pre-test Rust build). Prompted by PR #308's "Run tests" job hanging for ~55 minutes with nothing to stop it until a human noticed and cancelled it. diff --git a/pyproject.toml b/pyproject.toml index 96ba6cbf..d1c4dc8e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,6 +76,7 @@ onnx = [ test = [ "pytest>=8.0.0", "pytest-cov>=4.1.0", + "pytest-timeout>=2.4.0", "pytest-xdist>=3.5.0", ] # Full local dev env: test runners + lint/type/QA tooling. @@ -127,6 +128,27 @@ addopts = "-v" python_files = "test_*.py" python_functions = "test_*" testpaths = ["tests"] +# Per-test hard cap so a hung test fails loudly instead of running out the +# clock -- see .github/workflows/ci.yml's job-level `timeout-minutes: 20` for +# the backstop behind collection-time and pre-test hangs this can't catch. +# The full suite's slowest test currently runs in ~22s +# (test_distributed_training.py's two-rank checkpoint/gradient-parity tests, +# which spin up a real 2-process DDP group); 120s leaves >5x headroom for a +# slower or more contended CI runner while still failing well inside the +# job-level backstop. Measured via `srun -p rna -c 8 --mem 32G -t 45 -- +# uv run pytest --durations=10` against the extras CI installs +# (test,rust,onnx): 1707 passed, 45 skipped in 322.89s. +timeout = 120 +# pytest-timeout's own platform default is "signal" (SIGALRM) wherever +# available, which is Linux/CI here -- but a SIGALRM only interrupts the +# interpreter at the next bytecode boundary, which is exactly what a hang +# stuck in a C-level lock (a forked multiprocessing.Pool deadlocking against +# libtorch's already-initialized thread pool, as in rnabioco/leech#308) will +# never reach. "thread" runs a separate watchdog thread that dumps every +# thread's stack and force-exits the whole process via os._exit() regardless +# of what the main thread is blocked on, so it is set explicitly here rather +# than left to the per-platform default. +timeout_method = "thread" markers = ["slow: slow tests (torch.export, bundles) — run with --slow"] [tool.uv] diff --git a/tests/test_inference.py b/tests/test_inference.py index fea11343..55814e59 100644 --- a/tests/test_inference.py +++ b/tests/test_inference.py @@ -1176,6 +1176,13 @@ def test_rust_path_writes_tags(self, tmp_path): rust_predicted = self._predict(tmp_path, bundle_path, "rust", backend="rust") assert rust_predicted + # Longer than pyproject.toml's global 120s: the subprocess.run(..., + # timeout=120) below is this test's own hang guard for the exact #308 + # deadlock, and must be the one to fire (raising subprocess.TimeoutExpired + # with a clean assertion message) rather than racing the global per-test + # timeout, which would instead kill the whole pytest process via + # os._exit() before the subprocess's own timeout could report cleanly. + @pytest.mark.timeout(150) def test_parallel_path_alone_writes_tags(self, tmp_path): """``num_workers > 0`` (mp.Pool, always Python extraction) -- previously untested here (all prior bundle tests ran serial only). @@ -1784,6 +1791,12 @@ class TestSequentialThenParallelInProcess: print("OK") """ + # Longer than pyproject.toml's global 120s -- same reasoning as + # test_parallel_path_alone_writes_tags above: the subprocess.run(..., + # timeout=120) inside this test is the intended hang guard ("the bug + # under test IS a hang"), and needs room to fire before the global + # per-test timeout would otherwise race it and kill the whole process. + @pytest.mark.timeout(150) def test_a_fork_after_a_waited_shutdown_completes(self, tmp_path): """Runs in a subprocess with a hard timeout: the bug under test IS a hang.""" import subprocess diff --git a/tests/test_parallel_prep.py b/tests/test_parallel_prep.py index 65d264a0..f5d5c4de 100644 --- a/tests/test_parallel_prep.py +++ b/tests/test_parallel_prep.py @@ -806,6 +806,13 @@ def test_rust_backend_end_to_end(self): assert all(c["label"] == "Ala" for c in chunks) assert all(int(c["label_int"]) == 1 for c in chunks) + # Longer than pyproject.toml's global 120s: this test's own + # subprocess.run(..., timeout=120) below IS the hang guard (a real fork + # deadlock must raise subprocess.TimeoutExpired with a clean assertion + # message, not have the whole pytest process killed out from under it by + # the global timeout firing at the same 120s mark). See CLAUDE.md's "A + # test that exercises the real mp.Pool" note. + @pytest.mark.timeout(150) def test_python_backend_real_pool_matches_rust_chunk_set(self): """``backend_choice="python", num_workers=2`` -- the real mp.Pool path. diff --git a/uv.lock b/uv.lock index 302fe0d1..179b5538 100644 --- a/uv.lock +++ b/uv.lock @@ -1274,6 +1274,7 @@ dev = [ { name = "onnxscript", marker = "sys_platform != 'emscripten'" }, { name = "pytest", marker = "sys_platform != 'emscripten'" }, { name = "pytest-cov", marker = "sys_platform != 'emscripten'" }, + { name = "pytest-timeout", marker = "sys_platform != 'emscripten'" }, { name = "pytest-xdist", marker = "sys_platform != 'emscripten'" }, { name = "ruff", marker = "sys_platform != 'emscripten'" }, { name = "towncrier", marker = "sys_platform != 'emscripten'" }, @@ -1303,6 +1304,7 @@ rust = [ test = [ { name = "pytest", marker = "sys_platform != 'emscripten'" }, { name = "pytest-cov", marker = "sys_platform != 'emscripten'" }, + { name = "pytest-timeout", marker = "sys_platform != 'emscripten'" }, { name = "pytest-xdist", marker = "sys_platform != 'emscripten'" }, ] @@ -1334,6 +1336,7 @@ requires-dist = [ { name = "pysam", specifier = ">=0.22.0" }, { name = "pytest", marker = "extra == 'test'", specifier = ">=8.0.0" }, { name = "pytest-cov", marker = "extra == 'test'", specifier = ">=4.1.0" }, + { name = "pytest-timeout", marker = "extra == 'test'", specifier = ">=2.4.0" }, { name = "pytest-xdist", marker = "extra == 'test'", specifier = ">=3.5.0" }, { name = "pyyaml", specifier = ">=6.0" }, { name = "rich", specifier = ">=13.0.0" }, @@ -2634,6 +2637,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, ] +[[package]] +name = "pytest-timeout" +version = "2.4.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest", marker = "sys_platform != 'emscripten'" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/ac/82/4c9ecabab13363e72d880f2fb504c5f750433b2b6f16e99f4ec21ada284c/pytest_timeout-2.4.0.tar.gz", hash = "sha256:7e68e90b01f9eff71332b25001f85c75495fc4e3a836701876183c4bcfd0540a", size = 17973, upload-time = "2025-05-05T19:44:34.99Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fa/b6/3127540ecdf1464a00e5a01ee60a1b09175f6913f0644ac748494d9c4b21/pytest_timeout-2.4.0-py3-none-any.whl", hash = "sha256:c42667e5cdadb151aeb5b26d114aff6bdf5a907f176a007a30b940d3d865b5c2", size = 14382, upload-time = "2025-05-05T19:44:33.502Z" }, +] + [[package]] name = "pytest-xdist" version = "3.8.0"