From 7d71410a17cb4dd87ad1e0e66eeb61b74e3c7eaf Mon Sep 17 00:00:00 2001 From: cekees <613654+cekees@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:33:35 +0100 Subject: [PATCH] test: check the timeout scaling without waiting on an interpreter test_the_timeout_is_scaled_for_a_slow_runner started a python that slept 1.2 s under a 0.5 s timeout scaled to 1.5 s, so it passed only if the interpreter started in under 0.3 s; on a loaded CI runner it did not (linux-64 py3.12 on cekees/proteus#109). Hand the wrapper a stand-in for subprocess.run instead and look at the timeout it passes on. The wrapper is now retrying_run(run), exposed through the launch_policy fixture. Co-Authored-By: Claude Sonnet 5.5 --- test/conftest.py | 20 ++++++++++++++------ test/test_mpi_launch_retry.py | 21 ++++++++++++++++----- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 785576b11b..42d36ef1a0 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -129,12 +129,12 @@ def launch_policy(): --import-mode=importlib, so they reach them through this fixture).""" import types return types.SimpleNamespace(scaled_timeout=scaled_timeout, died_by_signal=died_by_signal, - launched_environment=launched_environment, os=os) + launched_environment=launched_environment, retrying_run=retrying_run, + os=os) -@pytest.fixture(autouse=True) -def _retry_an_mpi_launch_that_never_started(monkeypatch): - """Every ``subprocess.run`` from a test, hardened for a shared runner: +def retrying_run(run): + """``run`` (a ``subprocess.run``), hardened for a shared runner: * launched again, up to three times in all, if an mpiexec never started (MPI_LAUNCH_FAILURES); the pause before a relaunch is PROTEUS_TEST_MPI_RETRY_DELAY @@ -142,9 +142,11 @@ def _retry_an_mpi_launch_that_never_started(monkeypatch): * launched once more, with a RuntimeWarning, if it died by a signal (died_by_signal); * its ``timeout`` scaled by PROTEUS_TEST_TIMEOUT_SCALE (scaled_timeout); * run with PYTHONFAULTHANDLER=1 (launched_environment). + + A function of ``run``, rather than closed over ``subprocess.run`` inside the fixture, so + that a test can hand it a stand-in and look at what it was called with. """ import warnings - run = subprocess.run def run_with_retry(cmd, *args, **kwargs): if 'timeout' in kwargs: @@ -171,7 +173,13 @@ def run_with_retry(cmd, *args, **kwargs): time.sleep(float(os.environ.get('PROTEUS_TEST_MPI_RETRY_DELAY', 3.0))) continue return result - monkeypatch.setattr(subprocess, 'run', run_with_retry) + return run_with_retry + + +@pytest.fixture(autouse=True) +def _retry_an_mpi_launch_that_never_started(monkeypatch): + """Every ``subprocess.run`` from a test goes through :func:`retrying_run`.""" + monkeypatch.setattr(subprocess, 'run', retrying_run(subprocess.run)) @pytest.fixture(autouse=True) diff --git a/test/test_mpi_launch_retry.py b/test/test_mpi_launch_retry.py index b8b3fdcbcd..e555e05d15 100644 --- a/test/test_mpi_launch_retry.py +++ b/test/test_mpi_launch_retry.py @@ -157,12 +157,23 @@ def test_a_launchers_signal_report_counts_as_a_crash(launch_policy): assert died_by_signal(subprocess.CompletedProcess(['x'], 0, '', '')) is None -def test_the_timeout_is_scaled_for_a_slow_runner(monkeypatch): +def test_the_timeout_is_scaled_for_a_slow_runner(monkeypatch, launch_policy): + # Not by running something slow against a short timeout: that depends on how long an + # interpreter takes to start, which on a loaded runner is the very thing that varies + # (it failed there with 0.3 s of margin). The wrapper is handed a stand-in for + # subprocess.run and asked what timeout it passed on. monkeypatch.setenv('PROTEUS_TEST_TIMEOUT_SCALE', '3') - # 1.2 s of sleep against a 0.5 s timeout: only the scale (1.5 s) lets it finish - result = subprocess.run([sys.executable, '-c', 'import time; time.sleep(1.2)'], - timeout=0.5, capture_output=True) - assert result.returncode == 0 + seen = {} + + def stand_in(cmd, *args, **kwargs): + seen.update(kwargs) + return subprocess.CompletedProcess(cmd, 0, b'', b'') + + launch_policy.retrying_run(stand_in)(['x'], timeout=0.5) + assert seen['timeout'] == 1.5 + seen.clear() + launch_policy.retrying_run(stand_in)(['x']) + assert 'timeout' not in seen # none given, none invented def test_the_scale_is_at_least_one_and_at_most_four(monkeypatch, launch_policy):