Skip to content

Fix: raise app_loader integration-test timeout to 60s to stop CI flake - #256

Closed
AmaadMartin wants to merge 1 commit into
mainfrom
fix/app-loader-test-execution-timeout
Closed

Fix: raise app_loader integration-test timeout to 60s to stop CI flake#256
AmaadMartin wants to merge 1 commit into
mainfrom
fix/app-loader-test-execution-timeout

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

No existing issue.

  1. Or, if no issue exists, describe the change:

Problem: The validation workflow is a required check and runs the test suite on a
three-way OS matrix — ubuntu-latest, windows-latest, macos-latest
(.github/workflows/validation.yaml:17). The app_loader integration tests fail
intermittently on the Windows and macOS runners with:

Test timed out in 40000ms.

The failing case is AgentLoader discovery and loading integration > should discover apps vs agents across directories and standalone files. It is a runner-speed flake, not a
product defect: ubuntu-latest passes consistently on the same commit, re-runs of the
same commit fail on a different OS each time, and the case itself is deterministic — it
calls listApps() / listAgents() and asserts on fixed name lists.

The reason this particular case is the slow one is that it is the first test in its
describe block to touch the loader, so it absorbs the entire cold-start cost of
discovery. AgentLoader.listApps() calls preloadAgents(), which fans out over every
discovered entrypoint with Promise.all (dev/src/utils/agent_loader.ts:490). The
discovery fixture has four entrypoints (service_alpha/app.ts, service_beta/agent.ts,
standalone_agent.ts, standalone_app.ts), and because DEFAULT_AGENT_FILE_OPTIONS sets
bundle: true (dev/src/utils/agent_loader.ts:82-85), each one runs a full
esbuild.build() of the entrypoint plus its @google/adk dependency graph
(dev/src/utils/agent_loader.ts:181) and then dynamically imports the result. The later
cases in the block reuse the cached AgentFile instances and are cheap.

So the flaking case performs four concurrent esbuild bundles, four large dynamic imports,
and the associated temp-dir/symlink filesystem work. That fits within 40s on the Linux
runner but has no headroom for the Windows runner (slow NTFS metadata operations,
on-access AV scanning of freshly written bundle output) or the macOS runner (fewer, slower
vCPUs).

Solution: Raise the file's single shared TEST_EXECUTION_TIMEOUT from 40000 to
60000.

Why 60000 specifically: it is already the established budget for the subprocess-heavy
integration tests in this repository, so this makes app_loader_test.ts consistent rather
than inventing a new number.

File Constant Value
tests/integration/skills/script_js/agent_test.ts:15 TEST_EXECUTION_TIMEOUT 60000
tests/integration/a2a/stream/stream_test.ts:13 TEST_TIMEOUT 60000
tests/integration/a2a/input_required/input_required_test.ts:13 TEST_TIMEOUT 60000
tests/integration/test_api_server.ts:25 DEFAULT_TIMEOUT 60000

tests/integration/skills/script_js/agent_test.ts is the closest structural analogue —
same execAsync('npm install') in beforeAll, same spawn('npm', ['run', 'start'])
subprocess driven through the shared sendInput helper, same single shared constant
applied to both hooks and tests — and it already uses 60000.

Because the constant is the file's only budget, this lifts all eight call sites at once
(four hooks, four tests). That is intended: the hooks run npm install and recursive
node_modules teardown, which are exactly the operations that are slowest on the Windows
runner, so giving them the same headroom stops the flake from simply relocating from the
test body into a hook.

Scope notes:

  • No assertion is changed, weakened, skipped or retried. There is no it.skip,
    it.retry, retry: or bail:. The hook and test bodies are byte-identical to main.
  • No production code changes — nothing under core/src/, dev/src/ or
    integrations/src/. In particular AgentLoader is deliberately not "optimized"
    (e.g. lazy preloadAgents(), disabling bundle, caching esbuild output); that would be
    a behavioural change to shipped code and needs its own design and review.
  • vitest.config.ts and .github/workflows/validation.yaml are untouched.
  • No dependency added or removed, so package.json / package-lock.json are untouched.
  • Coverage is unaffected: instrumentation is scoped to core/src/**, dev/src/** and
    integrations/src/** (vitest.config.ts:109-113), and a test file is not instrumented.

Trade-off: a genuinely hung app_loader test now takes 60s instead of 40s to surface.
There is no cost on the passing path — Vitest timeouts are ceilings, not sleeps, so a test
that completes in 12s still completes in 12s and green-run CI duration is unchanged.

The diff is one file, +3 −1.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:

[x] I have added or updated unit tests for my change.

No new unit test was added, and that is a deliberate determination rather than an
omission: this change adds zero lines of production code, so there is nothing to cover. A
meta-test asserting TEST_EXECUTION_TIMEOUT === 60000 would test the edit rather than any
behaviour, so it was intentionally not written. The existing integration tests in the
modified file are the test plan, and all of them still pass unmodified.

[x] All unit tests pass locally.

Targeted run from the repository root, on the exact commit pushed:

npm install
npm run build
npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts

Result — all 6 cases pass (3 parameterized App entrypoint with %s cases for app_ts /
app_js / app_default, plus the 3 AgentLoader discovery and loading integration
cases):

 ✓ tests/integration/app_loader/app_loader_test.ts (6 tests) 48824ms
   ✓ App entrypoint with app_ts > should run app via package.json start script ...  6672ms
   ✓ App entrypoint with app_js > should run app via package.json start script ...  7261ms
   ✓ App entrypoint with app_default > should run app via package.json start ...    6746ms
   ✓ AgentLoader discovery ... > should discover apps vs agents across ...         16580ms
 Test Files  1 passed (1)
      Tests  6 passed (6)

The measurement that motivates the change. Locally the flaking discovery case takes
16.6s (19.9s on a second run). The decisive number, though, comes from CI on this
branch — on windows-latest that same case took:

✓ AgentLoader discovery and loading integration
    > should discover apps vs agents across directories and standalone files   34689ms   (run 2)
    > should discover apps vs agents across directories and standalone files   34803ms   (run 3)

~34.7s against the old 40s budget — 87% of the budget consumed, ~5.3s of headroom
and that figure is highly reproducible across runs (34689ms / 34803ms), so it is a stable
property of the Windows runner rather than a one-off spike. Any ordinary variance (AV
scanning, disk contention, a noisy runner) pushes it over the line, which is exactly the
observed intermittent failure. The whole file takes ~69s on Windows versus ~48s locally.
At 60s the same case has ~25s of headroom (58% of budget used), which is the margin this
PR buys.

Also run on the same commit:

npm run lint          # passes
npm run format:check  # passes
git status --porcelain  # clean; no fixture node_modules/ or package-lock.json

Per the repository JS guidelines the full suite (npm test / npm run test:coverage) was
deliberately not run locally; the full matrix run is the CI workflow's job.

Manual End-to-End (E2E) Tests:

Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

The authoritative verification is the CI matrix itself, because the bug only manifests on
the slower hosted runners:

  1. Confirm all three validation jobs are green for the head commit:
    run-tests (ubuntu-latest), run-tests (windows-latest), run-tests (macos-latest).
  2. Because the flake is intermittent, one green run is weak evidence — re-run the workflow
    on the same head commit at least twice more and confirm all three OS jobs stay green.

Observed on this branch:

  • Run 1 — all three OS jobs green (ubuntu-latest 5m24s, macos-latest 6m56s,
    windows-latest 7m52s). The ~45% wall-clock spread between the Linux and Windows
    runners is the runner-speed gap this fix accounts for.
  • Run 2 (same head commit) — ubuntu-latest and macos-latest green.
    app_loader_test.ts passed on all three OSes, including Windows (the 34.7s
    measurement above). The Windows job as a whole went red, but on an unrelated,
    pre-existing flake in a different file
    :
    tests/integration/tools/run_skill_script_tool_test.ts > successfully executes a real Python skill script, which timed out at 5000ms — Vitest's default budget, i.e.
    that test declares no explicit timeout at all. It is outside the scope of this PR and is
    deliberately not touched here; widening this PR to cover it would only blur the change.
  • Run 3 (same head commit, Windows job re-run) — green. The unrelated
    run_skill_script_tool_test.ts flake did not recur, confirming it is intermittent and
    independent of this change. app_loader_test.ts again passed on Windows, with the
    discovery case at 34803ms.

Net result across the three runs: app_loader_test.ts passed on all three OSes in every
run
, and the only red job was caused by a different file.

To reproduce the original failure locally as a deterministic proxy, temporarily lower
TEST_EXECUTION_TIMEOUT (e.g. to 5000) and re-run the command above: the identical
Test timed out in Nms message appears, showing the test is slow, not stuck.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

The discovery case in app_loader_test.ts is the first test to touch the
loader, so it absorbs the whole cold-start cost of AgentLoader discovery:
listApps() calls preloadAgents(), which eagerly loads all four discovered
entrypoints in parallel, each running a full esbuild bundle plus a dynamic
import of the result.

That fits in the 40s budget on ubuntu-latest but intermittently overruns it
on the slower windows-latest and macos-latest runners, failing the required
validation workflow with "Test timed out in 40000ms".

Raise the file's single shared TEST_EXECUTION_TIMEOUT to 60000, matching the
budget the other subprocess-heavy integration tests already use (e.g.
tests/integration/skills/script_js/agent_test.ts). No assertion is changed
and no test is skipped or retried.
This was referenced Jul 29, 2026
@AmaadMartin

Copy link
Copy Markdown
Owner Author

Close. Raising the budget to 60s hides the cost instead of moving it, and the discovery test body runs in about 4 ms once #506 lands. This is closed on approach, independent of which PR lands.

@AmaadMartin AmaadMartin closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant