From 81786988f071b6c47800278f7b64d26b07aadbba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Juaristi?= <127882282+juaristi22@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:23:10 +0200 Subject: [PATCH] Budget the trade CLI tests for the engine import their CLI pays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_cli_fails_when_margins_missing_from_window` timed out on main (run 32910539480) after passing on #766's final run, which tested a merge ref with the same content — nothing landed between that run and the merge. Same code, same group, 380 passed there against 379 plus one timeout here, so the break is marginal timing rather than logic. The margin is thin because the CLI pays for machinery it never uses. Importing `us_runtime.us_trade.*` executes `us_runtime/__init__`, which pulls in spine_agreement and through it the whole policyengine-us system. Measured on the failing test's own fixture — a CLI that reads a two-row parquet and exits 1 — that is 35.9 s with the engine installed against 5.4 s without, and `build_us_import_entry_margins.py` carries the identical 33.0 s chain. The import is parameter-tree file I/O, so it stretches with runner disk contention; 300 s left under 10x headroom on a call whose useful work is milliseconds. Name the budget and raise it to 900 s at the six full-CLI spawn sites across both trade test files, with the measurements recorded where the number lives. The lightweight `-c` spawns keep their 60 s: they do not import the chain. This buys headroom; it does not remove the cost. The cost disappears by deferring the engine import in `us_runtime/__init__`, which would also cut ~30 s from each of these spawns — US source with unclear load-bearing behavior, so it is filed for its owners rather than attempted in a hotfix. Verified: both files pass against the change, 91 passed in 10m15s in an engine environment. Co-Authored-By: Claude Fable 5 --- .../766-trade-cli-subprocess-timeout.fixed.md | 3 +++ .../tests/test_us_trade_entries_cli.py | 16 ++++++++++++++-- .../tests/test_us_trade_imdb_bulk.py | 15 +++++++++++---- 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 changelog.d/766-trade-cli-subprocess-timeout.fixed.md diff --git a/changelog.d/766-trade-cli-subprocess-timeout.fixed.md b/changelog.d/766-trade-cli-subprocess-timeout.fixed.md new file mode 100644 index 000000000..b647f2980 --- /dev/null +++ b/changelog.d/766-trade-cli-subprocess-timeout.fixed.md @@ -0,0 +1,3 @@ +Give the synthetic import-entry CLI tests a subprocess budget that covers the +rules-engine import their CLI pays before doing any work, so they stop timing +out on contended runners. diff --git a/packages/microcosm-build/tests/test_us_trade_entries_cli.py b/packages/microcosm-build/tests/test_us_trade_entries_cli.py index b2310c3ea..d34f02520 100644 --- a/packages/microcosm-build/tests/test_us_trade_entries_cli.py +++ b/packages/microcosm-build/tests/test_us_trade_entries_cli.py @@ -30,6 +30,18 @@ GOLDEN = Path(__file__).parent / "golden" / "us_trade" CLI = Path(__file__).resolve().parents[3] / "tools" / "build_us_import_entries.py" + +# Every spawn of the CLI pays its import chain before doing any work, and that +# chain is dominated by machinery the trade code never uses: importing +# `us_runtime.us_trade.*` executes `us_runtime/__init__`, which pulls in +# spine_agreement and, through it, the whole policyengine-us system. Measured +# on the same fixture as `test_cli_fails_when_margins_missing_from_window` +# (a CLI that only reads a 2-row parquet and exits 1): 35.9 s with the engine +# installed, 5.4 s without. That import is parameter-tree I/O, so it stretches +# with runner disk contention — at the old 300 s this test timed out on main +# (run 32910539480) after passing on the PR. The headroom belongs here until +# the import is made lazy; see the note on the PR that raised this. +CLI_TIMEOUT_SECONDS = 900 CONTRACT = GOLDEN / "engine_entry_contract.json" @@ -94,7 +106,7 @@ def _run(margins_dir: Path, out_dir: Path, start: str = "2026-01"): capture_output=True, text=True, check=False, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) @@ -346,7 +358,7 @@ def test_override_build_registers_override_basis(tmp_path): capture_output=True, text=True, check=False, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) assert result.returncode == 0, result.stderr register = json.loads((out_dir / "assumptions.json").read_text()) diff --git a/packages/microcosm-build/tests/test_us_trade_imdb_bulk.py b/packages/microcosm-build/tests/test_us_trade_imdb_bulk.py index 8ca0b5c6c..5cba02dfd 100644 --- a/packages/microcosm-build/tests/test_us_trade_imdb_bulk.py +++ b/packages/microcosm-build/tests/test_us_trade_imdb_bulk.py @@ -38,6 +38,13 @@ REPO_ROOT = Path(__file__).resolve().parents[3] BUILD_CLI = REPO_ROOT / "tools" / "build_us_import_entry_margins.py" +# Same import tax as test_us_trade_entries_cli: importing `us_runtime.us_trade.*` +# executes `us_runtime/__init__`, which drags in spine_agreement and the whole +# policyengine-us system before this CLI does any work — measured at 33.0 s of +# import in the engine lane. The lightweight `-c` spawns below keep their 60 s +# budget; only the full-CLI spawns pay this. +CLI_TIMEOUT_SECONDS = 900 + def _field(line: list[str], start: int, end: int, value: str, *, align: str) -> None: """Place ``value`` into the 1-indexed inclusive [start, end] span.""" @@ -822,7 +829,7 @@ def test_build_cli_end_to_end_offline(tmp_path): ], capture_output=True, text=True, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) assert result.returncode == 0, result.stderr report = json.loads((out_dir / "build_report.json").read_text()) @@ -905,7 +912,7 @@ def test_build_cli_failure_leaves_prior_publication_untouched(tmp_path): ], capture_output=True, text=True, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) assert result.returncode == 1 assert "nothing was published" in result.stderr @@ -963,7 +970,7 @@ def test_build_cli_success_replaces_prior_publication_completely(tmp_path): ], capture_output=True, text=True, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) assert result.returncode == 0, result.stderr assert not stale.exists() @@ -998,7 +1005,7 @@ def test_build_cli_fails_on_control_mismatch(tmp_path): ], capture_output=True, text=True, - timeout=300, + timeout=CLI_TIMEOUT_SECONDS, ) assert result.returncode == 1 assert "RECONCILIATION FAIL" in result.stderr