test(cli): stamp the background-work duration fixture when the test runs, not when it loads (#10881) - #10889
Conversation
…uns, not when it loads (#10881) `describeBlockingBackgroundWork` measures the duration it renders against the `Date.now()` it takes when it runs, but the fixture's 21h-old `startTime` came from a `now` captured in the describe body — at collection. The gap between the two is however long vitest spent collecting the shard, and `formatDuration` floors to whole seconds, so a gap of one second or more renders `21h 1s` and fails `(running 21h)`. Release run 33713579913 (v0.23.0) collected for 2260s on a contended host and reddened the quality gate. Reproduced by shifting that describe-scope stamp back by the same 2260s: Expected: "(running 21h)" Received: " [bg_run] Explore: research the codebase (running 21h 37m 40s)" Stamping the fixture at call time leaves the file green under the same shift, with the assertion unchanged. Same class as the run-ledger fix in 0e3094e, which that commit tied to this run. An AST sweep of all 2141 package test files for live clock reads that execute during collection found 15; the rest are class methods, wide-margin fixtures, or ordering-only fixtures that a gap cannot flip.
Release failure triage: v0.23.0,
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The v0.23.0 release reddened on the aggregate Direction: aligned. This is a release-gate deflake — the gate exists precisely so a fixture like this cannot silently redden a release, and the fix removes sensitivity to the collect→execute gap rather than weakening the check. CHANGELOG: no direct reference (test-only change, nothing ships to users), though the background-work surface it exercises is actively developed upstream. Size: no core-module paths touched — one test file, +5/−1, zero production lines. Tier gates not applicable. Approach: the scope is right. One fixture gets stamped at call time; the assertion Risk: no elevated risk signals — the only changed file is a test file; no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到,不是理论问题。v0.23.0 发布在聚合的 方向:对齐。这是一次发布关卡的 deflake —— 关卡的意义正是防止这类夹具悄悄把发布染红,而本修复消除的是对"收集→执行"时间差的敏感性,不是放宽检查。CHANGELOG:无直接引用(纯测试改动,不面向用户),但其覆盖的后台任务列表界面在上游正活跃开发。 规模:未触及核心模块路径 —— 单个测试文件,+5/−1,生产代码零行。分级门槛不适用。 方案:范围合理。只有一个夹具改为在调用时取时间戳;断言 风险:无升级风险信号 —— 唯一改动文件是测试文件,未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal for "fixture stamped at collection time, compared against a clock read taken at execution time" was exactly what this PR does: move the stamp to call time. The alternatives are worse — fake timers ( I then verified the mechanism against the source rather than taking the description at face value:
No blockers, no convention violations. One non-blocking residual, acknowledged in the PR's own risk section: a wall-clock step backwards between the fixture's read and the function's read could render just under 21h — inherent to any real-clock test, vanishingly unlikely on CI runners. TestingThis lane never executes PR-derived code; the evidence below is the PR's own CI read via the API. At the reviewed commit the unit/lint lanes are still running (~30 min suite — no polling), and there are no red checks so far. The macOS/Windows/CLI-integration skips are routine: the same skip pattern appears on #10878, merged in this same lane earlier. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Honest limits of this pass: the before/after reproduction and the mutation-probe transcript (shift the stamp back 2260s → fails before, passes after) are the author's results from the triage comment above, not independently re-run here — this lane cannot execute PR code. The mechanism itself does not depend on them: the arithmetic of the two Sandboxed verification would settle what CI here cannot: 中文说明代码审查在读 diff 之前,我对"夹具在收集时取时间戳、却与执行时的时钟读取比较"这一问题的独立方案,与本 PR 完全一致:把取时间戳移到调用时。备选方案都更差——假时钟( 随后我对照源码核实了机制,而不是照单接受描述:
无阻塞项,无规范违规。一个非阻塞的残余风险(PR 自己的风险一节也已承认):夹具读取与函数读取之间墙钟若被回拨,渲染可能略小于 21h —— 这是所有真实时钟测试的固有属性,在 CI 上几乎不可能发生。 测试本通道从不执行 PR 派生代码;以下为通过 API 读取的 PR 自身 CI 证据。在受审提交上,单测/静态检查通道仍在运行(套件套约 30 分钟——不做轮询),目前没有红色检查。macOS/Windows/CLI 集成被跳过属正常:同一通道稍早合并的 #10878 呈现完全相同的跳过模式。 本次审查的诚实边界:before/after 复现与变异探针记录(时间戳回拨 2260 秒 → 修复前失败、修复后通过)是上方排查评论中作者的结果,未在此独立重跑——本通道不能执行 PR 代码。但机制本身不依赖这些记录:两次 沙箱验证可以解决此处 CI 无法解决的问题: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — observed release failure, deterministic reproduction with exact arithmetic, precedent fix already merged for the same class, and a one-line diff that pins the fixture to the right clock phase without touching the assertion. My independent proposal for this class of flake was identical to what landed here — stamp the fixture in the same phase the code under test measures from — and I couldn't find a simpler path the PR missed. The alternatives (fake timers, relaxed assertion) are both larger and worse. Every line of the diff is necessary: one fixture, one explanatory comment, and the sweep results deliberately kept out of the diff. Six months from now, the comment telling the next editor why this fixture can't return to the shared Pattern check: the author carries nine open PRs, all from the project's own automation lane — this one stands on its own evidence, and the evidence is the strong kind: a failed release run, a repro where the received value is 21h plus exactly the injected gap, and a prior fix (#10878) proving the class real. And this is not the gate being soft — a test-only change that weakens nothing, from a lane that just watched this exact failure mode redden a release twice, is what the gate should wave through quickly. Verdict: approve — but the unit/lint lanes are still in flight on the reviewed commit, so approval is deferred until CI lands green on 中文说明置信度:5/5 —— 已观测到的发布失败、精确算术吻合的确定性复现、同类问题已有先行修复合入,且本次一行 diff 将夹具锚定到正确的时钟阶段而未触碰断言。 我对这类 flake 的独立方案与最终落地的完全一致 —— 让夹具在与被测代码相同的阶段取时间戳 —— 我也没找到比这更简单的路径。备选方案(假时钟、放宽断言)都更大、更差。Diff 中每一行都是必要的:一个夹具、一条解释性注释,排查结果被刻意排除在 diff 之外。六个月后,正是这条告诉后来者"该夹具为何不能回到共享 模式检查:该作者名下有九个开放 PR,均来自项目自身的自动化通道 —— 本 PR 凭自身证据成立,而证据是强证据:一次失败的发布 run、received 值恰好等于 21 小时加注入间隔的复现,以及证明该类问题真实存在的先行修复(#10878)。这也不是关卡放水:一个不削弱任何检查的纯测试改动,来自刚刚目睹同一失败模式两次染红发布的通道,正是关卡应当快速放行的对象。 结论:批准 —— 但单测/静态检查通道在受审提交上仍在运行,故批准推迟到 CI 在该提交上全绿之后( — Qwen Code · qwen3.8-max Reviewed at |
Local verification of PR #10889 — real release-run logs + A/B harnessI rebuilt this locally in two worktrees (base VerdictThe one-line change is correct and worth merging — it removes a genuine sub-second timing hazard from a fixture, the assertion is untouched, lint/prettier are clean, and the class sweep matches the PR's own inventory. But the story around it does not match the run logs, and one consequence is actionable before merge:
What I ran
B is the experiment I'd point reviewers at: it reproduces the flip at exactly the first whole second without editing the fixture, and the PR arm passes at every gap. C is the one that settles "is this fixture really exposed?": at 65× oversubscription of one core the untouched base fixture flips on its own ( ScreenshotsRelease run 33713579913 — what the job logs say (attempt 1 shard 1/3 with this file passing; attempt 1 & 2 shard 3/3 failures; attempt 4 still red): Experiment A — reviewer test plan replayed on both arms: Experiment B — collect→execute gap injected at config level, fixture untouched: Experiment C — real CPU contention (taskset + busy loops), no fixture edit: Experiment D — class sweep at PR head: Non-blocking suggestions (for the author / whoever merges)
Reproduce# worktrees: base = 19182d08 (merge-base), pr = d0de394a; deps symlinked from a built checkout
cd packages/cli
CI=true npx vitest run src/ui/utils/backgroundWorkUtils.test.ts # both arms: 26 passed
# A: sed -i '175s/const now = Date.now();/const now = Date.now() - 2_260_000;/' <file> → base 1 failed, pr 26 passed
# B: extra setup file with `beforeAll(() => sleep(Number(process.env.GAP_MS)))`, merged via mergeConfig(base, { test: { setupFiles: [...] } })
GAP_MS=1100 CI=true npx vitest run --config vitest.delay.config.ts src/ui/utils/backgroundWorkUtils.test.ts
# logs: gh api repos/QwenLM/qwen-code/actions/jobs/<100523858162|100523858105|100532938357|100569275626>/logs中文说明PR #10889 本地验证 —— 真实发布 run 日志 + A/B 环境我在本地建了两个 worktree(base 结论这一行改动本身是对的,值得合入 —— 它消除了夹具里一个真实存在的亚秒级时序隐患,断言未动,lint/prettier 干净,同类普查与 PR 自己的清单一致。但围绕它的叙述与 run 日志不符,其中一条需要在合并前处理:
我跑了什么
我最推荐评审者看 B:它在不改夹具的情况下精确复现了"第一个整秒翻转",而 PR 臂在所有间隔下都通过。 C 回答的是“这个夹具是不是真的暴露”:单核 65 倍超订下,未改动的 base 夹具自行翻转( 截图发布 run 33713579913 —— 作业日志实际内容(attempt 1 的 1/3 分片里本文件通过;attempt 1、2 的 3/3 失败;attempt 4 仍红): 实验 A —— 在两条臂上复刻评审测试计划: 实验 B —— 配置级注入收集→执行间隔,夹具不改: 实验 C —— 真实 CPU 争用(taskset + 忙循环),不改夹具: 实验 D —— PR head 上的同类普查: 非阻塞建议(给作者 / 合并者)
复现# worktree:base = 19182d08(merge-base),pr = d0de394a;依赖从已构建的检出 symlink
cd packages/cli
CI=true npx vitest run src/ui/utils/backgroundWorkUtils.test.ts # 两臂均 26 passed
# A:sed -i '175s/const now = Date.now();/const now = Date.now() - 2_260_000;/' <file> → base 1 failed,pr 26 passed
# B:额外 setup 文件 `beforeAll(() => sleep(Number(process.env.GAP_MS)))`,用 mergeConfig(base, { test: { setupFiles: [...] } }) 合入
GAP_MS=1100 CI=true npx vitest run --config vitest.delay.config.ts src/ui/utils/backgroundWorkUtils.test.ts
# 日志:gh api repos/QwenLM/qwen-code/actions/jobs/<100523858162|100523858105|100532938357|100569275626>/logs🤖 Generated with Claude Code — Claude Fable 5.1 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
🕐 Review received — an automatic review of the current head is still running, so this round is held until it lands (a push now would cancel it and discard its work, #8888). Your feedback stays queued for the next eligible round. 中文说明🕐 已收到评审 —— 当前 head 上仍有一轮自动 review 在运行,本轮暂缓(现在推送会取消该 review 并丢弃其工作,#8888)。反馈保持排队,等待下一次可运行的轮次处理。 |
|
Released in v0.23.0. |





What this PR does
Makes one unit-test fixture take its timestamp when the test runs instead of when the test file is collected, so the elapsed duration it asserts on is measured from the same phase as the code under test measures it from. The assertion is untouched and still exact.
It also sweeps the whole test tree for the same pattern and reports what it found, so the next release does not rediscover it one flake at a time.
Why it's needed
The v0.23.0 release failed on the aggregate quality gate. The lane that runs the workspace unit tests sharded spent 2260 seconds in vitest's collection phase on a contended host before executing any test.
That gap is fatal to a fixture whose clock reading is captured at collection but compared against a clock reading taken at execution. The background-work listing renders each blocking entry's elapsed duration from a read taken inside the function under test, while the fixture's 21-hour-old start time came from the enclosing
describebody. The duration formatter floors to whole seconds, so the expected(running 21h)only holds while the collect-to-execute gap stays under one second — at 2260 seconds it renders(running 21h 37m 40s)and the test fails on every retry.One instance of this class was already fixed on
mainfor the same run, in the review session ledger. This is the second instance, still live, in the same lane; without it the next slow-collecting release reddens the same gate for the same reason.Reviewer Test Plan
How to verify
The failure is deterministic once the collect-to-execute gap is reproduced, so no contended runner is needed.
describeBlockingBackgroundWork (#8741)block back by 2260 seconds — changeconst now = Date.now();toconst now = Date.now() - 2_260_000;.cd packages/cli && npx vitest run src/ui/utils/backgroundWorkUtils.test.ts. Expected: one failure,Expected: "(running 21h)"/Received: " [bg_run] Explore: research the codebase (running 21h 37m 40s)", 1 failed | 25 passed. The received value is 21h plus exactly the injected gap, which is the mechanism, not a coincidence.now.A reviewer should also confirm the assertion was not relaxed:
(running 21h)is asserted exactly as before, and the other fixtures in the block still use the shared collection-scopenowbecause they only establish relative ordering, which a uniform shift preserves.Evidence (Before & After)
N/A — no user-visible or TUI change. Test output for both directions is in the steps above and in the accompanying E2E report.
Tested on
Environment (optional)
N/A — unit tests only, run directly on the Linux CI runner checkout.
Risk & Scope
Linked Issues
Fixes #10881
中文说明
本 PR 做了什么
让一个单元测试夹具在测试运行时获取时间戳,而不是在测试文件被收集时获取,从而使它所断言的已运行时长与被测代码的度量来自同一个阶段。断言本身未改动,仍然是精确断言。
它还对整个测试树排查了同一种模式并给出结果,以免下一次发布再一个一个 flake 地重新发现它。
为什么需要
v0.23.0 发布在聚合的 quality 关卡上失败。分片运行工作区单元测试的那条通道,在一台资源紧张的机器上于 vitest 收集阶段耗时 2260 秒,之后才开始执行任何测试。
对于“时钟读取在收集时捕获、却与执行时取得的时钟读取做比较”的夹具,这段时间差是致命的。后台任务列表用被测函数内部取得的时钟读取来渲染每个阻塞条目的已运行时长,而夹具中那个“21 小时前”的开始时间来自外层
describe体。时长格式化函数向下取整到整秒,因此只有当收集到执行的间隔保持在 1 秒以内时,期望的(running 21h)才成立 —— 在 2260 秒下它渲染为(running 21h 37m 40s),于是每次重试都失败。这一类问题已有一个实例在
main上针对同一次 run 被修复,即 review 会话 ledger。这是第二个实例,仍然存在,且在同一条通道中;不修的话,下一次收集缓慢的发布会因为同样的原因再次弄红同一个关卡。评审测试计划
如何验证
一旦复现出收集到执行的间隔,该失败就是确定性的,因此不需要资源紧张的 runner。
describeBlockingBackgroundWork (#8741)块中收集阶段的时间戳回拨 2260 秒来模拟该发布 run 的间隔 —— 将const now = Date.now();改为const now = Date.now() - 2_260_000;。cd packages/cli && npx vitest run src/ui/utils/backgroundWorkUtils.test.ts。预期:一个失败,Expected: "(running 21h)"/Received: " [bg_run] Explore: research the codebase (running 21h 37m 40s)",1 failed | 25 passed。收到的值恰好是 21 小时加上注入的间隔,这说明的是机制本身,而不是巧合。now”的注释。评审者还应确认断言没有被放宽:
(running 21h)与之前完全一样是精确断言,并且该块中其余夹具仍然使用共享的收集阶段now,因为它们只用于确立相对顺序,而统一平移会保持该顺序。证据(前后对比)
N/A —— 无用户可见或 TUI 变更。两个方向的测试输出见上面的步骤以及随附的 E2E 报告。
测试环境
环境(可选)
N/A —— 仅单元测试,直接在 Linux CI runner 的检出上运行。
风险与范围
关联 Issue
Fixes #10881