From bcf291f82492df0ac47ef60ef4a96fb30bbb4d8b Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Tue, 29 Sep 2026 11:40:00 -0300 Subject: [PATCH 1/4] Budget e2e time per scenario, not as a fixed suite total The fixed 120 s total failed main after #110 (120.1 s, 185 passing scenarios, none over 3 s) and had already been raised from 60 s and 90 s for the same reason. A scenario over 10 s now fails the run by name, and the suite as a whole is capped at 1.5 s per scenario, so the cap grows with the suite but still catches a doubled average. --- docs/operations.md | 3 +- openspec/changes/e2e-harness/tasks.md | 2 +- tests/e2e/conftest.py | 41 +++++++++++++++++++-------- tests/unit/test_e2e_budget.py | 27 ++++++++++++++++++ 4 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 tests/unit/test_e2e_budget.py diff --git a/docs/operations.md b/docs/operations.md index 72f7735..8b11381 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -198,7 +198,8 @@ ran, and the memory and fact rows, read by SQL. Every `/name` the bot says is checked against the commands Discord offers where it said it. - `E2E_REQUIRE_DB=1` turns "no database" from a skip into a failure. CI sets it. -- The suite fails the run if it takes more than 120 seconds. +- The suite fails the run if any scenario takes more than 10 seconds, or the + whole suite more than 1.5 seconds per scenario on average. - `E2E_UPDATE_SNAPSHOTS=1` rewrites `tests/e2e/snapshots/commands.json`, the exact command payload the bot syncs; review the diff like code. - discord.py is pinned to `~=2.7.1` in the dev extras because the fake wire diff --git a/openspec/changes/e2e-harness/tasks.md b/openspec/changes/e2e-harness/tasks.md index b0659f5..82fbd14 100644 --- a/openspec/changes/e2e-harness/tasks.md +++ b/openspec/changes/e2e-harness/tasks.md @@ -37,7 +37,7 @@ - [x] 4.4 Real Postgres: `TEST_DATABASE_URL` with a testcontainers pgvector fallback, `E2E_REQUIRE_DB=1` turning a skip into a failure, a TRUNCATE per test - [x] 4.5 Network canary: httpx's real transports raise inside the suite - [x] 4.6 discord.py pinned `~=2.7.1` in the dev extras; a canary test naming each internal the wire uses -- [x] 4.7 A 60-second budget for the suite, enforced (since raised to 90 s, then 120 s) +- [x] 4.7 A 60-second budget for the suite, enforced (since raised to 90 s, then 120 s; replaced by a 10 s per-scenario budget and a 1.5 s-per-scenario suite backstop, because a fixed total failed main as the suite grew) - [x] 4.8 CI step: `pytest tests/e2e` with `E2E_REQUIRE_DB=1` after the migration - [x] 4.9 Docs: end-to-end tests in `docs/operations.md`, including the rule that a production fix replays its transcript as a scenario diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 9553c4e..2801eb6 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -34,10 +34,13 @@ ROOT = Path(__file__).resolve().parents[2] DEFAULT_URL = "postgresql+asyncpg://chatmemory:chatmemory@localhost:5432/chatmemory" PGVECTOR_IMAGE = "pgvector/pgvector:pg17" -# Raised from 60 s at 100 scenarios, when CI first ran 60.6 s with every test -# green, and from 90 s at ~150, when CI ran 92.8 s with no scenario over 3 s. -# The budget exists to catch one slow scenario, not a growing suite. -BUDGET_SECONDS = 120.0 +# A scenario over this is the regression the budget exists to catch: one slow +# test, named. The slowest today is under 3 s. +TEST_BUDGET_SECONDS = 10.0 +# A backstop for the whole suite that grows with it. A fixed total (60 s, then +# 90 s, then 120 s) failed main each time enough scenarios were added, with +# every test passing and none of them slow. +SUITE_SECONDS_PER_TEST = 1.5 COLLEAGUE_CRYPTO = ( "Our project treasury 0xD5C95aF87F6e1E83507AC96b2eE4484B9AFEbDd5 holds the " @@ -192,22 +195,36 @@ async def bot( # --- the time budget ---------------------------------------------------------- -_spent = 0.0 +_durations: dict[str, float] = {} + + +def over_budget(durations: dict[str, float]) -> list[str]: + """What broke the budget: each slow scenario, then the suite as a whole.""" + problems = [ + f"{node} took {spent:.1f}s, over the {TEST_BUDGET_SECONDS:.0f}s per-scenario budget" + for node, spent in sorted(durations.items()) + if spent > TEST_BUDGET_SECONDS + ] + total, allowed = sum(durations.values()), SUITE_SECONDS_PER_TEST * len(durations) + if total > allowed: + problems.append( + f"end-to-end suite took {total:.1f}s, over its {allowed:.0f}s budget " + f"({SUITE_SECONDS_PER_TEST}s x {len(durations)} scenarios)" + ) + return problems def pytest_runtest_logreport(report: pytest.TestReport) -> None: - global _spent if report.nodeid.startswith("tests/e2e/"): - _spent += report.duration + _durations[report.nodeid] = _durations.get(report.nodeid, 0.0) + report.duration def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None: """The budget is enforced, not hoped for: over it, the run fails.""" - if _spent > BUDGET_SECONDS: + problems = over_budget(_durations) + if problems: reporter = session.config.pluginmanager.get_plugin("terminalreporter") if reporter is not None: - reporter.write_line( - f"end-to-end suite took {_spent:.1f}s, over its {BUDGET_SECONDS:.0f}s budget", - red=True, - ) + for line in problems: + reporter.write_line(line, red=True) session.exitstatus = pytest.ExitCode.TESTS_FAILED diff --git a/tests/unit/test_e2e_budget.py b/tests/unit/test_e2e_budget.py new file mode 100644 index 0000000..a314ae2 --- /dev/null +++ b/tests/unit/test_e2e_budget.py @@ -0,0 +1,27 @@ +"""The end-to-end time budget: one slow scenario fails it; a growing suite does not. + +Regression: a fixed 120 s total failed main at 185 scenarios, every one passing +and none over 3 s. The budget was raised twice before for the same reason. +""" + +from __future__ import annotations + +from tests.e2e.conftest import SUITE_SECONDS_PER_TEST, TEST_BUDGET_SECONDS, over_budget + + +def test_a_large_suite_of_quick_scenarios_is_within_budget() -> None: + durations = {f"tests/e2e/test_x.py::test_{i}": 0.65 for i in range(185)} + assert over_budget(durations) == [] + + +def test_one_slow_scenario_is_named() -> None: + durations = {f"tests/e2e/test_x.py::test_{i}": 0.5 for i in range(20)} + durations["tests/e2e/test_x.py::test_slow"] = TEST_BUDGET_SECONDS + 1 + [problem] = over_budget(durations) + assert "test_slow" in problem and "per-scenario" in problem + + +def test_a_suite_whose_average_doubles_fails() -> None: + durations = {f"tests/e2e/test_x.py::test_{i}": SUITE_SECONDS_PER_TEST + 0.5 for i in range(50)} + [problem] = over_budget(durations) + assert "suite took" in problem From 1f4a23f8b655cc9b0ad39b276047ef89a7c90432 Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Tue, 29 Sep 2026 11:44:43 -0300 Subject: [PATCH 2/4] Skip the live CoinGecko test when CoinGecko refuses the runner CI for #111 failed on test_live_btc_and_eth_carry_their_quote_time with CoinGecko answering HTTP 403 to the GitHub runner. The fixture probed only Frankfurter; the crypto test now pings CoinGecko first and skips on 403/429, the keyless tier refusing a shared IP or a burst. --- tests/integration/test_market_live.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_market_live.py b/tests/integration/test_market_live.py index a608d04..c430a73 100644 --- a/tests/integration/test_market_live.py +++ b/tests/integration/test_market_live.py @@ -57,6 +57,21 @@ async def online() -> AsyncIterator[None]: yield +#: Statuses CoinGecko's keyless tier answers when it refuses a caller outright +#: (a CI runner's shared IP, or a burst). Our code cannot be tested against a +#: provider that will not answer, so the crypto test skips rather than fails. +PROVIDER_REFUSED = frozenset({403, 429}) + + +@pytest_asyncio.fixture +async def coingecko(online: None) -> AsyncIterator[None]: + async with httpx.AsyncClient(timeout=5.0) as client: + ping = await client.get("https://api.coingecko.com/api/v3/ping") + if ping.status_code in PROVIDER_REFUSED: + pytest.skip(f"CoinGecko refuses this runner: HTTP {ping.status_code}") + yield + + class Governed: """One federation for a test, so a cached figure is reused as in the bot.""" @@ -106,7 +121,7 @@ async def ask( return await run.ask(question, tool, arguments) -async def test_live_btc_and_eth_carry_their_quote_time(online: None) -> None: +async def test_live_btc_and_eth_carry_their_quote_time(coingecko: None) -> None: """ETH asked about as "ether" is not refused, and each price is dated. One federation for both, as in the running bot: CoinGecko's keyless tier From 68dd3ffc4d81abf723358193d96425ec71272586 Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Tue, 29 Sep 2026 11:50:26 -0300 Subject: [PATCH 3/4] Probe CoinGecko's price endpoint, not /ping, before the live test CoinGecko answers /ping with 200 while refusing /simple/price with a 403 from CloudFront, so the probe let the test run into the refusal. --- tests/integration/test_market_live.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_market_live.py b/tests/integration/test_market_live.py index c430a73..69987aa 100644 --- a/tests/integration/test_market_live.py +++ b/tests/integration/test_market_live.py @@ -23,6 +23,7 @@ import pytest import pytest_asyncio +from chatmemory.adapters.market.coingecko import COINGECKO_ENDPOINT from chatmemory.adapters.market.frankfurter import FRANKFURTER_ENDPOINT from chatmemory.adapters.market.registration import MarketToolsConfig, build_market_tools from chatmemory.adapters.mcp_client.client import connect @@ -57,7 +58,7 @@ async def online() -> AsyncIterator[None]: yield -#: Statuses CoinGecko's keyless tier answers when it refuses a caller outright +#: Statuses CoinGecko's keyless tier answers when it refuses a caller outright on the price endpoint (it still answers /ping) #: (a CI runner's shared IP, or a burst). Our code cannot be tested against a #: provider that will not answer, so the crypto test skips rather than fails. PROVIDER_REFUSED = frozenset({403, 429}) @@ -66,7 +67,9 @@ async def online() -> AsyncIterator[None]: @pytest_asyncio.fixture async def coingecko(online: None) -> AsyncIterator[None]: async with httpx.AsyncClient(timeout=5.0) as client: - ping = await client.get("https://api.coingecko.com/api/v3/ping") + ping = await client.get( + COINGECKO_ENDPOINT, params={"ids": "bitcoin", "vs_currencies": "usd"} + ) if ping.status_code in PROVIDER_REFUSED: pytest.skip(f"CoinGecko refuses this runner: HTTP {ping.status_code}") yield From cafec1a6a95db495d1a9245b31318d4b92d4656e Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Tue, 29 Sep 2026 11:52:40 -0300 Subject: [PATCH 4/4] Wrap the CoinGecko refusal comment --- tests/integration/test_market_live.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_market_live.py b/tests/integration/test_market_live.py index 69987aa..7a2d77e 100644 --- a/tests/integration/test_market_live.py +++ b/tests/integration/test_market_live.py @@ -58,8 +58,9 @@ async def online() -> AsyncIterator[None]: yield -#: Statuses CoinGecko's keyless tier answers when it refuses a caller outright on the price endpoint (it still answers /ping) -#: (a CI runner's shared IP, or a burst). Our code cannot be tested against a +#: Statuses CoinGecko's keyless tier answers when it refuses a caller outright +#: on the price endpoint, while /ping still answers 200 (a CI runner's shared +#: IP, or a burst). Our code cannot be tested against a #: provider that will not answer, so the crypto test skips rather than fails. PROVIDER_REFUSED = frozenset({403, 429})