From 5942f6b2b3780f853e7c443d940acbcca5dcd36d Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Fri, 5 Jun 2026 20:47:30 -0400 Subject: [PATCH] fix: eliminate pandoc subprocess thrash under pytest-xdist -n auto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of recurring 35929 / 38536 CI flake — `-n auto` on a 32-core CI runner spawns 32 pytest workers. test_pandoc.py contains ~14 tests that each fork a `pandoc` subprocess. When several land on parallel workers at once, pandoc's cold-start cost (GHC runtime + filter loading) plus runner CPU contention pushes wall time past the 5s metadata timeout and 30s convert timeout. Subprocess gets SIGKILL'd (returncode -9), tests fail, pipeline reruns the same flake. Three-part root cause fix: 1. Cache get_available_input_formats / get_available_output_formats with functools.lru_cache(maxsize=1). Pandoc's format list is static for a given binary; we only need one subprocess per Python process, not one per call site. Also bump the metadata-query timeout from 5s to 30s — the first cold-start under contention still has to succeed. 2. Mark TestConvert + TestAvailableFormats with @pytest.mark.xdist_group("pandoc-subprocess") so every pandoc-forking test pins to one xdist worker. Other 31 workers continue parallel- processing the rest of the suite; pandoc tests run sequentially on their single worker, each pandoc cold-start completes before the next starts. 3. Update Makefile test target to pass --dist=loadgroup so pytest-xdist honors the xdist_group marker. Other test classes in test_pandoc.py (TestNodeTreeToMarkdown, TestNamespaceToMarkdown, TestGetAuthorName) use MockNode objects without data_html, so node_tree_to_markdown's pandoc path stays cold — those classes don't need the marker. Replaces 1527ef7 (which just retriggered the same flake). --- Makefile | 8 ++++++-- remarkbox/lib/pandoc.py | 24 ++++++++++++++++++++---- remarkbox/tests/test_pandoc.py | 14 +++++++++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index f9ce0f9..15883e9 100644 --- a/Makefile +++ b/Makefile @@ -149,10 +149,14 @@ activate: @echo "To activate the virtual environment, run:" @echo " source $(VENV_DIR)/bin/activate" -# Run the test suite (installs test dependencies if needed) +# Run the test suite (installs test dependencies if needed). +# --dist=loadgroup pins tests sharing an xdist_group marker to a single +# worker — used by test_pandoc.py to serialize ~14 pandoc subprocesses +# that would otherwise race cold-start CPU contention on CI and exceed +# the 5s/30s subprocess timeouts. test: install-source-dev-and-test @echo "Running tests in parallel..." - $(VENV_DIR)/bin/py.test -n auto + $(VENV_DIR)/bin/py.test -n auto --dist=loadgroup # Start a simple HTTP server (for serving static files like index.html) http: venv diff --git a/remarkbox/lib/pandoc.py b/remarkbox/lib/pandoc.py index 1e9385d..f1e2e53 100644 --- a/remarkbox/lib/pandoc.py +++ b/remarkbox/lib/pandoc.py @@ -8,6 +8,7 @@ import logging import subprocess import tempfile import os +from functools import lru_cache log = logging.getLogger(__name__) @@ -155,12 +156,23 @@ def resolve_format(ext): return EXTENSION_ALIASES.get(ext, ext) +# 30s — generous on cold-start contended runners (xdist N-worker fan-out). +# The result is cached, so this cost gets paid at most once per process. +_FORMAT_QUERY_TIMEOUT = 30 + + +@lru_cache(maxsize=1) def get_available_output_formats(): - """Return the set of output formats pandoc supports on this system.""" + """Return the set of output formats pandoc supports on this system. + + Result cached per process — pandoc's format list is static for a given + binary, so subsequent callers (and parallel pytest workers each in their + own Python process) skip the subprocess entirely after first call. + """ try: result = subprocess.run( ["pandoc", "--list-output-formats"], - capture_output=True, text=True, timeout=5, + capture_output=True, text=True, timeout=_FORMAT_QUERY_TIMEOUT, ) return frozenset(result.stdout.strip().split("\n")) except Exception: @@ -168,12 +180,16 @@ def get_available_output_formats(): return frozenset() +@lru_cache(maxsize=1) def get_available_input_formats(): - """Return the set of input formats pandoc supports on this system.""" + """Return the set of input formats pandoc supports on this system. + + Result cached per process — see get_available_output_formats() above. + """ try: result = subprocess.run( ["pandoc", "--list-input-formats"], - capture_output=True, text=True, timeout=5, + capture_output=True, text=True, timeout=_FORMAT_QUERY_TIMEOUT, ) return frozenset(result.stdout.strip().split("\n")) except Exception: diff --git a/remarkbox/tests/test_pandoc.py b/remarkbox/tests/test_pandoc.py index 37f77bf..7829576 100644 --- a/remarkbox/tests/test_pandoc.py +++ b/remarkbox/tests/test_pandoc.py @@ -1,4 +1,14 @@ -"""Unit tests for remarkbox.lib.pandoc — pandoc subprocess wrapper.""" +"""Unit tests for remarkbox.lib.pandoc — pandoc subprocess wrapper. + +xdist_group("pandoc-subprocess") on every class that fires pandoc as a +subprocess. With -n auto on a 32-core CI runner, ~14 of these tests would +otherwise race for `pandoc` cold-starts simultaneously, exceeding 5s/30s +subprocess timeouts under GHC-runtime + filter-loading contention. +Grouping pins every pandoc-subprocess test onto one worker, so each pandoc +invocation completes before the next starts. Other workers keep parallel- +processing the rest of the suite. Requires Makefile to pass +--dist=loadgroup so pytest-xdist honors the marker. +""" import shutil import unittest @@ -19,6 +29,7 @@ from remarkbox.lib.pandoc import ( ) +@pytest.mark.xdist_group("pandoc-subprocess") class TestConvert(unittest.TestCase): """Unit tests for the pandoc convert() function.""" @@ -95,6 +106,7 @@ class TestConvert(unittest.TestCase): convert("test", "markdown", "not_a_real_format_xyz") +@pytest.mark.xdist_group("pandoc-subprocess") class TestAvailableFormats(unittest.TestCase): """Test format discovery."""