Review 5404

Review 5404 — the ui vitest project's timeout budget

#5404 · cixzhang · internal Head reviewed 38d2e07ca14057744ddf861e4119c91efa346520 Verdict comment — approve-with-nits in substance · posted 2026-08-24 Outcome merged 2026-08-24 by auto-merge, squash, 19 checks green (test 15m14s)

Problem

An author who touched DropdownMenu got a red test job on a file they never opened. Run 32690448839: FAIL ui packages/core/src/DateInput/DateInputTouch.test.tsx > clears without moving when the current month is out of range, Error: Test timed out in 5000ms. at :1504, in a job whose own duration was 880.54s. Locally the same person, running the core suite with a build alongside it, loses 14 of that file's 134 tests — not one of them for a wrong value.

The ui (jsdom) project runs on vitest's 5s default. The node project has carried testTimeout: 30_000 / hookTimeout: 30_000 since #4431, with a comment describing this exact failure mode. ui never got the same treatment.

Solution

One decision, two runtime lines of seven: the ui project takes testTimeout/hookTimeout 30_000, matching node. The other five lines are the explanatory comment. node untouched.

Impact

Builders only; nothing renders and nothing ships. vitest.config.ts is in no package's build output. Every PR author gets one fewer unrelated red — measured rate 1 in the last 40 failed CI runs. Anyone running the core suite alongside other work stops losing 14 tests to a deadline.

API

No API change. No prop, export, type or signature.

Theme targets

No new theme targets. Nothing in this diff renders.

Ossification

n/a — a config literal, not public surface. Reverting is a one-line revert with no consumer.

Breaking

axis	answer
API	no — nothing compiles differently
Visual	no — nothing renders
Theme	no — no target, token or override touched

Far side of the bound driven both ways: at 5s, 14 tests cross and fail; at 30s none cross and the worst lands at 16.7s. Same worktree, same contention, only the budget differing.

Evidence

No frames — not renderable. Numbers instead, re-derived by the gate from the raw logs rather than read off the body.

Unloaded, --project ui --no-file-parallelism:

suite	tests	test time	median	p95	slowest
DateInputTouch.test.tsx	134	36.0s	68ms	1819ms	2023ms
Selector.test.tsx	145	5.2s	21ms	82ms	883ms
MultiSelector.test.tsx	111	3.8s	23ms	92ms	325ms
Table.test.tsx	126	1.0s	6ms	24ms	80ms
BottomSheet.test.tsx	77	0.9s	10ms	27ms	73ms

Under ten busy cores, same file, same worktree:

budget	result	vitest duration	slowest test
5s	14 failed / 120 passed	305.0s	26.6s
30s	134 passed	239.4s	16.7s

All 14 failures are Test timed out in 5000ms; zero assertion failures. Thirteen of the fourteen failing bodies are synchronous it()s with no await in them, so they have nothing to race and can only fail by exceeding the deadline.

Judgement

Comment; approve-with-nits in substance. No blocking finding. Two body errors, both fixed before merge:

1. The body said four of the failing bodies were synchronous. Thirteen of the fourteen are — the "four" was carried over from an earlier investigation's nineteen-failure run.
2. The second table's "wall" column mixed vitest's Duration with the shell's wall clock. Both honest, the header was not.

Also noted, not blocking: the change is project-wide while the evidence shows one file. node's precedent is project-wide too and the deadline is a property of the machine rather than of a suite, but #4642 went the other way for the Markdown streaming perf tests — those declare their own budgets, which nothing here does. The rationale is now written into the body.

Left standing: after this, a ui test that regresses from 68ms to 20s passes green, where today it would fail the job loudly. node has had the same property since #4431. Worth a cheap per-file guard on the expensive suites eventually; not this PR.

Review as posted

Thanks — evidence checks out, nothing blocking. Two body edits: thirteen of fourteen failing bodies are sync, not four; why the project rather than one file — parser.perf.test.ts went the other way.

[Reviewed by Robohands]

What changed before posting

Two gate rounds. Round 1 accepted the body's "four synchronous bodies" claim; round 2 brace-matched all fourteen failing it()s and found thirteen. Round 2 also caught the mixed-clock table header and re-derived the "only occurrence in 40 failed CI runs" claim independently (39 confirmed clean, one run returned a 0-byte log).