diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 7ff244aca8..4279d9f243 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -3,6 +3,9 @@ ### Required Setup `vitest.config.ts` MUST include `smrtVitestPlugin()` in plugins array. Without it: "No field metadata" errors. +### Timeouts +Never set `testTimeout` without also setting `hookTimeout` — hooks do NOT inherit it and stay on vitest's 10s default. Symptom: the file fails with "Hook timed out in 10000ms" while every test reports *skipped*. Budget for where the work happens: setup that spawns processes or generates code is bounded by `hookTimeout`, not `testTimeout`. + ### Database - Use real in-memory SQLite for SmrtObject/SmrtCollection tests - `createIsolatedTestDb()` for transaction-isolated tests (rolls back on cleanup) diff --git a/TESTING_STANDARD.md b/TESTING_STANDARD.md index 8479920397..d63d486314 100644 --- a/TESTING_STANDARD.md +++ b/TESTING_STANDARD.md @@ -138,6 +138,31 @@ No field metadata found for 'Document'. **Note on watch mode**: The manifest is generated once at vitest startup. If you add new classes or fields while vitest is running in watch mode, restart vitest to pick up the changes. +### ⚠️ Never raise `testTimeout` without raising `hookTimeout` + +`testTimeout` and `hookTimeout` are independent. Raising only `testTimeout` +leaves every `beforeAll`/`beforeEach`/`afterAll`/`afterEach` on vitest's **10s +default**, so a slow hook fails the whole file while the test bodies report as +*skipped* — a failure mode that looks nothing like a timeout in the test you +were budgeting for. + +```typescript +test: { + testTimeout: 30000, + hookTimeout: 30000, // match testTimeout; hooks do not inherit it +} +``` + +This has bitten the repo repeatedly: smrt-svelte's teardown hook (#1426), and +`mcp-conformance-fixture`, whose `beforeAll` generates a server and spawns two +`tsx` children — it measured 10,022ms against the 10,000ms default and ejected +a PR from the merge queue (#2238). Packages run in shared CI shards, so a hook +that overruns takes unrelated PRs down with it. + +Budget by **where the work happens**, not by where the assertions are: if setup +does the expensive work (spawning processes, generating code, building +bundles), `hookTimeout` is the limit that matters. + ### Watch Mode ```bash diff --git a/packages/mcp-conformance-fixture/vitest.config.ts b/packages/mcp-conformance-fixture/vitest.config.ts index d536043279..4a5371e8cb 100644 --- a/packages/mcp-conformance-fixture/vitest.config.ts +++ b/packages/mcp-conformance-fixture/vitest.config.ts @@ -3,5 +3,16 @@ import { smrtVitestPlugin } from '../vitest/src/index.ts'; export default defineConfig({ plugins: [smrtVitestPlugin()], - test: { environment: 'node', testTimeout: 240_000 }, + test: { + environment: 'node', + testTimeout: 240_000, + // Match testTimeout. Nearly all of this suite's work happens in `beforeAll`, + // not in the test bodies: it regenerates the MCP server with `MCPGenerator`, + // spawns a `tsx` child over stdio to negotiate the protocol and list tools, + // then spawns a second `tsx` child for the HTTP adapter. Left at vitest's + // 10s `hookTimeout` default that hook measured 10,022ms on a loaded runner + // and ejected PR #2230 from the merge queue (#2238). This package sits in + // the shared `test-packages` shard, so the miss was not local to that PR. + hookTimeout: 240_000, + }, });