From 0fadffc654b0203f2d5033c25b3b2e4f66e02ed6 Mon Sep 17 00:00:00 2001 From: Jay Hesselberth Date: Sun, 13 Sep 2026 18:14:30 -0600 Subject: [PATCH 1/3] ci: add job-level timeout backstop and pytest-timeout PR #308 hung ~55 minutes with nothing to stop it; add a 20-minute job-level timeout plus a 120s per-test pytest-timeout (thread method, since SIGALRM can't interrupt a C-level lock). --- .github/workflows/ci.yml | 7 +++++++ pyproject.toml | 22 ++++++++++++++++++++++ uv.lock | 18 +++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) 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/pyproject.toml b/pyproject.toml index 96ba6cbf..910941cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,6 +104,7 @@ notebook = [ [dependency-groups] dev = [ "maturin>=1.12.6", + "pytest-timeout>=2.4.0", ] docs = [ "matplotlib>=3.10.7", @@ -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/uv.lock b/uv.lock index 302fe0d1..b7e35686 100644 --- a/uv.lock +++ b/uv.lock @@ -1309,6 +1309,7 @@ test = [ [package.dev-dependencies] dev = [ { name = "maturin", marker = "sys_platform != 'emscripten'" }, + { name = "pytest-timeout", marker = "sys_platform != 'emscripten'" }, ] docs = [ { name = "matplotlib", marker = "sys_platform != 'emscripten'" }, @@ -1354,7 +1355,10 @@ requires-dist = [ provides-extras = ["rust", "pod5", "onnx", "test", "dev", "pipeline", "notebook"] [package.metadata.requires-dev] -dev = [{ name = "maturin", specifier = ">=1.12.6" }] +dev = [ + { name = "maturin", specifier = ">=1.12.6" }, + { name = "pytest-timeout", specifier = ">=2.4.0" }, +] docs = [ { name = "matplotlib", specifier = ">=3.10.7" }, { name = "mkdocstrings", extras = ["python"], specifier = ">=1.0.6" }, @@ -2634,6 +2638,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" From 1e19202d55110f72c3ac7eb9fef6f4c912ff46ce Mon Sep 17 00:00:00 2001 From: Jay Hesselberth Date: Sun, 13 Sep 2026 18:22:45 -0600 Subject: [PATCH 2/3] chore: add changelog fragment for #309 --- changelog.d/309.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/309.changed.md 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. From 7e7ca3e1535d7194e2899f879e89ae814718664c Mon Sep 17 00:00:00 2001 From: Jay Hesselberth Date: Sun, 13 Sep 2026 18:49:51 -0600 Subject: [PATCH 3/3] fix: install pytest-timeout where CI actually reads it, fix hang-test races pytest-timeout was declared under [dependency-groups].dev, which CI's install step never installs (it runs `uv pip install -e ".[test,rust,onnx]"`, not `uv sync`) -- the whole per-test timeout was a silent no-op in CI. Move it to the `test` extra instead, verified by simulating CI's exact install command in a fresh venv. Also mark the three existing hang-regression tests that carry their own subprocess.run(timeout=120) (test_inference.py's test_parallel_path_alone_writes_tags and test_a_fork_after_a_waited_shutdown_completes, test_parallel_prep.py's test_python_backend_real_pool_matches_rust_chunk_set) with @pytest.mark.timeout(150), so their own internal timeout fires and reports cleanly instead of racing the new global 120s timeout. And distinguish "pytest finished with nothing to report" from "pytest was killed mid-run" in the job-summary action, so a future timeout kill doesn't get a falsely reassuring "no skipped/xfailed tests" summary. --- .github/actions/pytest-skip-summary/action.yml | 8 ++++++++ pyproject.toml | 2 +- tests/test_inference.py | 13 +++++++++++++ tests/test_parallel_prep.py | 7 +++++++ uv.lock | 9 ++++----- 5 files changed, 33 insertions(+), 6 deletions(-) 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/pyproject.toml b/pyproject.toml index 910941cb..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. @@ -104,7 +105,6 @@ notebook = [ [dependency-groups] dev = [ "maturin>=1.12.6", - "pytest-timeout>=2.4.0", ] docs = [ "matplotlib>=3.10.7", 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 b7e35686..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,13 +1304,13 @@ 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'" }, ] [package.dev-dependencies] dev = [ { name = "maturin", marker = "sys_platform != 'emscripten'" }, - { name = "pytest-timeout", marker = "sys_platform != 'emscripten'" }, ] docs = [ { name = "matplotlib", marker = "sys_platform != 'emscripten'" }, @@ -1335,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" }, @@ -1355,10 +1357,7 @@ requires-dist = [ provides-extras = ["rust", "pod5", "onnx", "test", "dev", "pipeline", "notebook"] [package.metadata.requires-dev] -dev = [ - { name = "maturin", specifier = ">=1.12.6" }, - { name = "pytest-timeout", specifier = ">=2.4.0" }, -] +dev = [{ name = "maturin", specifier = ">=1.12.6" }] docs = [ { name = "matplotlib", specifier = ">=3.10.7" }, { name = "mkdocstrings", extras = ["python"], specifier = ">=1.0.6" },