test: raise timeout ceiling for I/O-bound tests flaky under CI contention - #7230
Conversation
…tion The self-hosted CI runners are heavily oversubscribed (core runs maxThreads: 16), and a recurring class of tests blows vitest's 5s default timeout purely under that contention — not from any logic fault. Observed repeatedly across unrelated PRs (#7213, #7219, and noted in prior sessions): - packages/core/src/utils/shell-ast-parser-lazy.test.ts — fully mocked, but the dynamic import + async coordination exceeds 5s when 16 threads contend. - packages/cli/src/serve/workspace-registration-store.test.ts — tempdir round-trip. - packages/core/src/extension/github.test.ts > extractFile — its waitForFileData helper polled a FIXED 1_000 setImmediate turns, which elapse in <100ms while the tar extraction I/O is still catching up, throwing 'Timed out waiting for extracted data'. Fixes: - testTimeout: 15000 in the core and cli vitest configs — 3x the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a real hang still fails, just later, and the job timeout still bounds it). - waitForFileData now polls a real ~10s wall-clock budget (2_000 x 5ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15s ceiling. These are the deterministic root-cause fixes for the flake class the autofix loop and CI Failure Patrol were papering over with reruns.
|
Thanks for the PR! Template looks good ✓ (minor: "Why it's needed" is folded into the opening section rather than its own heading — fine here, the motivation is crystal clear.) Problem: observed and recurring. The flake class is well-evidenced across #7213 and #7219 — a different untouched test fails on each rerun, which is the classic runner-contention signature. Not theoretical. Direction: squarely in scope — fixing CI flakiness at the root cause instead of masking it with reruns. No auth/sandbox/model/telemetry concerns. Size: 0 production logic lines. All three changed files are test files or vitest configs. Core module protection not triggered. Approach: minimal and focused — two config lines raising the timeout ceiling, plus one poll-loop fix that switches from iteration-count to wall-clock budget. No unrelated changes, no scope creep. The Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓(小瑕疵:"Why it's needed" 没有单独标题,而是融入了开头部分——无妨,动机已经非常清楚。) 问题:已观测且反复出现。该 flaky 类别在 #7213 和 #7219 中有充分证据——每次重试失败的是不同的、PR 未触及的测试,这是典型的 runner 争用特征。非理论性问题。 方向:完全在范围内——从根因修复 CI flaky,而非靠重试掩盖。无 auth/sandbox/model/telemetry 相关顾虑。 规模:0 行生产逻辑。三个改动文件均为测试文件或 vitest 配置。未触发核心模块保护。 方案:最小且聚焦——两行配置抬高超时上限,加一处轮询修复(从固定迭代次数改为真实时钟预算)。无无关改动,无范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
|
Code review Independent proposal for "fix CI-contention flaky tests": raise The PR matches this exactly. Three files, no surprises:
No critical blockers. No convention violations. No unrelated changes. Real-scenario testing Ran all three affected test files on the PR branch ( All three pass. Note 中文说明代码审查 独立方案:抬高 vitest 配置中的 PR 与此完全一致。三个文件,无意外:
无关键阻塞。无规范违反。无无关改动。 真实场景测试 在 PR 分支上用 tmux 运行了全部三个受影响的测试文件,均通过。注意 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 This is a textbook flaky-test root-cause fix: small, well-evidenced, and strictly one-directional — raising a timeout ceiling and fixing a broken poll loop can only reduce flakiness, never introduce it. The evidence from #7213/#7219 is convincing (different untouched test fails each rerun = runner load, not the diff), and the No logic changes, no assertion changes, no production code touched. The tradeoff (real hangs take 15s instead of 5s to surface) is well-understood and bounded by the job-level timeout. LGTM — approving. ✅ 中文说明置信度:5/5 这是一个教科书级的 flaky 测试根因修复:小改动、充分证据、严格单向——抬高超时上限和修复坏掉的轮询循环只可能减少 flaky,不可能引入。#7213/#7219 的证据令人信服(每次重试失败的是不同的、未触及的测试 = runner 负载,而非 diff), 无逻辑改动、无断言改动、未触及生产代码。权衡(真正的卡死需要 15s 而非 5s 才浮现)是充分理解的,且有 job 级超时兜底。 LGTM——批准。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max-preview via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
|
Released in v0.20.1. |
|
Agent run timed out after 1800000ms ❌ failed |
What this PR does
Deterministically eliminates a recurring flaky-test class by fixing its root cause — too-tight timeouts under CI contention — instead of leaning on reruns.
The self-hosted runners are heavily oversubscribed (core runs
maxThreads: 16), and a handful of I/O- or load-bound tests blow vitest's 5s default purely under that contention — not from any logic fault. This has recurred across unrelated PRs (#7213, #7219) and earlier sessions; each time the failing test was in a package the PR didn't touch, and each rerun landed a different one of these:packages/core/src/utils/shell-ast-parser-lazy.test.ts— fully mocked (it never loads the real WASM), but the dynamicimport()+ async coordination exceeds 5s when 16 threads contend.packages/cli/src/serve/workspace-registration-store.test.ts— tempdir round-trip.packages/core/src/extension/github.test.ts > extractFile— itswaitForFileDatahelper polled a fixed 1 000setImmediateturns, which elapse in <100 ms while the tar-extraction I/O is still catching up, throwingTimed out waiting for extracted data.The fix
testTimeout: 15000in the core and cli vitest configs — 3× the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a genuine hang still fails, just later, and the job-level timeout still bounds it).waitForFileDatanow polls a real ~10 s wall-clock budget (2 000 × 5 ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15 s ceiling.Reruns and the CI Failure Patrol's model classifier were treating the symptom; these are the deterministic cures for the cause.
Reviewer Test Plan
How to verify
ajv/diff; they must run in a full install, i.e. CI or a built checkout.)Evidence (why it's contention, not logic)
Tested on
Local package runs aren't possible in the review worktree (partial
node_modules→Cannot find package 'ajv/...'at import, unrelated to this change). vitest did load the modified core config and reach the test-run phase, confirming the config is valid.Risk & Scope
testTimeoutis global to each package's suite; it does not weaken any assertion.Linked Issues
Root-cause fix for the flake class seen on #7213 / #7219; complements #7229 (which stops a crash from stranding a PR) so the loop stops re-encountering these as unrecoverable.
中文说明
本 PR 做了什么
从根因上确定性消除一类反复出现的 flaky 测试 —— CI 争用下超时预算太紧 —— 而不是靠重试硬扛。
自建 runner 严重超订(core 用
maxThreads: 16),少数 I/O 或加载型测试在争用下挤爆 vitest 的 5s 默认超时,并非任何逻辑错。这在多个不相关 PR(#7213、#7219)反复出现,每次失败的测试都在 PR 没碰过的包里,且每次重试命中不同的一个:shell-ast-parser-lazy.test.ts—— 全 mock(根本没加载真 WASM),但 16 线程争用下import()+ 异步编排超过 5s。workspace-registration-store.test.ts—— tempdir 往返。github.test.ts > extractFile—— 其waitForFileData轮询固定 1000 次setImmediate,这些在 <100ms 内就跑完,而 tar 解压 I/O 还没跟上,于是抛Timed out waiting for extracted data。修复
testTimeout: 15000(默认的 3 倍)。断言仍秒失败,只是抬高超时上限 —— 不掩盖任何逻辑 bug(真卡死仍会失败,只是晚一点,且 job 级超时兜底)。waitForFileData改为真实 ~10s 时钟预算(2000 × 5ms)轮询,而非固定迭代次数 —— 慢解压是被等待而非被赛过。仍在 15s 上限内。重试与 CI Failure Patrol 的模型分类只治标;这是治本的确定性修复。
评审验证
ajv/diff等包依赖连加载都失败,须在完整安装即 CI 或已构建检出中运行。)风险与范围
testTimeout对各包套件全局生效,不削弱任何断言。关联 Issue
#7213 / #7219 所见 flaky 类别的根因修复;与 #7229(阻止崩溃卡死 PR)互补,使循环不再把它们当作不可恢复反复遇到。