From 0d30805211f7fc46f3005c72c0709564921578d4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 20:01:31 -0700 Subject: [PATCH 1/8] chore(tests): ratchet real wall-clock waits in unit tests (check:test-patterns real-wait) New `real-wait` rule in scripts/check-test-patterns.ts flags a unit test that waits on the wall clock with no fake timers in scope: - `sleep`/`interruptibleSleep` resolved through their `@sim/utils/helpers` import binding (aliases included; skipped when the file mocks that module), - `setTimeout` from `node:timers/promises`, - `new Promise((resolve) => setTimeout(resolve, ms))` and variants whose timer callback resolves the promise (reject-only timeout guards are not waits). Zero or omitted delays are macrotask flushes and are not flagged. Fake timers cover a wait when an earlier `vi.useFakeTimers()` in an enclosing function or the module, or a `beforeEach`/`beforeAll` of an enclosing describe or the module, fakes `setTimeout` (`toFake: ['Date']` and `shouldAdvanceTime: true` do not). Integration and *.live.test.ts files are exempt: their waits are on real services. Each occurrence is a numbered entry, capping the count per file. `--update --init-rule ` admits the one-time baseline for a rule the baseline does not track yet; `--update` stays shrink-only. Baseline: 121 real-wait entries across 46 files (72 -> 193 total). --- .claude/rules/sim-testing.md | 1 + .cursor/rules/sim-testing.mdc | 1 + scripts/check-test-patterns.test.ts | 138 +++++++++++++++- scripts/check-test-patterns.ts | 239 +++++++++++++++++++++++++++- scripts/test-patterns-baseline.json | 121 ++++++++++++++ 5 files changed, 498 insertions(+), 2 deletions(-) diff --git a/.claude/rules/sim-testing.md b/.claude/rules/sim-testing.md index 5e84f540ab9..0133eb7e717 100644 --- a/.claude/rules/sim-testing.md +++ b/.claude/rules/sim-testing.md @@ -169,6 +169,7 @@ The suite's wall time is bound by the single Vite server thread that serves ever 3. Mock heavy graphs a test does not need and the setup does not already mock: `@/blocks`, `@/triggers/registry`, `@/tools/generated/*`. 4. No real timers: `vi.useFakeTimers()`, `flushMicrotasks()`, or `flushMacrotask()`. + `check:test-patterns` (`real-wait`) fails on a new wall-clock wait in a unit test. 5. Absolute imports only. ### Running diff --git a/.cursor/rules/sim-testing.mdc b/.cursor/rules/sim-testing.mdc index 6ef5cf6e8b5..8b13fd92250 100644 --- a/.cursor/rules/sim-testing.mdc +++ b/.cursor/rules/sim-testing.mdc @@ -167,6 +167,7 @@ The suite's wall time is bound by the single Vite server thread that serves ever 3. Mock heavy graphs a test does not need and the setup does not already mock: `@/blocks`, `@/triggers/registry`, `@/tools/generated/*`. 4. No real timers: `vi.useFakeTimers()`, `flushMicrotasks()`, or `flushMacrotask()`. + `check:test-patterns` (`real-wait`) fails on a new wall-clock wait in a unit test. 5. Absolute imports only. ### Running diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 09ffb4106b6..f01f99a17d0 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { findStubAndHookViolations, parseSource } from './check-test-patterns' +import { + findRealWaitViolations, + findStubAndHookViolations, + parseSource, +} from './check-test-patterns' function rules(source: string, file = 'apps/sim/lib/example.test.ts') { return findStubAndHookViolations(file, parseSource(file, source)).map( @@ -76,3 +80,135 @@ describe('redundant-hook', () => { ]) }) }) + +describe('real-wait', () => { + const HELPERS = "import { sleep, interruptibleSleep } from '@sim/utils/helpers'" + + function waits(source: string, file = 'apps/sim/lib/example.test.ts') { + return findRealWaitViolations(file, parseSource(file, source)).map(({ detail }) => detail) + } + + it.each<[string, string[], string[]]>([ + ['an imported sleep', [HELPERS, "it('t', async () => { await sleep(10) })"], ['sleep() #1']], + [ + 'an aliased sleep', + ["import { sleep as wait } from '@sim/utils/helpers'", 'await wait(10)'], + ['sleep() #1'], + ], + [ + 'interruptibleSleep', + [HELPERS, 'await interruptibleSleep(10, signal)'], + ['interruptibleSleep() #1'], + ], + ['a sleep with a computed delay', [HELPERS, 'await sleep(delayMs)'], ['sleep() #1']], + [ + 'timers/promises setTimeout', + ["import { setTimeout as delay } from 'node:timers/promises'", 'await delay(50)'], + ['timers/promises setTimeout() #1'], + ], + [ + 'a promise resolved by setTimeout', + ['await new Promise((r) => setTimeout(r, 5))'], + ['setTimeout promise #1'], + ], + [ + 'a promise resolved from a setTimeout callback', + ['await new Promise((resolve) => { globalThis.setTimeout(() => resolve(), ms) })'], + ['setTimeout promise #1'], + ], + [ + 'fake timers that leave setTimeout real', + [HELPERS, "beforeEach(() => { vi.useFakeTimers({ toFake: ['Date'] }) })", 'await sleep(10)'], + ['sleep() #1'], + ], + [ + 'fake timers that advance with the wall clock', + [ + HELPERS, + "it('t', async () => {", + ' vi.useFakeTimers({ shouldAdvanceTime: true })', + ' await sleep(10)', + '})', + ], + ['sleep() #1'], + ], + [ + 'a sleep before the test installs fake timers', + [HELPERS, "it('t', async () => {", ' await sleep(10)', ' vi.useFakeTimers()', '})'], + ['sleep() #1'], + ], + [ + 'a sleep in a test beside one that installs fake timers', + [ + HELPERS, + "it('a', () => { vi.useFakeTimers() })", + "it('b', async () => { await sleep(10) })", + ], + ['sleep() #1'], + ], + [ + 'every occurrence, numbered per kind', + [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], + ['sleep() #1', 'setTimeout promise #1', 'sleep() #2'], + ], + ])('flags %s', (_name, lines, expected) => { + expect(waits(lines.join('\n'))).toEqual(expected) + }) + + it.each<[string, string[]]>([ + [ + 'a sleep not imported from @sim/utils/helpers', + ["import { sleep } from './helpers'", 'await sleep(10)'], + ], + [ + 'a mocked @sim/utils/helpers', + [HELPERS, "vi.mock('@sim/utils/helpers', () => utilsHelpersMock)", 'await sleep(10)'], + ], + ['a zero-delay sleep', [HELPERS, 'await sleep(0)']], + [ + 'a zero-delay setTimeout promise', + ['await new Promise((r) => setTimeout(r, 0))', 'await new Promise((r) => setTimeout(r))'], + ], + [ + 'a setTimeout that only rejects', + [ + "await new Promise((resolve, reject) => setTimeout(() => reject(new Error('timeout')), 50))", + ], + ], + [ + 'a sleep after the test installs fake timers', + [ + HELPERS, + "it('t', async () => {", + ' vi.useFakeTimers()', + ' const done = sleep(10)', + ' await vi.advanceTimersByTimeAsync(10)', + ' await done', + '})', + ], + ], + [ + 'a sleep in a describe whose beforeEach installs fake timers', + [ + HELPERS, + "describe('d', () => {", + " beforeEach(() => { vi.useFakeTimers({ toFake: ['setTimeout'] }) })", + " it('t', async () => { await sleep(10) })", + '})', + ], + ], + [ + 'a sleep under a module-level beforeAll that installs fake timers', + [HELPERS, "it('t', async () => { await sleep(10) })", 'beforeAll(() => vi.useFakeTimers())'], + ], + ])('ignores %s', (_name, lines) => { + expect(waits(lines.join('\n'))).toEqual([]) + }) + + it.each(['apps/sim/lib/example.integration.ts', 'apps/sim/tools/example.live.test.ts'])( + 'exempts %s, which waits on real services', + (file) => { + expect(waits([HELPERS, 'await sleep(10)'].join('\n'), file)).toEqual([]) + } + ) +}) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index 97aca30ef3a..edc94f72b17 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -16,6 +16,10 @@ * - `module-scope-stub`: a non-integration test calls `vi.stubGlobal`, `vi.stubEnv` or `vi.spyOn` * at module scope, where the shared config undoes it before the first test. * - `test-dir`: a test lives in a `__tests__/` or `tests/` directory instead of next to its source. + * - `real-wait`: a unit test waits on the wall clock (`sleep`, `new Promise(r => setTimeout(r, ms))`) + * with no fake timers in scope. Integration and `*.live.test.ts` files wait on real Postgres, + * Redis and external services, whose time is real, so the rule skips them; e2e scripts and + * Playwright specs are not `*.test.ts` files and are never scanned. * * A central mock, or a global mock in `apps/sim/vitest.setup.ts`, that `vi.mock`s an `@/…` module * id that no longer resolves fails outright: it mocks nothing, and a dead central `@example` id @@ -23,7 +27,8 @@ * * Existing exceptions are recorded in `scripts/test-patterns-baseline.json`. A new violation fails; * so does a baseline entry that no longer occurs, so the baseline only ever shrinks. Regenerate it - * after removing violations with `bun run scripts/check-test-patterns.ts --update`. + * after removing violations with `bun run scripts/check-test-patterns.ts --update`. A newly added rule + * gets its one-time baseline with `--update --init-rule `. * * Run: `bun run check:test-patterns` */ @@ -199,6 +204,211 @@ export function findStubAndHookViolations( return violations } +const FUNCTION_TYPES = new Set([ + 'ArrowFunctionExpression', + 'FunctionExpression', + 'FunctionDeclaration', + 'ObjectMethod', + 'ClassMethod', +]) + +/** Like `walk`, but hands each node its ancestors, nearest last. */ +function walkWithAncestors( + node: unknown, + visit: (node: Node, ancestors: Node[]) => void, + ancestors: Node[] = [] +): void { + if (!node || typeof node !== 'object') return + if (Array.isArray(node)) { + for (const child of node) walkWithAncestors(child, visit, ancestors) + return + } + const current = node as Node + const isNode = typeof current.type === 'string' + if (isNode) visit(current, ancestors) + const next = isNode ? [...ancestors, current] : ancestors + for (const [key, value] of Object.entries(current)) { + if (key === 'loc' || key === 'start' || key === 'end' || key === 'extra') continue + walkWithAncestors(value, visit, next) + } +} + +/** Index of the nearest function among `ancestors[0..before)`, or -1. */ +function nearestFunctionIndex(ancestors: Node[], before: number): number { + for (let index = before - 1; index >= 0; index--) { + if (FUNCTION_TYPES.has(ancestors[index].type)) return index + } + return -1 +} + +function nodeRange(node: Node): [number, number] { + return [node.start as number, node.end as number] +} + +/** A `setTimeout` delay that is a real wait: anything but an omitted or literal `0` delay. */ +function isNonZeroDelay(delay: Node | undefined): boolean { + return !(delay === undefined || (delay.type === 'NumericLiteral' && delay.value === 0)) +} + +/** Whether a `vi.useFakeTimers(config)` call fakes `setTimeout` on a clock that only moves when told. */ +function fakesSetTimeout(call: Node): boolean { + const config = (call.arguments as Node[])[0] + if (config?.type !== 'ObjectExpression') return true + for (const property of config.properties as Node[]) { + if (property.type !== 'ObjectProperty') continue + const key = property.key as { name?: string; value?: string } + const name = key.name ?? key.value + const value = property.value as Node + if (name === 'shouldAdvanceTime' && value.type === 'BooleanLiteral' && value.value) { + return false + } + if ((name === 'toFake' || name === 'toNotFake') && value.type === 'ArrayExpression') { + const listed = (value.elements as Node[]).some( + (element) => element?.type === 'StringLiteral' && element.value === 'setTimeout' + ) + if (name === 'toFake' ? !listed : listed) return false + } + } + return true +} + +/** Whether `node` calls `setTimeout` (bare or as `globalThis.setTimeout`/`window.setTimeout`). */ +function isSetTimeoutCall(node: Node): boolean { + if (node.type !== 'CallExpression') return false + const callee = node.callee as Node + return ( + (callee.type === 'Identifier' && callee.name === 'setTimeout') || + (callee.type === 'MemberExpression' && + (callee.property as { name?: string }).name === 'setTimeout') + ) +} + +function referencesIdentifier(node: Node, name: string): boolean { + if (node.type === 'Identifier') return node.name === name + let found = false + walk(node, (child) => { + if (child.type === 'Identifier' && child.name === name) found = true + }) + return found +} + +/** + * `new Promise((resolve) => setTimeout(resolve, ms))` and its variants: a promise whose executor + * resolves it from a non-zero `setTimeout`. A timer that only rejects is a timeout guard, not a + * wait, and a zero delay is a macrotask flush. + */ +function isTimedPromise(node: Node): boolean { + if ( + node.type !== 'NewExpression' || + (node.callee as Node).type !== 'Identifier' || + (node.callee as { name: string }).name !== 'Promise' + ) { + return false + } + const executor = (node.arguments as Node[])[0] + if (!executor || !FUNCTION_TYPES.has(executor.type)) return false + const resolve = (executor.params as Node[])[0] + if (resolve?.type !== 'Identifier') return false + const resolveName = resolve.name as string + let timed = false + walk(executor.body, (child) => { + if (!isSetTimeoutCall(child)) return + const [callback, delay] = child.arguments as Node[] + if (callback && isNonZeroDelay(delay) && referencesIdentifier(callback, resolveName)) { + timed = true + } + }) + return timed +} + +/** + * `real-wait` violations in one unit test file: a call that waits on the wall clock — `sleep` or + * `interruptibleSleep` from `@sim/utils/helpers`, `setTimeout` from `node:timers/promises`, or a + * promise resolved by `setTimeout` — with a non-zero delay and no `vi.useFakeTimers()` in scope. + * + * Vitest's fake clock fakes each of these whenever it fakes `setTimeout`, and then the wait costs + * no wall-clock time. Fake timers are in scope when the call sits inside a function (or + * the module) that installed them in an earlier statement, or inside a `describe` (or the module) + * whose `beforeEach`/`beforeAll` installs them. A file that mocks `@sim/utils/helpers` gets the + * mock's `sleep`, so its imports are not real waits. Each occurrence is its own numbered entry, so + * the baseline caps the count per file. + */ +export function findRealWaitViolations( + file: string, + program: ReturnType +): Violation[] { + if (file.endsWith('.integration.ts') || file.endsWith('.live.test.ts')) return [] + const waitImports = new Map() + let helpersMocked = false + walk(program, (node) => { + if (mockedId(node) === '@sim/utils/helpers') helpersMocked = true + }) + for (const statement of program.body as Node[]) { + if (statement.type !== 'ImportDeclaration') continue + const source = (statement.source as { value: string }).value + const exported = + source === '@sim/utils/helpers' && !helpersMocked + ? ['sleep', 'interruptibleSleep'] + : source === 'node:timers/promises' || source === 'timers/promises' + ? ['setTimeout'] + : [] + for (const specifier of statement.specifiers as Node[]) { + if (specifier.type !== 'ImportSpecifier') continue + const imported = specifier.imported as { name?: string; value?: string } + const name = imported.name ?? imported.value ?? '' + if (!exported.includes(name)) continue + const label = name === 'setTimeout' ? 'timers/promises setTimeout()' : `${name}()` + waitImports.set((specifier.local as { name: string }).name, label) + } + } + + const programRange: [number, number] = [program.start ?? 0, program.end ?? 0] + const fakeClocks: Array<{ scope: [number, number]; after: number }> = [] + const waits: Array<{ node: Node; label: string }> = [] + walkWithAncestors(program, (node, ancestors) => { + if (isViCall(node, 'useFakeTimers') && fakesSetTimeout(node)) { + const fnIndex = nearestFunctionIndex(ancestors, ancestors.length) + const fn = ancestors[fnIndex] + const hook = ancestors[fnIndex - 1] + const hookName = + hook?.type === 'CallExpression' && (hook.callee as Node).type === 'Identifier' + ? (hook.callee as { name: string }).name + : undefined + if ( + fn && + (hookName === 'beforeEach' || hookName === 'beforeAll') && + (hook.arguments as Node[])[0] === fn + ) { + const owner = ancestors[nearestFunctionIndex(ancestors, fnIndex - 1)] + fakeClocks.push({ scope: owner ? nodeRange(owner) : programRange, after: -1 }) + } else { + fakeClocks.push({ scope: fn ? nodeRange(fn) : programRange, after: node.start as number }) + } + return + } + if (node.type === 'CallExpression' && (node.callee as Node).type === 'Identifier') { + const label = waitImports.get((node.callee as { name: string }).name) + if (label && isNonZeroDelay((node.arguments as Node[])[0])) waits.push({ node, label }) + return + } + if (isTimedPromise(node)) waits.push({ node, label: 'setTimeout promise' }) + }) + + const counts = new Map() + const violations: Violation[] = [] + for (const { node, label } of waits) { + const start = node.start as number + const faked = fakeClocks.some( + ({ scope, after }) => scope[0] <= start && start < scope[1] && start > after + ) + if (faked) continue + const count = (counts.get(label) ?? 0) + 1 + counts.set(label, count) + violations.push({ file, rule: 'real-wait', detail: `${label} #${count}` }) + } + return violations +} + function mockedId(node: Node): string | undefined { if (!isViCall(node, 'mock')) return undefined const first = (node.arguments as Node[])[0] @@ -348,6 +558,7 @@ function collect(globals: Set, declaredCentralIds: Set): Violati } violations.push(...findStubAndHookViolations(file, program)) + violations.push(...findRealWaitViolations(file, program)) walk(program, (node) => { const id = mockedId(node) @@ -399,6 +610,25 @@ function readBaseline(current: string[]): string[] { process.exit(1) } +/** + * `--update --init-rule ` admits every current violation of a rule that has no baseline + * entry yet: the one-time baseline for a newly introduced rule. Refuses an unknown rule or one the + * baseline already tracks, so it can never top up an existing rule. + */ +function readNewRuleEntries(current: string[], baseline: Set): string[] { + const flag = process.argv.indexOf('--init-rule') + if (flag === -1) return [] + const rule = process.argv[flag + 1] + const tracked = [...baseline].some((entry) => entry.startsWith(`${rule}\t`)) + if (!rule || !Object.keys(FIX).includes(rule) || tracked) { + console.error( + `✗ --init-rule ${rule ?? ''}: ${tracked ? 'the baseline already tracks this rule' : 'unknown rule'}.` + ) + process.exit(1) + } + return current.filter((entry) => entry.startsWith(`${rule}\t`)) +} + /** How to fix each rule, printed once per rule with new violations. */ const FIX: Record = { 'global-remock': 'drive the global mock through its knobs instead of re-mocking it', @@ -408,6 +638,10 @@ const FIX: Record = { 'module-scope-stub': 'move the stub into a beforeEach or the test; the shared config undoes it before the first test', 'test-dir': 'move the test next to its source file', + 'real-wait': + 'replace the wall-clock wait: vi.useFakeTimers() + await vi.advanceTimersByTimeAsync(ms) for ' + + 'time-based code, await vi.waitFor(() => …) for a condition, or flushMicrotasks()/flushMacrotask() ' + + 'from @sim/testing to let pending work settle', } /** Prints each new violation, then the fix for every rule involved. */ @@ -446,6 +680,9 @@ function main(): void { const current = [...new Set(violations.map(key))].sort() const baseline = new Set(readBaseline(current)) + if (process.argv.includes('--update')) { + for (const entry of readNewRuleEntries(current, baseline)) baseline.add(entry) + } const added = current.filter((entry) => !baseline.has(entry)) if (process.argv.includes('--update')) { diff --git a/scripts/test-patterns-baseline.json b/scripts/test-patterns-baseline.json index faeda51cc7b..336ba63d942 100644 --- a/scripts/test-patterns-baseline.json +++ b/scripts/test-patterns-baseline.json @@ -70,5 +70,126 @@ "local-factory\tapps/sim/serializer/index.edge-case-blocks.test.ts\t@/blocks", "local-factory\tapps/sim/serializer/private-inputs.test.ts\t@/blocks", "local-factory\tapps/sim/tools/assistant-execution.test.ts\t@/tools/utils", + "real-wait\tapps/desktop/src/main/browser-import/chromium-cookies.test.ts\tsleep() #1", + "real-wait\tapps/desktop/src/main/desktop-executor/executor.test.ts\tsleep() #1", + "real-wait\tapps/desktop/src/main/desktop-executor/executor.test.ts\tsleep() #2", + "real-wait\tapps/desktop/src/main/desktop-executor/executor.test.ts\tsleep() #3", + "real-wait\tapps/desktop/src/main/desktop-executor/executor.test.ts\tsleep() #4", + "real-wait\tapps/desktop/src/main/desktop-executor/executor.test.ts\tsleep() #5", + "real-wait\tapps/desktop/src/main/desktop-executor/service.test.ts\tsleep() #1", + "real-wait\tapps/desktop/src/main/desktop-executor/service.test.ts\tsleep() #2", + "real-wait\tapps/desktop/src/main/desktop-executor/service.test.ts\tsleep() #3", + "real-wait\tapps/desktop/src/main/desktop-executor/service.test.ts\tsleep() #4", + "real-wait\tapps/desktop/src/main/desktop-executor/service.test.ts\tsleep() #5", + "real-wait\tapps/desktop/src/main/terminal/service.test.ts\tsleep() #1", + "real-wait\tapps/desktop/src/main/terminal/service.test.ts\tsleep() #2", + "real-wait\tapps/desktop/src/main/terminal/tmux.test.ts\tsleep() #1", + "real-wait\tapps/desktop/src/main/terminal/tmux.test.ts\tsleep() #2", + "real-wait\tapps/desktop/src/main/terminal/tmux.test.ts\tsleep() #3", + "real-wait\tapps/realtime/src/handlers/file-doc-store.test.ts\tsleep() #1", + "real-wait\tapps/realtime/src/handlers/file-doc.redis-store.test.ts\tsetTimeout promise #1", + "real-wait\tapps/sim/app/(interfaces)/chat/hooks/use-chat-streaming.test.tsx\tsetTimeout promise #1", + "real-wait\tapps/sim/app/api/v2/chat/route.test.ts\tsleep() #1", + "real-wait\tapps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/dirty-on-open.test.ts\tsleep() #1", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #1", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #10", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #11", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #12", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #13", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #14", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #15", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #16", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #17", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #18", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #19", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #2", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #20", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #21", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #22", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #23", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #24", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #25", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #26", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #27", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #28", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #29", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #3", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #30", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #31", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #32", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #33", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #34", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #35", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #36", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #37", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #38", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #39", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #4", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #40", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #41", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #5", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #6", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #7", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #8", + "real-wait\tapps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx\tsleep() #9", + "real-wait\tapps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts\tsleep() #1", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #1", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #2", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #3", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #4", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #5", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #6", + "real-wait\tapps/sim/executor/execution/engine.test.ts\tsleep() #7", + "real-wait\tapps/sim/hooks/queries/mcp.test.tsx\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/mothership-chat-effort.test.tsx\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/mothership-chats.test.ts\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/mothership-chats.test.ts\tsleep() #2", + "real-wait\tapps/sim/hooks/queries/organization-accounts.test.tsx\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/workflow-mcp-servers.test.tsx\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/workspace-files.test.tsx\tsleep() #1", + "real-wait\tapps/sim/hooks/queries/workspace-files.test.tsx\tsleep() #2", + "real-wait\tapps/sim/hooks/queries/workspace.test.tsx\tsleep() #1", + "real-wait\tapps/sim/lib/billing/core/reporting-usage-cache.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/billing/core/reporting-usage-cache.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/billing/core/usage-gate-cache.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/concurrency/leader-lock.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/concurrency/singleflight.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/core/async-jobs/backends/database.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/core/async-jobs/backends/database.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/core/async-jobs/backends/database.test.ts\tsleep() #3", + "real-wait\tapps/sim/lib/core/utils/background.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/core/utils/background.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/execution/isolated-vm.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/execution/remote-sandbox/e2b-session.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/execution/remote-sandbox/e2b-session.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts\tsleep() #3", + "real-wait\tapps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts\tsleep() #4", + "real-wait\tapps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts\tsleep() #5", + "real-wait\tapps/sim/lib/internal/identity-center/concurrency.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/internal/identity-center/concurrency.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/internal/identity-center/concurrency.test.ts\tsleep() #3", + "real-wait\tapps/sim/lib/knowledge/projection/enqueue-inline.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/knowledge/transfer/export-archive.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/knowledge/transfer/export-archive.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/mcp/oauth/storage.test.ts\tsetTimeout promise #1", + "real-wait\tapps/sim/lib/mcp/storage/memory-cache.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts\tsleep() #3", + "real-wait\tapps/sim/lib/mothership/request/handlers/handlers.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/tools/client/browser-tool-execution.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/tools/client/local-filesystem.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/tools/client/local-filesystem.test.ts\tsleep() #2", + "real-wait\tapps/sim/lib/mothership/tools/client/run-tool-execution.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/mothership/tools/server/files/file-intent-store.test.ts\tsleep() #1", + "real-wait\tapps/sim/lib/table/import.test.ts\tsleep() #1", + "real-wait\tpackages/audit/src/log.test.ts\tsleep() #1", + "real-wait\tpackages/sim-cli/src/commands/protocol/files-get.test.ts\tsleep() #1", + "real-wait\tpackages/sim-cli/src/config/profile.test.ts\tsleep() #1", + "real-wait\tpackages/sim-cli/src/config/profile.test.ts\tsleep() #2", + "real-wait\tpackages/sim-cli/src/embed.test.ts\tsleep() #1", + "real-wait\tpackages/sim-cli/src/http/client.test.ts\tsleep() #1", "redundant-hook\tapps/sim/lib/mothership/tools/hosted-workbench.live.test.ts\tvi.restoreAllMocks()" ] From 819e62dec25775e50803679147baccbb43d45061 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 20:08:39 -0700 Subject: [PATCH 2/8] fix(tests): keep a real-wait flagged before the fake-timer install in its own hook A beforeEach/beforeAll that installs fake timers covered its whole suite, including waits earlier in the same hook body, which run on the real clock. Only waits after the install in that hook are exempt now; tests and other hooks in the suite keep the exemption. --- scripts/check-test-patterns.test.ts | 18 ++++++++++++++++++ scripts/check-test-patterns.ts | 16 +++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index f01f99a17d0..576e5bc7c73 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -146,6 +146,20 @@ describe('real-wait', () => { ], ['sleep() #1'], ], + [ + 'a sleep before the install in the hook that installs fake timers', + [ + HELPERS, + "describe('d', () => {", + ' beforeEach(async () => {', + ' await sleep(100)', + ' vi.useFakeTimers()', + ' })', + " it('t', async () => { await sleep(10) })", + '})', + ], + ['sleep() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], @@ -201,6 +215,10 @@ describe('real-wait', () => { 'a sleep under a module-level beforeAll that installs fake timers', [HELPERS, "it('t', async () => { await sleep(10) })", 'beforeAll(() => vi.useFakeTimers())'], ], + [ + 'a sleep after the install in the hook that installs fake timers', + [HELPERS, 'beforeAll(async () => {', ' vi.useFakeTimers()', ' await sleep(100)', '})'], + ], ])('ignores %s', (_name, lines) => { expect(waits(lines.join('\n'))).toEqual([]) }) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index edc94f72b17..5bfbe29dc36 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -329,7 +329,7 @@ function isTimedPromise(node: Node): boolean { * Vitest's fake clock fakes each of these whenever it fakes `setTimeout`, and then the wait costs * no wall-clock time. Fake timers are in scope when the call sits inside a function (or * the module) that installed them in an earlier statement, or inside a `describe` (or the module) - * whose `beforeEach`/`beforeAll` installs them. A file that mocks `@sim/utils/helpers` gets the + * whose `beforeEach`/`beforeAll` installs them, except before the install in that hook's own body. A file that mocks `@sim/utils/helpers` gets the * mock's `sleep`, so its imports are not real waits. Each occurrence is its own numbered entry, so * the baseline caps the count per file. */ @@ -363,7 +363,8 @@ export function findRealWaitViolations( } const programRange: [number, number] = [program.start ?? 0, program.end ?? 0] - const fakeClocks: Array<{ scope: [number, number]; after: number }> = [] + // `real` is the stretch of the scope that runs before the clock is installed. + const fakeClocks: Array<{ scope: [number, number]; real: [number, number] }> = [] const waits: Array<{ node: Node; label: string }> = [] walkWithAncestors(program, (node, ancestors) => { if (isViCall(node, 'useFakeTimers') && fakesSetTimeout(node)) { @@ -380,9 +381,13 @@ export function findRealWaitViolations( (hook.arguments as Node[])[0] === fn ) { const owner = ancestors[nearestFunctionIndex(ancestors, fnIndex - 1)] - fakeClocks.push({ scope: owner ? nodeRange(owner) : programRange, after: -1 }) + fakeClocks.push({ + scope: owner ? nodeRange(owner) : programRange, + real: [fn.start as number, node.start as number], + }) } else { - fakeClocks.push({ scope: fn ? nodeRange(fn) : programRange, after: node.start as number }) + const scope = fn ? nodeRange(fn) : programRange + fakeClocks.push({ scope, real: [scope[0], node.start as number] }) } return } @@ -399,7 +404,8 @@ export function findRealWaitViolations( for (const { node, label } of waits) { const start = node.start as number const faked = fakeClocks.some( - ({ scope, after }) => scope[0] <= start && start < scope[1] && start > after + ({ scope, real }) => + scope[0] <= start && start < scope[1] && !(real[0] <= start && start < real[1]) ) if (faked) continue const count = (counts.get(label) ?? 0) + 1 From 645d0f07d3c08e3a632528d1511d993a2573d657 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 20:33:00 -0700 Subject: [PATCH 3/8] fix(tests): make real-wait's fake-timer exemption exact and drop --init-rule - Remove `--update --init-rule`: it admitted entries for any rule with no remaining baseline entries. `--update` is strictly shrink-only again; the real-wait entries enter once, in this PR's baseline diff. - `vi.useRealTimers()` ends the exemption from where it runs, including a nested suite restoring real timers after an outer hook faked them. - A clock installed in `beforeEach` no longer covers `beforeAll`/`afterAll` or earlier sibling `beforeEach` hooks; `beforeAll` no longer covers earlier sibling `beforeAll` hooks. - Only `vi.useFakeTimers()` with no config, or a literal config proven to fake setTimeout without shouldAdvanceTime, exempts a wait. - Namespace imports (`helpers.sleep`, `timers.setTimeout`) are flagged. --- scripts/check-test-patterns.test.ts | 93 +++++++++++++ scripts/check-test-patterns.ts | 206 +++++++++++++++++----------- 2 files changed, 219 insertions(+), 80 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 576e5bc7c73..512aed729b2 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -160,6 +160,80 @@ describe('real-wait', () => { ], ['sleep() #1'], ], + [ + 'a sleep after the test restores real timers', + [ + HELPERS, + "it('t', async () => {", + ' vi.useFakeTimers()', + ' vi.useRealTimers()', + ' await sleep(10)', + '})', + ], + ['sleep() #1'], + ], + [ + 'a sleep in a nested suite that restores real timers', + [ + HELPERS, + "describe('outer', () => {", + ' beforeEach(() => { vi.useFakeTimers() })', + " describe('inner', () => {", + ' beforeEach(() => { vi.useRealTimers() })', + " it('t', async () => { await sleep(10) })", + ' })', + '})', + ], + ['sleep() #1'], + ], + [ + 'a sleep in a beforeAll beside a beforeEach that installs fake timers', + [ + HELPERS, + 'beforeEach(() => { vi.useFakeTimers() })', + 'beforeAll(async () => { await sleep(10) })', + ], + ['sleep() #1'], + ], + [ + 'a sleep in a beforeEach declared before the one that installs fake timers', + [ + HELPERS, + 'beforeEach(async () => { await sleep(10) })', + 'beforeEach(() => { vi.useFakeTimers() })', + ], + ['sleep() #1'], + ], + [ + 'fake timers with a config the AST cannot read', + [ + HELPERS, + 'beforeEach(() => { vi.useFakeTimers(config) })', + "it('t', async () => { await sleep(10) })", + ], + ['sleep() #1'], + ], + [ + 'fake timers with a spread config', + [ + HELPERS, + "it('t', async () => {", + ' vi.useFakeTimers({ ...config })', + ' await sleep(10)', + '})', + ], + ['sleep() #1'], + ], + [ + 'namespace imports', + [ + "import * as helpers from '@sim/utils/helpers'", + "import * as timers from 'node:timers/promises'", + 'await helpers.sleep(10)', + 'await timers.setTimeout(10)', + ], + ['sleep() #1', 'timers/promises setTimeout() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], @@ -219,6 +293,25 @@ describe('real-wait', () => { 'a sleep after the install in the hook that installs fake timers', [HELPERS, 'beforeAll(async () => {', ' vi.useFakeTimers()', ' await sleep(100)', '})'], ], + [ + 'a sleep after real timers are restored and fake ones installed again', + [ + HELPERS, + "it('t', async () => {", + ' vi.useRealTimers()', + ' vi.useFakeTimers()', + ' await sleep(10)', + '})', + ], + ], + [ + 'a sleep in an afterEach beside a beforeEach that installs fake timers', + [ + HELPERS, + 'beforeEach(() => { vi.useFakeTimers() })', + 'afterEach(async () => { await sleep(10) })', + ], + ], ])('ignores %s', (_name, lines) => { expect(waits(lines.join('\n'))).toEqual([]) }) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index 5bfbe29dc36..70ae35389d7 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -27,8 +27,7 @@ * * Existing exceptions are recorded in `scripts/test-patterns-baseline.json`. A new violation fails; * so does a baseline entry that no longer occurs, so the baseline only ever shrinks. Regenerate it - * after removing violations with `bun run scripts/check-test-patterns.ts --update`. A newly added rule - * gets its one-time baseline with `--update --init-rule `. + * after removing violations with `bun run scripts/check-test-patterns.ts --update`. * * Run: `bun run check:test-patterns` */ @@ -250,19 +249,26 @@ function isNonZeroDelay(delay: Node | undefined): boolean { return !(delay === undefined || (delay.type === 'NumericLiteral' && delay.value === 0)) } -/** Whether a `vi.useFakeTimers(config)` call fakes `setTimeout` on a clock that only moves when told. */ +/** + * Whether a `vi.useFakeTimers(config)` call provably fakes `setTimeout` on a clock that only moves + * when told: no config, or a literal one whose `toFake`/`toNotFake` keep `setTimeout` faked and + * that does not set `shouldAdvanceTime`. A config the AST cannot read is not proof. + */ function fakesSetTimeout(call: Node): boolean { - const config = (call.arguments as Node[])[0] - if (config?.type !== 'ObjectExpression') return true + const args = call.arguments as Node[] + if (args.length === 0) return true + const config = args[0] + if (args.length > 1 || config.type !== 'ObjectExpression') return false for (const property of config.properties as Node[]) { - if (property.type !== 'ObjectProperty') continue + if (property.type !== 'ObjectProperty' || property.computed) return false const key = property.key as { name?: string; value?: string } const name = key.name ?? key.value const value = property.value as Node - if (name === 'shouldAdvanceTime' && value.type === 'BooleanLiteral' && value.value) { + if (name === 'shouldAdvanceTime' && !(value.type === 'BooleanLiteral' && !value.value)) { return false } - if ((name === 'toFake' || name === 'toNotFake') && value.type === 'ArrayExpression') { + if (name === 'toFake' || name === 'toNotFake') { + if (value.type !== 'ArrayExpression') return false const listed = (value.elements as Node[]).some( (element) => element?.type === 'StringLiteral' && element.value === 'setTimeout' ) @@ -321,91 +327,149 @@ function isTimedPromise(node: Node): boolean { return timed } +const WAIT_EXPORTS: Record = { + '@sim/utils/helpers': ['sleep', 'interruptibleSleep'], + 'node:timers/promises': ['setTimeout'], + 'timers/promises': ['setTimeout'], +} + +type HookKind = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll' +const HOOK_KINDS = new Set(['beforeAll', 'beforeEach', 'afterEach', 'afterAll']) + +/** A call and where it runs: its nearest function, and the innermost hook callback around it. */ +interface Site { + node: Node + fn: [number, number] + hook?: { kind: HookKind; range: [number, number]; suite: [number, number] } +} + +function within(position: number, range: [number, number]): boolean { + return range[0] <= position && position < range[1] +} + +function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): Site { + const fnIndex = nearestFunctionIndex(ancestors, ancestors.length) + const site: Site = { node, fn: fnIndex === -1 ? programRange : nodeRange(ancestors[fnIndex]) } + for (let index = ancestors.length - 1; index > 0; index--) { + const fn = ancestors[index] + const call = ancestors[index - 1] + if (!FUNCTION_TYPES.has(fn.type) || call.type !== 'CallExpression') continue + const callee = call.callee as Node + const kind = callee.type === 'Identifier' ? (callee.name as string) : '' + if (!HOOK_KINDS.has(kind) || (call.arguments as Node[])[0] !== fn) continue + const owner = nearestFunctionIndex(ancestors, index - 1) + site.hook = { + kind: kind as HookKind, + range: nodeRange(fn), + suite: owner === -1 ? programRange : nodeRange(ancestors[owner]), + } + break + } + return site +} + +/** + * Whether a `vi.useFakeTimers()`/`vi.useRealTimers()` call at `timer` is in effect at `target`. + * Inside its own function it applies to what follows it. From a `beforeAll` it also applies to the + * rest of its suite except `beforeAll` hooks declared before it; from a `beforeEach`, to the + * suite's tests, `afterEach` hooks, and `beforeEach` hooks declared after it or in nested suites. + */ +function inEffect(timer: Site, target: Site): boolean { + const position = target.node.start as number + if (within(position, timer.fn)) return position > (timer.node.start as number) + const hook = timer.hook + if (!hook || hook.range[0] !== timer.fn[0] || !within(position, hook.suite)) return false + if (hook.kind !== 'beforeAll' && hook.kind !== 'beforeEach') return false + const other = target.hook + if (!other) return true + const earlierSibling = other.suite[0] === hook.suite[0] && other.range[0] < hook.range[0] + if (hook.kind === 'beforeAll') return !(other.kind === 'beforeAll' && earlierSibling) + if (other.kind === 'beforeAll' || other.kind === 'afterAll') return false + return !(other.kind === 'beforeEach' && earlierSibling) +} + /** * `real-wait` violations in one unit test file: a call that waits on the wall clock — `sleep` or - * `interruptibleSleep` from `@sim/utils/helpers`, `setTimeout` from `node:timers/promises`, or a - * promise resolved by `setTimeout` — with a non-zero delay and no `vi.useFakeTimers()` in scope. + * `interruptibleSleep` from `@sim/utils/helpers`, `setTimeout` from `node:timers/promises` (named + * or through a namespace import), or a promise resolved by `setTimeout` — with a non-zero delay + * and no fake clock in effect. * * Vitest's fake clock fakes each of these whenever it fakes `setTimeout`, and then the wait costs - * no wall-clock time. Fake timers are in scope when the call sits inside a function (or - * the module) that installed them in an earlier statement, or inside a `describe` (or the module) - * whose `beforeEach`/`beforeAll` installs them, except before the install in that hook's own body. A file that mocks `@sim/utils/helpers` gets the - * mock's `sleep`, so its imports are not real waits. Each occurrence is its own numbered entry, so - * the baseline caps the count per file. + * no wall-clock time. A wait is exempt when a `vi.useFakeTimers()` that provably fakes + * `setTimeout` is in effect there (see `inEffect`) and no `vi.useRealTimers()` in effect there runs + * after it. A file that mocks `@sim/utils/helpers` gets the mock's `sleep`, so its imports are not + * real waits. Each occurrence is its own numbered entry, so the baseline caps the count per file. */ export function findRealWaitViolations( file: string, program: ReturnType ): Violation[] { if (file.endsWith('.integration.ts') || file.endsWith('.live.test.ts')) return [] - const waitImports = new Map() let helpersMocked = false walk(program, (node) => { if (mockedId(node) === '@sim/utils/helpers') helpersMocked = true }) + const waitImports = new Map() + const namespaces = new Map() for (const statement of program.body as Node[]) { if (statement.type !== 'ImportDeclaration') continue const source = (statement.source as { value: string }).value - const exported = - source === '@sim/utils/helpers' && !helpersMocked - ? ['sleep', 'interruptibleSleep'] - : source === 'node:timers/promises' || source === 'timers/promises' - ? ['setTimeout'] - : [] + const exported = source === '@sim/utils/helpers' && helpersMocked ? [] : WAIT_EXPORTS[source] + if (!exported?.length) continue for (const specifier of statement.specifiers as Node[]) { + const local = (specifier.local as { name: string }).name + if (specifier.type === 'ImportNamespaceSpecifier') namespaces.set(local, exported) if (specifier.type !== 'ImportSpecifier') continue const imported = specifier.imported as { name?: string; value?: string } const name = imported.name ?? imported.value ?? '' - if (!exported.includes(name)) continue - const label = name === 'setTimeout' ? 'timers/promises setTimeout()' : `${name}()` - waitImports.set((specifier.local as { name: string }).name, label) + if (exported.includes(name)) waitImports.set(local, waitLabel(name)) } } + function importedWait(node: Node): string | undefined { + if (node.type !== 'CallExpression' || !isNonZeroDelay((node.arguments as Node[])[0])) { + return undefined + } + const callee = node.callee as Node + if (callee.type === 'Identifier') return waitImports.get(callee.name as string) + const object = callee.object as Node | undefined + const property = callee.property as Node | undefined + if ( + callee.type !== 'MemberExpression' || + callee.computed || + object?.type !== 'Identifier' || + property?.type !== 'Identifier' + ) { + return undefined + } + const exported = namespaces.get(object.name as string) + const name = property.name as string + return exported?.includes(name) ? waitLabel(name) : undefined + } const programRange: [number, number] = [program.start ?? 0, program.end ?? 0] - // `real` is the stretch of the scope that runs before the clock is installed. - const fakeClocks: Array<{ scope: [number, number]; real: [number, number] }> = [] - const waits: Array<{ node: Node; label: string }> = [] + const fakeClocks: Site[] = [] + const realClocks: Site[] = [] + const waits: Array<{ site: Site; label: string }> = [] walkWithAncestors(program, (node, ancestors) => { - if (isViCall(node, 'useFakeTimers') && fakesSetTimeout(node)) { - const fnIndex = nearestFunctionIndex(ancestors, ancestors.length) - const fn = ancestors[fnIndex] - const hook = ancestors[fnIndex - 1] - const hookName = - hook?.type === 'CallExpression' && (hook.callee as Node).type === 'Identifier' - ? (hook.callee as { name: string }).name - : undefined - if ( - fn && - (hookName === 'beforeEach' || hookName === 'beforeAll') && - (hook.arguments as Node[])[0] === fn - ) { - const owner = ancestors[nearestFunctionIndex(ancestors, fnIndex - 1)] - fakeClocks.push({ - scope: owner ? nodeRange(owner) : programRange, - real: [fn.start as number, node.start as number], - }) - } else { - const scope = fn ? nodeRange(fn) : programRange - fakeClocks.push({ scope, real: [scope[0], node.start as number] }) - } + if (isViCall(node, 'useFakeTimers')) { + if (fakesSetTimeout(node)) fakeClocks.push(siteOf(node, ancestors, programRange)) return } - if (node.type === 'CallExpression' && (node.callee as Node).type === 'Identifier') { - const label = waitImports.get((node.callee as { name: string }).name) - if (label && isNonZeroDelay((node.arguments as Node[])[0])) waits.push({ node, label }) + if (isViCall(node, 'useRealTimers')) { + realClocks.push(siteOf(node, ancestors, programRange)) return } - if (isTimedPromise(node)) waits.push({ node, label: 'setTimeout promise' }) + const label = isTimedPromise(node) ? 'setTimeout promise' : importedWait(node) + if (label) waits.push({ site: siteOf(node, ancestors, programRange), label }) }) const counts = new Map() const violations: Violation[] = [] - for (const { node, label } of waits) { - const start = node.start as number + for (const { site, label } of waits) { const faked = fakeClocks.some( - ({ scope, real }) => - scope[0] <= start && start < scope[1] && !(real[0] <= start && start < real[1]) + (fake) => + inEffect(fake, site) && + !realClocks.some((real) => inEffect(real, site) && inEffect(fake, real)) ) if (faked) continue const count = (counts.get(label) ?? 0) + 1 @@ -415,6 +479,10 @@ export function findRealWaitViolations( return violations } +function waitLabel(name: string): string { + return name === 'setTimeout' ? 'timers/promises setTimeout()' : `${name}()` +} + function mockedId(node: Node): string | undefined { if (!isViCall(node, 'mock')) return undefined const first = (node.arguments as Node[])[0] @@ -616,25 +684,6 @@ function readBaseline(current: string[]): string[] { process.exit(1) } -/** - * `--update --init-rule ` admits every current violation of a rule that has no baseline - * entry yet: the one-time baseline for a newly introduced rule. Refuses an unknown rule or one the - * baseline already tracks, so it can never top up an existing rule. - */ -function readNewRuleEntries(current: string[], baseline: Set): string[] { - const flag = process.argv.indexOf('--init-rule') - if (flag === -1) return [] - const rule = process.argv[flag + 1] - const tracked = [...baseline].some((entry) => entry.startsWith(`${rule}\t`)) - if (!rule || !Object.keys(FIX).includes(rule) || tracked) { - console.error( - `✗ --init-rule ${rule ?? ''}: ${tracked ? 'the baseline already tracks this rule' : 'unknown rule'}.` - ) - process.exit(1) - } - return current.filter((entry) => entry.startsWith(`${rule}\t`)) -} - /** How to fix each rule, printed once per rule with new violations. */ const FIX: Record = { 'global-remock': 'drive the global mock through its knobs instead of re-mocking it', @@ -686,9 +735,6 @@ function main(): void { const current = [...new Set(violations.map(key))].sort() const baseline = new Set(readBaseline(current)) - if (process.argv.includes('--update')) { - for (const entry of readNewRuleEntries(current, baseline)) baseline.add(entry) - } const added = current.filter((entry) => !baseline.has(entry)) if (process.argv.includes('--update')) { From 7eb3e88e6912e967bd062e63cfecdbe6a27103c3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 20:43:22 -0700 Subject: [PATCH 4/8] chore(tests): make the wait-export table read-only --- scripts/check-test-patterns.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index 70ae35389d7..b3e4be08024 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -327,7 +327,7 @@ function isTimedPromise(node: Node): boolean { return timed } -const WAIT_EXPORTS: Record = { +const WAIT_EXPORTS: Readonly> = { '@sim/utils/helpers': ['sleep', 'interruptibleSleep'], 'node:timers/promises': ['setTimeout'], 'timers/promises': ['setTimeout'], @@ -410,7 +410,7 @@ export function findRealWaitViolations( if (mockedId(node) === '@sim/utils/helpers') helpersMocked = true }) const waitImports = new Map() - const namespaces = new Map() + const namespaces = new Map() for (const statement of program.body as Node[]) { if (statement.type !== 'ImportDeclaration') continue const source = (statement.source as { value: string }).value From 33d9ca3593100b348a043bd58a9c2bd1a1be8620 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 20:54:19 -0700 Subject: [PATCH 5/8] fix(tests): let a module-scope fake-timer install cover helper functions declared above it Functions run only after the module loads, so a module-scope vi.useFakeTimers() (or vi.useRealTimers()) applies to every function body regardless of source order; module-scope waits still follow source order. --- scripts/check-test-patterns.test.ts | 19 +++++++++++++++++++ scripts/check-test-patterns.ts | 16 +++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 512aed729b2..376a5c2b671 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -234,6 +234,21 @@ describe('real-wait', () => { ], ['sleep() #1', 'timers/promises setTimeout() #1'], ], + [ + 'a module-scope wait before a module-scope install', + [HELPERS, 'await sleep(10)', 'vi.useFakeTimers()'], + ['sleep() #1'], + ], + [ + 'a helper wait after module-scope real timers are restored', + [ + HELPERS, + 'async function settle() { await sleep(10) }', + 'vi.useFakeTimers()', + 'vi.useRealTimers()', + ], + ['sleep() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], @@ -312,6 +327,10 @@ describe('real-wait', () => { 'afterEach(async () => { await sleep(10) })', ], ], + [ + 'a helper declared before a module-scope install', + [HELPERS, 'async function settle() { await sleep(10) }', 'vi.useFakeTimers()'], + ], ])('ignores %s', (_name, lines) => { expect(waits(lines.join('\n'))).toEqual([]) }) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index b3e4be08024..3f09231a278 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -336,10 +336,14 @@ const WAIT_EXPORTS: Readonly> = { type HookKind = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll' const HOOK_KINDS = new Set(['beforeAll', 'beforeEach', 'afterEach', 'afterAll']) -/** A call and where it runs: its nearest function, and the innermost hook callback around it. */ +/** + * A call and where it runs: its nearest function (the module when `inModule`), and the innermost + * hook callback around it. + */ interface Site { node: Node fn: [number, number] + inModule: boolean hook?: { kind: HookKind; range: [number, number]; suite: [number, number] } } @@ -349,7 +353,11 @@ function within(position: number, range: [number, number]): boolean { function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): Site { const fnIndex = nearestFunctionIndex(ancestors, ancestors.length) - const site: Site = { node, fn: fnIndex === -1 ? programRange : nodeRange(ancestors[fnIndex]) } + const site: Site = { + node, + fn: fnIndex === -1 ? programRange : nodeRange(ancestors[fnIndex]), + inModule: fnIndex === -1, + } for (let index = ancestors.length - 1; index > 0; index--) { const fn = ancestors[index] const call = ancestors[index - 1] @@ -370,12 +378,14 @@ function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): /** * Whether a `vi.useFakeTimers()`/`vi.useRealTimers()` call at `timer` is in effect at `target`. - * Inside its own function it applies to what follows it. From a `beforeAll` it also applies to the + * Inside its own function it applies to what follows it; at module scope, also to every function + * body, since functions run only after the module has loaded. From a `beforeAll` it also applies to the * rest of its suite except `beforeAll` hooks declared before it; from a `beforeEach`, to the * suite's tests, `afterEach` hooks, and `beforeEach` hooks declared after it or in nested suites. */ function inEffect(timer: Site, target: Site): boolean { const position = target.node.start as number + if (timer.inModule && !target.inModule) return true if (within(position, timer.fn)) return position > (timer.node.start as number) const hook = timer.hook if (!hook || hook.range[0] !== timer.fn[0] || !within(position, hook.suite)) return false From c1d5c6174c9adbc3f2266fdc95c4bf42399291aa Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 21:06:06 -0700 Subject: [PATCH 6/8] fix(tests): exempt helpers from a module-scope fake-timer install only when nothing ran before it A helper called at module scope before vi.useFakeTimers() waits on the real clock. A module-scope install now reaches function bodies only when every earlier module-scope statement is load-safe (imports, function/class declarations, type-only statements, vi.mock/doMock/hoisted, and variables whose initializers call nothing); otherwise source order applies. A module-scope vi.useRealTimers() still reaches every function body. --- scripts/check-test-patterns.test.ts | 21 ++++++++- scripts/check-test-patterns.ts | 73 +++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 376a5c2b671..8c7e8b19c43 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -249,6 +249,16 @@ describe('real-wait', () => { ], ['sleep() #1'], ], + [ + 'a helper called at module scope before a module-scope install', + [ + HELPERS, + 'async function settle() { await sleep(10) }', + 'await settle()', + 'vi.useFakeTimers()', + ], + ['sleep() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], @@ -329,7 +339,16 @@ describe('real-wait', () => { ], [ 'a helper declared before a module-scope install', - [HELPERS, 'async function settle() { await sleep(10) }', 'vi.useFakeTimers()'], + [ + HELPERS, + "vi.mock('@/lib/example', () => ({}))", + 'const mocks = vi.hoisted(() => ({ run: vi.fn() }))', + 'const DELAY_MS = 10', + 'type Options = { delay: number }', + 'async function settle() { await sleep(DELAY_MS) }', + 'const settleLater = async () => { await settle() }', + 'vi.useFakeTimers()', + ], ], ])('ignores %s', (_name, lines) => { expect(waits(lines.join('\n'))).toEqual([]) diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index 3f09231a278..d86c42db64c 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -338,12 +338,14 @@ const HOOK_KINDS = new Set(['beforeAll', 'beforeEach', 'afterEach', 'aft /** * A call and where it runs: its nearest function (the module when `inModule`), and the innermost - * hook callback around it. + * hook callback around it. `coversFunctions` marks a module-scope timer call that applies to every + * function body: a restore, or an install no executing module-scope statement precedes. */ interface Site { node: Node fn: [number, number] inModule: boolean + coversFunctions?: boolean hook?: { kind: HookKind; range: [number, number]; suite: [number, number] } } @@ -376,16 +378,69 @@ function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): return site } +const LOAD_SAFE_VI_CALLS = ['mock', 'doMock', 'hoisted'] +const TYPE_ONLY_STATEMENTS = new Set([ + 'TSTypeAliasDeclaration', + 'TSInterfaceDeclaration', + 'TSDeclareFunction', +]) + +/** Whether evaluating `node` runs code: a call, `new`, `await` or tagged template outside a function. */ +function runsCode(node: unknown): boolean { + if (!node || typeof node !== 'object') return false + if (Array.isArray(node)) return node.some(runsCode) + const current = node as Node + if (FUNCTION_TYPES.has(current.type)) return false + if (LOAD_SAFE_VI_CALLS.some((method) => isViCall(current, method))) return false + if ( + ['CallExpression', 'NewExpression', 'AwaitExpression', 'TaggedTemplateExpression'].includes( + current.type + ) + ) { + return true + } + return Object.entries(current).some( + ([key, value]) => !['loc', 'start', 'end', 'extra'].includes(key) && runsCode(value) + ) +} + +/** + * Whether a module-scope statement runs code when the module loads. Imports, function and class + * declarations, type-only statements, `vi.mock`/`vi.doMock`/`vi.hoisted`, and variables whose + * initializers call nothing do not. + */ +function executesAtLoad(statement: Node): boolean { + const declaration = + (statement.type === 'ExportNamedDeclaration' || + statement.type === 'ExportDefaultDeclaration') && + statement.declaration + ? (statement.declaration as Node) + : statement + if ( + declaration.type === 'ImportDeclaration' || + declaration.type === 'FunctionDeclaration' || + declaration.type === 'ClassDeclaration' || + TYPE_ONLY_STATEMENTS.has(declaration.type) + ) { + return false + } + if (declaration.type === 'VariableDeclaration') { + return (declaration.declarations as Node[]).some((declarator) => runsCode(declarator.init)) + } + if (declaration.type === 'ExpressionStatement') return runsCode(declaration.expression) + return true +} + /** * Whether a `vi.useFakeTimers()`/`vi.useRealTimers()` call at `timer` is in effect at `target`. * Inside its own function it applies to what follows it; at module scope, also to every function - * body, since functions run only after the module has loaded. From a `beforeAll` it also applies to the + * body when `coversFunctions`. From a `beforeAll` it also applies to the * rest of its suite except `beforeAll` hooks declared before it; from a `beforeEach`, to the * suite's tests, `afterEach` hooks, and `beforeEach` hooks declared after it or in nested suites. */ function inEffect(timer: Site, target: Site): boolean { const position = target.node.start as number - if (timer.inModule && !target.inModule) return true + if (timer.coversFunctions && !target.inModule) return true if (within(position, timer.fn)) return position > (timer.node.start as number) const hook = timer.hook if (!hook || hook.range[0] !== timer.fn[0] || !within(position, hook.suite)) return false @@ -457,16 +512,24 @@ export function findRealWaitViolations( return exported?.includes(name) ? waitLabel(name) : undefined } const programRange: [number, number] = [program.start ?? 0, program.end ?? 0] + const firstExecuting = (program.body as Node[]).find(executesAtLoad) + const quietUntil = firstExecuting ? (firstExecuting.end as number) : Number.POSITIVE_INFINITY + // A module-scope restore always reaches function bodies; an install only when nothing ran first. + const timerSite = (node: Node, ancestors: Node[], restore: boolean): Site => { + const site = siteOf(node, ancestors, programRange) + site.coversFunctions = site.inModule && (restore || (node.start as number) < quietUntil) + return site + } const fakeClocks: Site[] = [] const realClocks: Site[] = [] const waits: Array<{ site: Site; label: string }> = [] walkWithAncestors(program, (node, ancestors) => { if (isViCall(node, 'useFakeTimers')) { - if (fakesSetTimeout(node)) fakeClocks.push(siteOf(node, ancestors, programRange)) + if (fakesSetTimeout(node)) fakeClocks.push(timerSite(node, ancestors, false)) return } if (isViCall(node, 'useRealTimers')) { - realClocks.push(siteOf(node, ancestors, programRange)) + realClocks.push(timerSite(node, ancestors, true)) return } const label = isTimedPromise(node) ? 'setTimeout promise' : importedWait(node) From b42f77be6f012f3a9a8b034e1c82548cc39ee670 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 21:18:14 -0700 Subject: [PATCH 7/8] fix(tests): skip only helpers called before a module-scope fake-timer install Any unrelated module-scope call (const run = vi.fn()) used to drop the helper exemption. Now a module-scope vi.useFakeTimers() covers every function body except a helper (function declaration or variable bound to a function) that earlier module-scope code calls directly by name; calls through other helpers are not traced. WAIT_EXPORTS and HOOK_KINDS are as const. --- scripts/check-test-patterns.test.ts | 14 +++ scripts/check-test-patterns.ts | 133 ++++++++++++++-------------- 2 files changed, 80 insertions(+), 67 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 8c7e8b19c43..878bd6c988b 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -259,6 +259,16 @@ describe('real-wait', () => { ], ['sleep() #1'], ], + [ + 'an arrow helper called at module scope before a module-scope install', + [ + HELPERS, + 'const settle = async () => { await sleep(10) }', + 'await settle()', + 'vi.useFakeTimers()', + ], + ['sleep() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], @@ -347,7 +357,11 @@ describe('real-wait', () => { 'type Options = { delay: number }', 'async function settle() { await sleep(DELAY_MS) }', 'const settleLater = async () => { await settle() }', + 'const run = vi.fn()', + 'const fixture = createFixture()', 'vi.useFakeTimers()', + 'await settle()', + "it('t', async () => { await settle() })", ], ], ])('ignores %s', (_name, lines) => { diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index d86c42db64c..50bed9481ad 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -327,25 +327,29 @@ function isTimedPromise(node: Node): boolean { return timed } -const WAIT_EXPORTS: Readonly> = { +const WAIT_EXPORTS = { '@sim/utils/helpers': ['sleep', 'interruptibleSleep'], 'node:timers/promises': ['setTimeout'], 'timers/promises': ['setTimeout'], -} +} as const +const WAIT_EXPORTS_BY_SOURCE: Readonly> = WAIT_EXPORTS -type HookKind = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll' -const HOOK_KINDS = new Set(['beforeAll', 'beforeEach', 'afterEach', 'afterAll']) +const HOOK_KINDS = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'] as const +type HookKind = (typeof HOOK_KINDS)[number] +const HOOK_KIND_NAMES: readonly string[] = HOOK_KINDS /** - * A call and where it runs: its nearest function (the module when `inModule`), and the innermost - * hook callback around it. `coversFunctions` marks a module-scope timer call that applies to every - * function body: a restore, or an install no executing module-scope statement precedes. + * A call and where it runs: its nearest function (the module when `inModule`), the module-scope + * helper it sits in (a function declaration or a variable bound to a function), and the innermost + * hook callback around it. For a module-scope `vi.useFakeTimers()`, `calledBefore` names the + * functions module-scope code calls before it. */ interface Site { node: Node fn: [number, number] inModule: boolean - coversFunctions?: boolean + helper?: string + calledBefore?: Set hook?: { kind: HookKind; range: [number, number]; suite: [number, number] } } @@ -359,6 +363,7 @@ function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): node, fn: fnIndex === -1 ? programRange : nodeRange(ancestors[fnIndex]), inModule: fnIndex === -1, + helper: helperName(ancestors), } for (let index = ancestors.length - 1; index > 0; index--) { const fn = ancestors[index] @@ -366,7 +371,7 @@ function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): if (!FUNCTION_TYPES.has(fn.type) || call.type !== 'CallExpression') continue const callee = call.callee as Node const kind = callee.type === 'Identifier' ? (callee.name as string) : '' - if (!HOOK_KINDS.has(kind) || (call.arguments as Node[])[0] !== fn) continue + if (!HOOK_KIND_NAMES.includes(kind) || (call.arguments as Node[])[0] !== fn) continue const owner = nearestFunctionIndex(ancestors, index - 1) site.hook = { kind: kind as HookKind, @@ -378,69 +383,61 @@ function siteOf(node: Node, ancestors: Node[], programRange: [number, number]): return site } -const LOAD_SAFE_VI_CALLS = ['mock', 'doMock', 'hoisted'] -const TYPE_ONLY_STATEMENTS = new Set([ - 'TSTypeAliasDeclaration', - 'TSInterfaceDeclaration', - 'TSDeclareFunction', -]) - -/** Whether evaluating `node` runs code: a call, `new`, `await` or tagged template outside a function. */ -function runsCode(node: unknown): boolean { - if (!node || typeof node !== 'object') return false - if (Array.isArray(node)) return node.some(runsCode) - const current = node as Node - if (FUNCTION_TYPES.has(current.type)) return false - if (LOAD_SAFE_VI_CALLS.some((method) => isViCall(current, method))) return false +/** The module-scope function declaration, or variable bound to a function, that `ancestors` sit in. */ +function helperName(ancestors: Node[]): string | undefined { + let index = 1 + while (ancestors[index]?.type.startsWith('Export')) index++ + const statement = ancestors[index] + const inner = ancestors[index + 1] + if (statement?.type === 'FunctionDeclaration') { + return (statement.id as Node | null)?.name as string | undefined + } if ( - ['CallExpression', 'NewExpression', 'AwaitExpression', 'TaggedTemplateExpression'].includes( - current.type - ) + statement?.type === 'VariableDeclaration' && + inner?.type === 'VariableDeclarator' && + (inner.id as Node).type === 'Identifier' && + ancestors[index + 2] === inner.init && + FUNCTION_TYPES.has((inner.init as Node).type) ) { - return true + return (inner.id as Node).name as string } - return Object.entries(current).some( - ([key, value]) => !['loc', 'start', 'end', 'extra'].includes(key) && runsCode(value) - ) + return undefined } -/** - * Whether a module-scope statement runs code when the module loads. Imports, function and class - * declarations, type-only statements, `vi.mock`/`vi.doMock`/`vi.hoisted`, and variables whose - * initializers call nothing do not. - */ -function executesAtLoad(statement: Node): boolean { - const declaration = - (statement.type === 'ExportNamedDeclaration' || - statement.type === 'ExportDefaultDeclaration') && - statement.declaration - ? (statement.declaration as Node) - : statement - if ( - declaration.type === 'ImportDeclaration' || - declaration.type === 'FunctionDeclaration' || - declaration.type === 'ClassDeclaration' || - TYPE_ONLY_STATEMENTS.has(declaration.type) - ) { - return false +/** Names of the functions `node` calls directly by identifier when it runs, outside nested functions. */ +function directCalls(node: unknown, names: Set): Set { + if (!node || typeof node !== 'object') return names + if (Array.isArray(node)) { + for (const child of node) directCalls(child, names) + return names } - if (declaration.type === 'VariableDeclaration') { - return (declaration.declarations as Node[]).some((declarator) => runsCode(declarator.init)) + const current = node as Node + if (FUNCTION_TYPES.has(current.type)) return names + if (current.type === 'CallExpression' && (current.callee as Node).type === 'Identifier') { + names.add((current.callee as Node).name as string) } - if (declaration.type === 'ExpressionStatement') return runsCode(declaration.expression) - return true + for (const [key, value] of Object.entries(current)) { + if (key !== 'loc' && key !== 'start' && key !== 'end' && key !== 'extra') { + directCalls(value, names) + } + } + return names } /** * Whether a `vi.useFakeTimers()`/`vi.useRealTimers()` call at `timer` is in effect at `target`. - * Inside its own function it applies to what follows it; at module scope, also to every function - * body when `coversFunctions`. From a `beforeAll` it also applies to the + * Inside its own function it applies to what follows it. At module scope it also applies to every + * function body, since functions run after the module loads, except an install skips helpers that + * module-scope code calls before it (directly only: a helper reached through another call is + * missed). From a `beforeAll` it also applies to the * rest of its suite except `beforeAll` hooks declared before it; from a `beforeEach`, to the * suite's tests, `afterEach` hooks, and `beforeEach` hooks declared after it or in nested suites. */ function inEffect(timer: Site, target: Site): boolean { const position = target.node.start as number - if (timer.coversFunctions && !target.inModule) return true + if (timer.inModule && !target.inModule) { + return !(target.helper && timer.calledBefore?.has(target.helper)) + } if (within(position, timer.fn)) return position > (timer.node.start as number) const hook = timer.hook if (!hook || hook.range[0] !== timer.fn[0] || !within(position, hook.suite)) return false @@ -479,7 +476,8 @@ export function findRealWaitViolations( for (const statement of program.body as Node[]) { if (statement.type !== 'ImportDeclaration') continue const source = (statement.source as { value: string }).value - const exported = source === '@sim/utils/helpers' && helpersMocked ? [] : WAIT_EXPORTS[source] + const exported = + source === '@sim/utils/helpers' && helpersMocked ? [] : WAIT_EXPORTS_BY_SOURCE[source] if (!exported?.length) continue for (const specifier of statement.specifiers as Node[]) { const local = (specifier.local as { name: string }).name @@ -512,24 +510,25 @@ export function findRealWaitViolations( return exported?.includes(name) ? waitLabel(name) : undefined } const programRange: [number, number] = [program.start ?? 0, program.end ?? 0] - const firstExecuting = (program.body as Node[]).find(executesAtLoad) - const quietUntil = firstExecuting ? (firstExecuting.end as number) : Number.POSITIVE_INFINITY - // A module-scope restore always reaches function bodies; an install only when nothing ran first. - const timerSite = (node: Node, ancestors: Node[], restore: boolean): Site => { - const site = siteOf(node, ancestors, programRange) - site.coversFunctions = site.inModule && (restore || (node.start as number) < quietUntil) - return site - } const fakeClocks: Site[] = [] const realClocks: Site[] = [] const waits: Array<{ site: Site; label: string }> = [] walkWithAncestors(program, (node, ancestors) => { if (isViCall(node, 'useFakeTimers')) { - if (fakesSetTimeout(node)) fakeClocks.push(timerSite(node, ancestors, false)) + if (fakesSetTimeout(node)) { + const site = siteOf(node, ancestors, programRange) + if (site.inModule) { + const before = (program.body as Node[]).filter( + (statement) => (statement.end as number) <= (node.start as number) + ) + site.calledBefore = directCalls(before, new Set()) + } + fakeClocks.push(site) + } return } if (isViCall(node, 'useRealTimers')) { - realClocks.push(timerSite(node, ancestors, true)) + realClocks.push(siteOf(node, ancestors, programRange)) return } const label = isTimedPromise(node) ? 'setTimeout promise' : importedWait(node) From 9e04746821dec9427b55acf3d7c4f285a2b4d193 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 21:29:26 -0700 Subject: [PATCH 8/8] fix(tests): recognize module-scope helpers whose initializer is wrapped in a type assertion helperName now unwraps as/satisfies/non-null/parenthesized initializers with the shared unwrap helper, so a wrapped arrow called before a module-scope vi.useFakeTimers() stays flagged. --- scripts/check-test-patterns.test.ts | 10 ++++++++++ scripts/check-test-patterns.ts | 5 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/check-test-patterns.test.ts b/scripts/check-test-patterns.test.ts index 878bd6c988b..76b0b9e9b05 100644 --- a/scripts/check-test-patterns.test.ts +++ b/scripts/check-test-patterns.test.ts @@ -269,6 +269,16 @@ describe('real-wait', () => { ], ['sleep() #1'], ], + [ + 'a wrapped arrow helper called at module scope before a module-scope install', + [ + HELPERS, + 'const settle = (async () => { await sleep(10) }) satisfies (() => Promise)', + 'await settle()', + 'vi.useFakeTimers()', + ], + ['sleep() #1'], + ], [ 'every occurrence, numbered per kind', [HELPERS, 'await sleep(10)', 'await new Promise((r) => setTimeout(r, 5))', 'await sleep(20)'], diff --git a/scripts/check-test-patterns.ts b/scripts/check-test-patterns.ts index 50bed9481ad..622bfba77ab 100644 --- a/scripts/check-test-patterns.ts +++ b/scripts/check-test-patterns.ts @@ -389,6 +389,7 @@ function helperName(ancestors: Node[]): string | undefined { while (ancestors[index]?.type.startsWith('Export')) index++ const statement = ancestors[index] const inner = ancestors[index + 1] + const init = inner?.type === 'VariableDeclarator' ? unwrap(inner.init as Node) : undefined if (statement?.type === 'FunctionDeclaration') { return (statement.id as Node | null)?.name as string | undefined } @@ -396,8 +397,8 @@ function helperName(ancestors: Node[]): string | undefined { statement?.type === 'VariableDeclaration' && inner?.type === 'VariableDeclarator' && (inner.id as Node).type === 'Identifier' && - ancestors[index + 2] === inner.init && - FUNCTION_TYPES.has((inner.init as Node).type) + init && + FUNCTION_TYPES.has(init.type) ) { return (inner.id as Node).name as string }