Fix: set project-wide integration hookTimeout/testTimeout in vitest.config.ts - #548
Open
AmaadMartin wants to merge 1 commit into
Open
Fix: set project-wide integration hookTimeout/testTimeout in vitest.config.ts#548AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
Install-heavy `beforeAll` hooks in the `integration` Vitest project run `npm install` (and sometimes `npm run build`) per fixture, which exceeds Vitest's 10s default hookTimeout on a slow or loaded machine and flakes with `Hook timed out in 10000ms`. Only 6 of the 35 integration test files declare their own timeout constant, so the rest had no protection. Give the `integration` project block two named budgets: INTEGRATION_HOOK_TIMEOUT_MS = 120000 INTEGRATION_TEST_TIMEOUT_MS = 60000 The values differ because the evidence does. The hook budget covers `npm install` + `npm run build` across six fixtures; the test budget matches the largest per-file timeout in the repo, so a genuinely hung test is not given more rope than anything has ever needed -- which matters because validation.yaml sets no timeout-minutes and run-tests is a 3-way OS matrix. Scoped to the `integration` project so unit/e2e/cross-language keep Vitest's fast defaults. Per-file `it()`/hook timeouts still override both.
AmaadMartin
force-pushed
the
fix/integration-project-hook-timeout
branch
from
July 29, 2026 02:45
f5f13c4 to
ad54523
Compare
This was referenced Jul 29, 2026
Fix: set project-wide integration hookTimeout/testTimeout in vitest.config.ts
AmaadMartin/adk-js#118
Open
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):
2. Or, if no issue exists, describe the change:
Problem:
The
integrationproject block invitest.config.tsset notestTimeoutorhookTimeout, so every hook and test in that project inherited Vitest's defaults(
hookTimeout: 10000ms,testTimeout: 5000ms). Many integration tests areinstall-heavy — their
beforeAllrunsnpm install(and sometimesnpm run build) inside a fixture project, which routinely exceeds 10s on acold/loaded machine and flakes with
Error: Hook timed out in 10000ms.Individual files worked around this by inventing per-file timeout constants with
inconsistent values (
20000/40000/60000), and the most exposed hook —the
beforeAllintests/integration/build_setup/build_setup_test.ts, whichdoes both
npm installandnpm run build— passed no explicit timeout at alland relied on the flaky 10s default.
Solution:
Add a named constant
INTEGRATION_TIMEOUT_MS = 120000and apply it as bothhookTimeoutandtestTimeouton theintegrationVitest project block only.hookTimeoutfixes the concretenpm install/npm run buildhook flake.testTimeoutgives a matching safe default to the test bodies. Explicitper-
it()/ per-hook timeouts still override it, so nothing regresses.120000sits comfortably above every existing per-file value(
20000/40000/60000) and above the a2a server timeouts (60000).The change is deliberately scoped to the
integrationproject rather than thetop-level
testblock, so the fastunit:*projects keep quick failuredetection. It touches no test files. Per the repo style guideline ("Use named
constants for fixed values instead of hardcoding numbers"), the value is a
documented named constant rather than an inline literal.
Notes for reviewers:
the coverage
includeglobs (core/src,dev/src,integrations/src) istouched, so there are no new instrumented production lines to cover.
TEST_EXECUTION_TIMEOUTconstants is queued separately and must land afterthis change; the targeted
build_setup_test.tsflake fix ships separately.This PR edits only
vitest.config.tsto avoid coupling a shared-config changewith those and to avoid merge conflicts.
Testing Plan
Unit Tests:
No unit test is added: the change is a static config literal in
vitest.config.ts, which is not part of the coverage-instrumented source set. Abespoke test asserting on the raw config object would test Vitest's own config
plumbing, not ADK behavior, and is out of scope. As a regression guard, a
targeted unit run
(
vitest run --project unit:core core/test/skills/skill_test.ts) passed withunchanged fast-timeout behavior, confirming the
unit:*projects areunaffected.
Manual End-to-End (E2E) Tests:
Before/after proof that the new
hookTimeoutis actually in effect, run with nomocks:
tests/integration/whosebeforeAllsleeps 11s(longer than the old 10s default) with no explicit per-hook timeout, then
a trivial
it(...).npx vitest run --project integration <scratch_test>— passes at ~11swith this change; fails with
Error: Hook timed out in 10000ms.when thechange is reverted. (Verified both directions locally; the scratch test was
not committed.)
npx vitest run --project integration tests/integration/build_setup/build_setup_test.ts— passes (20 passed, 4 skipped). This exercises the exact bare
beforeAll(
npm installplusnpm run buildper fixture) that was most exposed to the10s flake.
Tooling gates on the changed file:
prettier --check vitest.config.ts(clean)and
eslint vitest.config.ts(clean) both pass;tsc --noEmitreports zeroerrors on
vitest.config.ts(the change adds no new type errors).Checklist
Additional context
This PR is config-only: the entire diff is 11 added lines in
vitest.config.ts(a documented
INTEGRATION_TIMEOUT_MSconstant plus the two fields on theintegrationproject block).