fix(ci): yield the event loop between script tests to avoid vitest RPC timeouts (#10037) - #10050
Conversation
…C timeouts (#10037) The v0.22.1 release quality job exited 1 on `npm run test:scripts` with every test green. vitest's worker->main `onTaskUpdate` RPC has a fixed 60s timeout; the synchronous spawnSync-driven script suites keep a forked worker's event loop blocked for an entire file (~66s on the heaviest suite), so the queued RPC response is never processed before the timer fires, surfacing as an unhandled `[vitest-worker]: Timeout calling "onTaskUpdate"` error. Linux keeps unhandled errors fatal (the scripts vitest config only exempts non-Linux since #9728), so the release died. Add a global per-test event-loop yield to the scripts test setup. The timer is captured at setup load so `vi.useFakeTimers()` inside a test cannot intercept the yield. Any continuous stall is now bounded by a single test, so RPC responses drain long before the 60s deadline. Real test failures stay fatal on every platform; the Linux unhandled-error signal is untouched.
E2E Report — Issue #10037: Release v0.22.1 failed on the
|
| Run | Scope | Result |
|---|---|---|
| baseline | vitest run qwen-autofix-workflow at HEAD |
exit 1: 219/219 passed + unhandled Timeout calling "onTaskUpdate" |
| fixed | same file, with the yield hook | exit 0: 219/219 passed, no errors (67.2s) |
| mutation probe | same file, hook removed again | exit 1: the same RPC timeout recurs — the hook is the load-bearing change |
| full run 1 | npm run test:scripts |
exit 0: 65 files, 1738 passed / 16 skipped |
| full run 2 | npm run test:scripts |
exit 0: 65 files, 1738 passed / 16 skipped |
| full run 3 | npm run test:scripts |
exit 1: only the pre-existing verify-capture pixel flake (below); no RPC error |
| full run 4 | npm run test:scripts |
exit 1: same pre-existing flake only; no RPC error |
| full run 5 | npm run test:scripts under taskset -c 0-3 (4-core CI surrogate, deliberately oversubscribed) |
exit 1: same pre-existing flake only; no RPC error |
Across all five full-suite runs with the fix, the deterministic release blocker (unhandled onTaskUpdate timeout) never recurred.
Pre-existing environmental flake observed during verification (not caused by, and not fixed by, this change)
verify-capture.test.js > renders 256-colour and truecolor via the default-grey fallback asserts exact #d4d4d4 pixels in an SVG→PNG rasterisation. On this self-hosted runner it flakes even without the fix — an A/B of the file alone gives 6/10 failures at the base commit vs 4/10 with the fix (statistically indistinguishable). The machine has no fonts installed at all (/usr/share/fonts does not exist, no fontconfig configuration), which makes pango/librsvg glyph rasterisation non-deterministic there. The release quality job runs on GitHub-hosted ubuntu-latest, which ships DejaVu fonts, and the v0.22.1 run passed this test (it died on the RPC error with every test green). This is a runner-environment artifact, pre-existing at the base commit, out of scope for this fix; installing fonts on the fleet (or otherwise hardening that pixel assertion) is a candidate follow-up.
Verification
npm run build— passed (exit 0)npm run typecheck— passed (exit 0)npm run lint— passed (exit 0)npx eslint scripts/tests/test-setup.ts+npx prettier --check scripts/tests/test-setup.ts— passednpm run test:scripts— full-suite runs listed in the Evidence table: 2 fully green runs, plus 3 runs whose only failure is the pre-existing font-less-runner pixel flake A/B-proven independent of this change; zero RPC-timeout errors in all 5 runs- Mutation probe — removing the new hook re-produces the exact release failure (exit 1,
Timeout calling "onTaskUpdate"); restoring it returns the suite to exit 0. The existing 219-testqwen-autofix-workflow.test.jssuite is the committed witness. - Not applicable: workspace package unit tests (the change is confined to the scripts-tests setup file, which no workspace config loads), integration tests (no bundled-CLI behavior touched),
npm run generate:settings-schema(no settings source touched).
中文说明
E2E 报告 — Issue #10037:v0.22.1 发布在 quality 任务上失败
问题
v0.22.1 发布工作流在 quality 任务(ubuntu-latest)的最后一步失败:Run Workspace Tests → npm run test:release → npm run test:scripts(对 scripts/tests 运行 vitest)。
根因
在 HEAD a6d30ebc6b 上单独运行最重的脚本测试套件即可确定性复现:219 个测试全部通过,随后 vitest 报告一个未处理错误并以退出码 1 结束:
Error: [vitest-worker]: Timeout calling "onTaskUpdate"
❯ Object.onTimeoutError node_modules/vitest/dist/chunks/rpc.-pEldfrD.js:53:10
❯ Timeout._onTimeout node_modules/vitest/dist/chunks/index.B521nVV-.js:59:62
在已安装的 vitest 3.2.7 源码中追溯到的机制:
- vitest worker 通过
onTaskUpdateRPC 向主进程上报任务进度,节流间隔约 100ms(@vitest/runner)。 - 该 RPC 有固定的 60 秒超时(内置 birpc 中的
DEFAULT_TIMEOUT = 6e4);超时后 worker 抛出[vitest-worker]: Timeout calling "onTaskUpdate"。 - 脚本测试套件以完全同步的
spawnSync驱动测试为主(尤其是qwen-autofix-workflow.test.js:219 个测试,约 66 秒)。vitest 串行执行一个文件内的测试,同步测试之间从不返回事件循环,因此 fork 出的 worker 的事件循环在整个文件期间一直被阻塞。 - 排队中的 RPC 响应在 60 秒计时器到期前一直得不到处理;当阻塞最终释放时,已过期的计时器先触发,该错误以未处理错误的形式浮现。
scripts/tests/vitest.config.ts设置了dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux'(fix: repair the Windows and macOS test lane failures #9728 为 macOS/Windows 通道添加)。发布quality任务运行在 ubuntu-latest 上,未处理错误在那里仍是致命的——于是所有测试全绿却以退出码 1 结束。
修复
scripts/tests/test-setup.ts(脚本 vitest 配置的 setup 文件)现在在每个测试前用真实计时器让出一次事件循环:
const realSetTimeout = setTimeout;
beforeEach(() => new Promise((resolve) => realSetTimeout(resolve, 0)));该计时器在 setup 加载时捕获,因此测试内部的 vi.useFakeTimers() 永远无法拦截这次让出。这把任何连续的事件循环停滞限制在单个测试内(最长的测试远低于 60 秒),RPC 响应在发出后几毫秒内即可被处理。真实的测试失败在所有平台上仍然是致命的;Linux 上的未处理错误信号保持不变。#9728 添加的 macOS/Windows 豁免保持原样。
证据
| 运行 | 范围 | 结果 |
|---|---|---|
| 基线 | HEAD 上 vitest run qwen-autofix-workflow |
退出码 1:219/219 通过 + 未处理的 Timeout calling "onTaskUpdate" |
| 修复后 | 同一文件,带让出钩子 | 退出码 0:219/219 通过,无错误(67.2 秒) |
| 变异探针 | 同一文件,再次移除钩子 | 退出码 1:同样的 RPC 超时复现——该钩子是起决定作用的变更 |
| 全量运行 1 | npm run test:scripts |
退出码 0:65 个文件,1738 通过 / 16 跳过 |
| 全量运行 2 | npm run test:scripts |
退出码 0:65 个文件,1738 通过 / 16 跳过 |
| 全量运行 3 | npm run test:scripts |
退出码 1:仅既有的 verify-capture 像素抖动(见下文);无 RPC 错误 |
| 全量运行 4 | npm run test:scripts |
退出码 1:仅同一既有抖动;无 RPC 错误 |
| 全量运行 5 | taskset -c 0-3 下的 npm run test:scripts(4 核 CI 替代环境,刻意超额订阅) |
退出码 1:仅同一既有抖动;无 RPC 错误 |
在修复后的全部五次全量套件运行中,确定性的发布阻塞因素(未处理的 onTaskUpdate 超时)没有再出现过一次。
验证过程中观察到的既有环境性抖动(与本变更无关,也不由本变更修复)
verify-capture.test.js > renders 256-colour and truecolor via the default-grey fallback 断言 SVG→PNG 栅格化结果中存在精确的 #d4d4d4 像素。在这台自托管 runner 上,即使不带本修复它也会抖动——单独运行该文件的 A/B 对照:基线提交上 10 次失败 6 次,带修复 10 次失败 4 次(统计上无差异)。这台机器完全没有安装字体(/usr/share/fonts 不存在,也没有 fontconfig 配置),导致 pango/librsvg 的字形栅格化在那里是非确定性的。发布 quality 任务运行在 GitHub 托管的 ubuntu-latest 上,该镜像自带 DejaVu 字体,且 v0.22.1 那次运行通过了这个测试(它死于 RPC 错误,当时所有测试全绿)。这是 runner 环境造成的假象,在基线提交上就已存在,不属于本修复的范围;在 runner 集群上安装字体(或另行加固该像素断言)可以作为后续事项。
验证
npm run build— 通过(退出码 0)npm run typecheck— 通过(退出码 0)npm run lint— 通过(退出码 0)npx eslint scripts/tests/test-setup.ts+npx prettier --check scripts/tests/test-setup.ts— 通过npm run test:scripts— 全量套件运行见证据表:2 次完全通过,另有 3 次运行的唯一失败是已经过 A/B 证明与本变更无关的"无字体 runner"像素抖动;5 次运行中 RPC 超时错误为零- 变异探针 — 移除新增钩子会重新产生与发布失败完全一致的结果(退出码 1,
Timeout calling "onTaskUpdate");恢复后套件回到退出码 0。既有的 219 个测试的qwen-autofix-workflow.test.js套件就是随提交一起存在的见证测试。 - 不适用:各 workspace 包的单元测试(变更仅限于脚本测试的 setup 文件,任何 workspace 配置都不加载它)、集成测试(未触及任何打包 CLI 行为)、
npm run generate:settings-schema(未触及任何设置源)。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
|
|
|
Thanks for the PR! Template looks good ✓ — all required headings present, with before/after evidence and the Chinese translation. Problem: observed, not theoretical. Linked #10037 is a real release failure: the v0.22.1 Release run (32860613912) died on Direction: aligned. This removes the root cause instead of widening the #9728 exemption to Linux, which would mask real unhandled errors on exactly the lane it was kept strict for. CHANGELOG: no reference needed — this is CI plumbing that unblocks releases, squarely in scope. Size: not applicable — one test-infra file ( Approach: the scope feels right; this is the minimal root-cause fix. The alternatives are worse: widening the ignore-exemption masks real errors, vitest's 60s RPC timeout is not configurable, and splitting the heavy suite wouldn't generalize to the other script suites. Capturing Risk: no elevated risk signals — no high-revert-correlation paths touched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ ——所需标题齐全,含 before/after 证据和中文翻译。 问题:已观测到,不是理论性的。关联的 #10037 是一次真实的发布失败:v0.22.1 的 Release 运行(32860613912)死于 方向:对齐。这消除根因,而不是把 #9728 的豁免扩大到 Linux——扩大豁免会在恰好被刻意保留严格信号的通道上掩盖真实的未处理错误。CHANGELOG:无需引用——这是解除发布阻塞的 CI 基建修复,完全在范围内。 规模:不适用——单个测试基建文件( 方案:范围合理,这是最小的根因修复。备选方案更差:放宽忽略豁免会掩盖真实错误;vitest 的 60 秒 RPC 超时不可配置;拆分重型套件对其他脚本套件也不通用。在 setup 文件加载时捕获 风险:无升级风险信号——未触及与 revert 高相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent baseline first: for "synchronous script suites block the worker event loop past vitest's fixed 60s progress-reporting RPC — fatally on Linux", the minimal root-cause fix is a per-test event-loop yield in the shared scripts setup file, backed by a timer captured before any test can install fake timers. That is exactly what this PR does; I found no simpler path. The alternatives are worse in specific ways: widening the Linux exemption masks real errors, vitest's RPC timeout is not configurable, and splitting the heavy suite wouldn't generalize. The implementation is clean:
No blocking findings. Test evidence — the PR's own CI, via the APIThis is an unattended CI run, so no PR code was built or executed here; the evidence is the PR's own checks on the reviewed commit. Snapshot taken just now — the lanes that matter are still running, and the table below is wrapped so the finalize pass can update it when they land. The CI lanes are themselves the oracle for this fix: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 No failed checks in this snapshot. Not verified locally: the author's self-hosted runs quoted above — unattended CI runs never execute PR code. Real-scenario (tmux) testing: N/A — test-harness change with no user-visible behavior. 中文说明代码审查:先写独立基线。对"同步脚本套件阻塞 worker 事件循环、超过 vitest 固定 60 秒进度上报 RPC(Linux 上致命)"这个问题,最小的根因修复是在共享脚本 setup 文件里做每测试一次的事件循环让出,并用一个在任何假计时器安装之前捕获的计时器支撑——本 PR 做的正是这个,我没有找到更简的路径。备选方案各有明确的劣势:放宽 Linux 豁免会掩盖真实错误,vitest 的 RPC 超时不可配置,拆分重型套件不通用。 实现干净: 测试证据:无人值守 CI 运行,未构建或执行任何 PR 代码;证据取自被审查提交上 PR 自身的 check。快照刚刚获取——关键通道仍在运行,表格已加标记,供 CI 落定后自动更新。CI 通道本身就是本修复的预言: 本地未验证:上文引用的作者自托管运行结果——无人值守 CI 运行从不执行 PR 代码。 真实场景(tmux)测试:N/A——测试基建变更,无用户可见行为。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — observed release failure, exact root-cause fix in 11 lines, strict-Linux error signal preserved; the only thing outstanding is CI itself. Stepping back: this is the kind of PR the gate should wave through quickly. The problem is not a hypothesis — v0.22.1's release run died on the Quality Checks job with every test green, confirmed through the run API. The fix matches my independent proposal exactly, and I found nothing simpler: yield once between tests on a timer captured before fake timers can exist, so vitest's fixed 60s progress RPC always drains. It deliberately does not take the easy route of widening the platform exemption, which would have silenced real unhandled errors on the very lane the release died on. The diff carries nothing extra, and the property I care about most survives it: a future single test blocking for over 60s would still trip the timeout, and test failures stay fatal everywhere. In six months nobody will remember this file — that is the correct outcome for a setup hook. Approval is deferred until CI lands green on 中文说明置信度 5/5:已观测到的发布失败,11 行的精确根因修复,Linux 严格错误信号得以保留;唯一未决项是 CI 本身。 退一步看:这是门禁应当快速放行的那类 PR。问题不是假设——v0.22.1 的发布运行在所有测试全绿的情况下死于 Quality Checks 任务,已通过运行 API 核实。修复与我的独立方案完全一致,且我没有找到更简的路径:在测试之间用一个在任何假计时器存在之前捕获的计时器让出一次,使 vitest 固定 60 秒的进度 RPC 总能被处理。它刻意没有走放宽平台豁免的捷径——那会在发布失败的通道上静默掉真实的未处理错误。diff 没有任何多余改动;最关键的性质得以保留:未来若单个测试阻塞超过 60 秒仍会触发超时,测试失败在所有平台上仍然致命。 六个月后没人会记得这个文件——对一个 setup 钩子来说这是正确的结局。 批准推迟到 CI 在被审查提交上全绿之后(本次审查时 Test 通道仍在运行);finalize 阶段会更新 CI 表格,随后提交绑定该提交的批准。 — Qwen Code · qwen3.8-max Reviewed at |
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.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test / empirical run of the scripts suite under the new hook — the changed file sits outside every npm workspace and no node_modules was available, so no repo suite executed the new hook in this review; it was exercised only in the verifier's isolated vitest 3.2.7 probe, and CI's Test lanes are the remaining oracle (the windows lane fails on unrelated pre-existing package-suite failures).
Not explored to full depth (tool budget reached): "agent 5": empirical run of the scripts suite under the new hook — the worktree and parent checkout have no node_modules , and a full monorepo npm ci in the shared revi….
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test / empirical run of the scripts suite under the new hook — the changed file sits outside every npm workspace and no node_modules was available, so no repo suite executed the new hook in this review; it was exercised only in the verifier's isolated vitest 3.2.7 probe, and CI's Test lanes are the remaining oracle (the windows lane fails on unrelated pre-existing package-suite failures)。
未探索到全部深度(达到工具调用预算):"agent 5":empirical run of the scripts suite under the new hook — the worktree and parent checkout have no node_modules , and a full monorepo npm ci in the shared revi…。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // run exit 1 with every test green. Yielding between tests releases the | ||
| // loop so the RPC response is always processed in time. |
There was a problem hiding this comment.
[Suggestion] The closing sentence here overstates the guarantee the code provides. The yield runs between tests, so what it actually establishes is that any continuous event-loop stall is bounded by a single test's synchronous work — a single synchronous block over ~60s, e.g. in a beforeAll hook or at module level, can still trip vitest's fixed 60s worker→main onTaskUpdate RPC timeout on Linux (testTimeout: 30_000 cannot preempt a synchronous body). If a future change adds >60s of continuous synchronous work to a single beforeAll or module-level block, the exact v0.22.1 failure this PR fixes — [vitest-worker]: Timeout calling "onTaskUpdate", every test green, exit 1 — recurs with the fix in place, and the word "always" then misdirects the debugging. Reproduced deterministically in an isolated vitest 3.2.7 probe with these two added lines as the setup file: a 70s synchronous beforeAll exits 1 with Unhandled Error: [vitest-worker]: Timeout calling "onTaskUpdate", while the same workload spread across tests exits 0. Suggest stating the actual invariant instead:
| // run exit 1 with every test green. Yielding between tests releases the | |
| // loop so the RPC response is always processed in time. | |
| // run exit 1 with every test green. Yielding between tests bounds any | |
| // continuous stall to a single test, so the RPC response drains well before | |
| // the deadline. (A single test, beforeAll, or module-level block stalling | |
| // for 60s would still trip it: testTimeout cannot interrupt synchronous | |
| // bodies.) |
中文说明
此处结尾的表述夸大了代码实际提供的保证。该让出只在测试之间运行,因此它建立的不变量是:任何连续的事件循环停滞都被限制在单个测试的同步工作内——而单个超过约 60 秒的同步块(例如 beforeAll 钩子或模块级代码)在 Linux 上仍然可能触发 vitest 固定的 60 秒 worker→主进程 onTaskUpdate RPC 超时(testTimeout: 30_000 无法打断同步执行的代码体)。如果未来的改动在单个 beforeAll 或模块级块中加入超过 60 秒的连续同步工作,本 PR 所修复的 v0.22.1 失败——[vitest-worker]: Timeout calling "onTaskUpdate"、所有测试全绿、退出码 1——会在本修复存在的情况下复现,而 "always" 的措辞届时会误导排查。我在以这两行新增代码作为 setup 文件的隔离 vitest 3.2.7 探针中确定性地复现了这一点:70 秒的同步 beforeAll 以 Unhandled Error: [vitest-worker]: Timeout calling "onTaskUpdate"、退出码 1 结束,而分摊到各测试之间的相同工作量退出码为 0。建议改为表述实际的不变量(见上方 suggestion 代码块)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/10 轮)。改动内容与我反驳保留之处如下: Review round summary — PR #10050 (issue #10037)Commit this round: No base merge performed ( Feedback points and dispositions[rc:3858017935] scripts/tests/test-setup.ts:27 — [Suggestion] the comment's closing sentence overstates the guarantee → RESOLVEDThe finding claims the closing sentence "Yielding between tests releases the loop so the RPC response is always processed in time" overstates what the code establishes: the yield runs between tests, so a single continuous synchronous block over ~60s (a Reproduced before implementing with an isolated probe using this repo's own vitest 3.2.7 (same version the reviewer probed) and the PR's exact two added lines as the setup file:
The claim holds empirically. Fix applied: replaced the overstated closing sentence with the actual invariant (the reviewer's suggested wording, integrated into the existing paragraph): the yield bounds any continuous stall to a single test, and a single test, [rv:5024879092] review body — "Partially reviewed — gaps disclosed" → No actionInformational: a COMMENTED review disclosing what the reviewer did not cover (no build-and-test / empirical scripts-suite run inside the review). It carries no findings of its own; the single inline suggestion it produced is handled above. Failed check — Test (windows-latest, Node 22.x): FAILURE → assessed, not actionable from this PREvidence gathered (no logs were available to this runner and the exact windows lane cannot be reproduced on Linux):
Conclusion: there is no in-scope change this PR could make to address a windows package-suite failure; fixing whatever windows-only package test is failing belongs in a separate change. Flagging it here so the thread keeps visibility until the lane is green or the failure is filed separately. Environmental flake disclosed (verification artifact, not a finding)Both full VerificationCommands actually run this round (post-fix unless noted):
中文说明审查轮次总结 — PR #10050(issue #10037)本轮提交: 未执行 base 合并( 反馈点及处理[rc:3858017935] scripts/tests/test-setup.ts:27 — [Suggestion] 注释结尾句夸大了保证 → 已解决该发现指出:结尾句 "Yielding between tests releases the loop so the RPC response is always processed in time" 夸大了代码实际建立的保证——让出只在测试之间运行,因此单个超过约 60 秒的连续同步块( 实现前已复现:使用本仓库自带的 vitest 3.2.7(与审查者探针相同的版本)、以本 PR 新增的那两行作为 setup 文件的隔离探针:
该主张经实证成立。已修复:用实际不变量替换夸大的结尾句(采用审查者建议的措辞,并入现有段落):让出把任何连续停滞限制在单个测试内;单个测试、 [rv:5024879092] 审查正文 — "部分审查——缺口已披露" → 无需处理信息性内容:一条 COMMENTED 审查,披露了审查者未覆盖的部分(未在审查内部做 build-and-test / 脚本套件的实证运行)。它本身没有发现;其产生的唯一行内建议已在上面处理。 失败检查 — Test (windows-latest, Node 22.x): FAILURE → 已评估,本 PR 无法处理收集的证据(本 runner 拿不到日志,且 windows 通道无法在 Linux 上复现):
结论:本 PR 不存在任何范围内(in-scope)的变更可以解决 windows 包套件失败;修复那个 windows 专属的包测试失败应属于单独的变更。在此明确标出,保持该问题的可见性,直到该通道变绿或该失败被单独建档。 环境性抖动披露(验证过程中的附带现象,非审查发现)本轮两次完整的 验证本轮实际执行的命令(除注明外均为修复后):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — the full scripts suite was not executed in this review (the changed file sits outside every npm workspace); the hook ran only in the verifier's isolated vitest 3.2.7 probe, and CI's Test lanes (which run test:scripts) are still pending.
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — the full scripts suite was not executed in this review (the changed file sits outside every npm workspace); the hook ran only in the verifier's isolated vitest 3.2.7 probe, and CI's Test lanes (which run test:scripts) are still pending。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // for 60s would still trip it: testTimeout cannot interrupt synchronous | ||
| // bodies.) | ||
| const realSetTimeout = setTimeout; | ||
| beforeEach(() => new Promise((resolve) => realSetTimeout(resolve, 0))); |
There was a problem hiding this comment.
[Suggestion] No automated test pins the yield invariant this hook establishes. The reviewer test plan performs the mutation check manually ("delete the beforeEach yield … the failure must come back"), so once this PR merges, nothing in CI stops a future edit from silently removing the yield: a cleanup refactor, a restructuring into globalSetup, or a vitest upgrade that changes setup-file hook semantics could remove or neutralise the hook with no test failing. The regression would first surface at the next release, when a long synchronous script suite (the autofix-workflow suite alone stalls ~66s) re-triggers vitest's fixed 60s onTaskUpdate RPC timeout — the exact v0.22.1 release failure this PR fixes (exit 1 with every test green on the Linux release lane; silently swallowed on the non-Linux lanes by dangerouslyIgnoreUnhandledErrors).
Verified by probe (vitest 3.2.7 in an isolated scratch tree, this repo's real scripts/tests/test-setup.ts as setupFiles, two-test oracle): with the hook — Tests 2 passed (2); with the parent-commit setup file (no yield) — FAIL event-loop-yield.test.js > observes the event loop turned between tests / AssertionError: expected false to be true. Deterministic flip.
Suggested fix: add a small scripts/tests/event-loop-yield.test.js — test 1 arms a flag from a real macrotask callback (setTimeout(..., 0) — not process.nextTick, which is a microtask), test 2 asserts the flag fired. The probe verified this pair fails on the parent commit and passes with the hook.
中文说明
目前没有任何自动化测试来固定本钩子所确立的让出(yield)不变量。评审者测试计划中的变异检查是手动执行的("删除 beforeEach 让出……失败必须复现"),因此本 PR 合并后,CI 中没有任何东西能阻止未来的修改悄悄移除该让出:清理性重构、重构为 globalSetup、或改变 setup 文件钩子语义的 vitest 升级,都可能在没有任何测试失败的情况下移除或使该钩子失效。回归会在下一次发布时首次暴露:一个长时间同步运行的脚本套件(仅 autofix-workflow 套件就会停滞约 66 秒)将再次触发 vitest 固定的 60 秒 onTaskUpdate RPC 超时——也就是本 PR 修复的 v0.22.1 发布失败(Linux 发布通道上所有测试全绿却退出码 1;非 Linux 通道则被 dangerouslyIgnoreUnhandledErrors 静默吞掉)。
已通过探针验证(在隔离的临时树中使用 vitest 3.2.7,以本仓库真实的 scripts/tests/test-setup.ts 作为 setupFiles,采用双测试预言机):带钩子 —— Tests 2 passed (2);换成父提交的 setup 文件(无让出)—— FAIL event-loop-yield.test.js > observes the event loop turned between tests / AssertionError: expected false to be true。确定性翻转。
建议修复:新增一个小的 scripts/tests/event-loop-yield.test.js —— 测试 1 从一个真实宏任务回调(setTimeout(..., 0)——而不是微任务 process.nextTick)中置位一个标志,测试 2 断言该标志已置位。探针已验证该测试对在父提交上失败、带钩子时通过。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/10 轮)。改动内容与我反驳保留之处如下: Review round summary — PR #10050 (issue #10037)Commit this round: No base merge performed ( Feedback points and dispositions[rc:3858736314] scripts/tests/test-setup.ts:32 — [Suggestion] no automated test pins the yield invariant this hook establishes → RESOLVEDThe finding is real and worth the diff: today the invariant (the worker event loop turns between tests, so vitest's fixed 60s worker→main Implemented exactly the suggested shape: new Reproduced + witness-verified by mutation probe on this checkout (this repo's vitest 3.2.7,
The test also ran green inside the full [rv:5025682468] review body — "Partially reviewed — gaps disclosed" → No actionInformational COMMENTED review disclosing what the reviewer did not execute (the full scripts suite under this config; CI's Test lanes still pending at review time). It carries no findings of its own; the single inline suggestion it produced is handled above. Failed check — Test (windows-latest, Node 22.x): FAILURE → assessed, not actionable from this PR (unchanged from round 1, re-verified)Same lane, same signature as round 1, failing on the pre-round head
Fixing whatever windows-only package test is failing belongs in a separate change; keeping this flagged in the thread until the lane is green or the failure is filed separately. Environmental flake disclosed (verification artifact, not a finding)This round's full VerificationCommands actually run this round, in the checkout at the committed head unless noted:
中文说明审查轮次总结 — PR #10050(issue #10037)本轮提交: 未执行 base 合并( 反馈点及处理[rc:3858736314] scripts/tests/test-setup.ts:32 — [建议] 没有自动化测试固定本钩子所确立的让出(yield)不变量 → 已解决该发现真实存在且值得这部分 diff:目前该不变量(worker 事件循环在测试之间转动,使 vitest 固定的 60 秒 worker→主进程 按建议的形态精确实现:新增 在本检出上以变异探针复现并验证了见证有效性(本仓库的 vitest 3.2.7,
该测试也在完整的 [rv:5025682468] 审查正文 — "部分审查——缺口已披露" → 无需处理信息性的 COMMENTED 审查,披露了审查者未执行的部分(未在本配置下完整运行脚本套件;审查时 CI 的 Test 通道仍在运行)。它本身没有发现;其产生的唯一行内建议已在上面处理。 失败检查 — Test (windows-latest, Node 22.x): FAILURE → 已评估,本 PR 无法处理(与第 1 轮结论一致,本轮已重新核验)与第 1 轮相同的通道、相同的失败特征:在轮前头部提交
修复那个 windows 专属的包测试失败属于单独的变更;在此继续标出,保持可见性,直到该通道变绿或该失败被单独建档。 环境性抖动披露(验证过程中的附带现象,非审查发现)本轮完整的 验证本轮实际执行的命令(除注明外均在已提交的头部检出上运行):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: 🤖 No action this round (round 3/10) — every feedback point triaged, none actionable in code. Review round summary — PR #10050 (issue #10037)No commit this round. The working tree is unchanged at head Feedback points and dispositions[rv:5026102240] review body — "
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
scripts/tests/test-setup.ts:31 — [probe] no automated test pins the setup-load-time capture of realSetTimeout against fake-timer refactors
中文说明
无阻断问题。LGTM!✅
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Released in v0.22.2. |
What this PR does
Adds a one-line global hook to the script-test setup that yields the event loop to a real timer before every test. Script suites dominated by synchronous
spawnSynctests (most notably the autofix workflow suite: 219 tests, ~66s) used to keep a vitest worker's event loop blocked continuously for the entire test file, which made vitest's fixed 60-second worker→main progress-reporting RPC time out and surface as an unhandled[vitest-worker]: Timeout calling "onTaskUpdate"error. The timer backing the yield is captured when the setup file loads, sovi.useFakeTimers()inside a test can never intercept it.Why it's needed
The v0.22.1 release died on the
qualityjob with every test green:npm run test:releaseends innpm run test:scripts, and on Linux that unhandled RPC-timeout error is fatal — the scripts vitest config deliberately ignores unhandled errors only off Linux (#9728 exempted the macOS/Windows lanes, where the same stall was observed, precisely so the ubuntu lane and Linux local runs keep the unhandled-error signal). This fix removes the root cause instead of widening that exemption: any continuous event-loop stall is now bounded by a single test, RPC responses drain within milliseconds, and genuine unhandled errors stay fatal everywhere.Reviewer Test Plan
How to verify
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow. Expected: all 219 tests pass, then one unhandledError: [vitest-worker]: Timeout calling "onTaskUpdate", exit code 1 (the exact release failure).beforeEachyield fromscripts/tests/test-setup.tsand rerun step 1's command — the failure must come back.npm run test:scripts(expect 65 files, 1738 passed / 16 skipped; note the pre-existing font-dependentverify-capturepixel assertion flakes on runners with no fonts installed — it flakes identically with and without this change, see Risk & Scope).Evidence (Before & After)
N/A (test-harness change, no user-visible behavior).
Before (parent commit, heavy suite alone):
Test Files 1 passed (1) / Tests 219 passed (219) / Errors 1 error—Error: [vitest-worker]: Timeout calling "onTaskUpdate"— exit 1.After (this PR, same command):
Test Files 1 passed (1) / Tests 219 passed (219)— no errors section — exit 0. Fullnpm run test:scripts: two fully green runs (1738 passed), plus three runs whose only failure was the unrelated pre-existing font flake below; zero RPC-timeout errors in all five runs.Tested on
Environment (optional)
Local self-hosted Linux runner (64 cores), plus a
taskset -c 0-3run as a 4-core CI surrogate. The runner has no fonts installed, which causes the pre-existingverify-capturepixel flake documented below; GitHub-hostedubuntu-latestrunners used by the releasequalityjob ship fonts and do not show it.Risk & Scope
verify-capture.test.js > renders 256-colour and truecolor via the default-grey fallbackflakes on this font-less self-hosted runner with or without this change (A/B: 6/10 failures without vs 4/10 with); it asserts exact rasterised pixels and needs a resolvable font. Candidate follow-up: install fonts on the fleet.Linked Issues
Fixes #10037
中文说明
本 PR 做了什么
在脚本测试的 setup 文件中增加一个一行的全局钩子:在每个测试之前用真实计时器让出一次事件循环。以同步
spawnSync测试为主的脚本套件(尤其是 autofix workflow 套件:219 个测试,约 66 秒)过去会让 vitest worker 的事件循环在整个测试文件期间被连续阻塞,导致 vitest 固定的 60 秒 worker→主进程进度上报 RPC 超时,并以未处理的[vitest-worker]: Timeout calling "onTaskUpdate"错误浮现。支撑这次让出的计时器在 setup 文件加载时捕获,因此测试内部的vi.useFakeTimers()永远无法拦截它。为什么需要
v0.22.1 发布在所有测试全绿的情况下死于
quality任务:npm run test:release的最后一步是npm run test:scripts,而在 Linux 上这个未处理的 RPC 超时错误是致命的——脚本 vitest 配置刻意只在非 Linux 平台忽略未处理错误(#9728 豁免了 macOS/Windows 通道,因为同样的停滞在那里被观察到;正是为了让 ubuntu 通道和 Linux 本地运行保留未处理错误信号)。本修复消除根因,而不是放宽那个豁免:现在任何连续的事件循环停滞都被限制在单个测试内,RPC 响应在几毫秒内即可被处理,真正的未处理错误在所有平台上仍然是致命的。评审者测试计划
如何验证
npx vitest run --config ./scripts/tests/vitest.config.ts qwen-autofix-workflow。预期:219 个测试全部通过,然后出现一个未处理的Error: [vitest-worker]: Timeout calling "onTaskUpdate",退出码 1(与发布失败完全一致)。scripts/tests/test-setup.ts中的beforeEach让出钩子,重新运行第 1 步的命令——失败必须复现。npm run test:scripts(预期 65 个文件,1738 通过 / 16 跳过;注意既有的、依赖字体的verify-capture像素断言在没有安装字体的 runner 上会抖动——该抖动在本变更前后完全一致,见"风险与范围")。证据(修改前后)
N/A(测试基础设施变更,无用户可见行为)。
修改前(父提交,单独运行最重套件):
Test Files 1 passed (1) / Tests 219 passed (219) / Errors 1 error——Error: [vitest-worker]: Timeout calling "onTaskUpdate"—— 退出码 1。修改后(本 PR,同一命令):
Test Files 1 passed (1) / Tests 219 passed (219)—— 无 Errors —— 退出码 0。完整npm run test:scripts:两次完全通过(1738 通过),另有三次运行的唯一失败是下述无关的既有字体抖动;五次运行中 RPC 超时错误为零。测试环境
环境(可选)
本地自托管 Linux runner(64 核),另有一次
taskset -c 0-3运行作为 4 核 CI 的替代验证。该 runner 没有安装任何字体,这正是下文记录的既有verify-capture像素抖动的原因;发布quality任务所用的 GitHub 托管ubuntu-latestrunner 自带字体,不会出现该问题。风险与范围
verify-capture.test.js > renders 256-colour and truecolor via the default-grey fallback在这台无字体的自托管 runner 上无论是否带本变更都会抖动(A/B:不带变更 10 次失败 6 次,带变更 10 次失败 4 次);它断言精确的栅格化像素,需要一个可解析的字体。后续事项候选:在 runner 集群上安装字体。关联 Issue
Fixes #10037