test(web-shell): mock the provider module main.tsx actually imports in the boot test - #10526
Conversation
|
|
|
Thanks for the PR! Template looks good ✓ Problem: real, and verified. On Direction: clearly aligned — a test file that fails at collection on Size: not applicable — test-only, +1/−1, no production lines. Approach: the one-line re-point is the minimal fix. The mock's shape is unchanged, the new specifier resolves through the vitest alias to Risk: no elevated risk signals — test-only change, no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实存在,已验证。 方向:明确对齐—— 规模:不适用——仅测试,+1/−1,无生产代码行。 方案:一行改指向是最小修复。mock 形状不变,新说明符经 vitest alias 解析到 风险:无升级风险信号——仅测试改动,未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Code review: clean. Before looking at the diff I would have done exactly this — re-point the Testing evidence: this is an unattended CI run, so per the gate rules I did not build or execute any PR code — the evidence below is the PR's own CI, read through the API. The decisive signal is this PR's Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 For reference, the before-state is documented from Not verified: runtime behavior beyond the suite — there is none to verify; this is a test-collection fix with zero production change. 中文说明代码审查:干净。在看 diff 之前我的独立方案也正是如此——把 测试证据:这是无人值守的 CI 运行,按门禁规则未构建或执行任何 PR 代码——以下证据为本 PR 自身 CI 经 API 读取。决定性信号是本 PR 的 供参考,修复前状态来自 未验证:套件之外的运行时行为——无物可验;这是测试收集修复,零生产代码改动。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — verified problem, minimal fix, exactly what I would have done. This is the kind of PR that is easy to take for granted: one line, test-only, no drama. But it unblocks something concrete — the web-shell suite cannot go green on One CI workflow run is still in flight on the reviewed commit, so approval is deferred until CI lands green on 中文说明Confidence: 5/5 —— 问题已证实,修复极小,与我的独立方案完全一致。 这类 PR 容易被轻看:一行、仅测试、毫无戏剧性。但它解锁的东西很具体:只要这个文件在收集阶段失败,web-shell 套件在 被审提交上仍有一个 CI 工作流运行在进行中,因此批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
CanReader
left a comment
There was a problem hiding this comment.
Confirmed the diagnosis — the old mock really was pointing at a module nothing imports.
What I checked at this commit:
client/main.tsx:4importsDaemonWorkspaceProviderfrom@qwen-code/web-shell/daemon-react-sdk.packages/web-shell/package.jsonexports./daemon-react-sdk.@qwen-code/webui— the path the test was mocking — has nodaemon-react-sdkexport at all.
So vi.mock('@qwen-code/webui/daemon-react-sdk', ...) was registering a factory against a specifier that never resolves in this test, and main.tsx was pulling in the real provider. The one-character-per-word fix is correct and the mock now intercepts what the module under test actually loads.
The thing worth flagging is the failure mode rather than this diff: a vi.mock whose specifier matches nothing is silently a no-op, so this class of drift produces a test that still passes while quietly exercising unmocked code. If the provider ever grows a side effect at import time, that would surface as a confusing failure far from the cause. Not something this PR needs to solve, but if the same pattern exists in sibling boot tests it is worth a sweep.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped before round 1 by the review time budget.
Test Plan (not a blocker): 2 passed — this review observed 5102, 481 passed.
中文说明
未审查:反向审计——评审时间预算不足,未能开始第 1 轮。
Test Plan(非阻断):2 passed — this review observed 5102, 481 passed。
— qwen3.8-max via Qwen Code /review (v0.22.3)
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
What I checked:
- Diff (1 line):
vi.mock('@qwen-code/webui/daemon-react-sdk', …)→vi.mock('@qwen-code/web-shell/daemon-react-sdk', …) - Cross-file:
packages/web-shell/client/main.tsximportsDaemonWorkspaceProviderfrom@qwen-code/web-shell/daemon-react-sdk(post-#9811 cutover) — the old mock path was targeting a specifier that package@qwen-code/webuino longer exports; the new path matches the real import. packages/web-shell/package.jsonexports./daemon-react-sdk— the mock target is resolvable.- CI: Integration Tests ✓, Desktop Shell ubuntu+windows ✓, Capture web-shell visuals ✓.
Test (ubuntu-latest, Node 22.x)was CANCELLED (runner issue, not a test failure);Post Coverage Commentfailed downstream of the cancellation. These are pre-existing infra issues unrelated to this change. - Existing reviews: CanReader independently confirmed the diagnosis; yiliang114 approved.
Reviewed with AI assistance.
What this PR does
main-boot.test.tsxmocks@qwen-code/webui/daemon-react-sdk, butmain.tsxhas importedDaemonWorkspaceProviderfrom@qwen-code/web-shell/daemon-react-sdksince the WebShell UI cutover (#9811), and thewebuipackage no longer exports that path. Vitest resolves avi.mockspecifier even when nothing imports it, so the suite fails at collection withMissing "./daemon-react-sdk" specifier in "@qwen-code/webui" packageand both boot tests are red onmain. This points the mock at the module the boot actually loads.Why it's needed
Every PR's
Test (ubuntu)job runs the web-shell unit suite; with this file failing at collection onmain, the job cannot go green for anyone once the self-hosted pool gets through it.Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/main-boot.test.tsx— 2 failed onmain(the missing-specifier error above), 2 passed with this change. Nothing else in the file changes; the mock's shape is unchanged.Evidence (Before & After)
N/A (test-only; the vitest output above).
Tested on
Environment (optional)
N/A
Risk & Scope
Linked Issues
Follow-up to #9811 and #10215 (the two commits whose combination left the mock pointing at a removed export).
中文说明
本 PR 的改动
main-boot.test.tsxmock 的是@qwen-code/webui/daemon-react-sdk,但自 WebShell UI 切换(#9811)起main.tsx已从@qwen-code/web-shell/daemon-react-sdk导入DaemonWorkspaceProvider,且webui包不再导出该路径。Vitest 即使没有模块导入也会解析vi.mock的说明符,因此该套件在收集阶段就报Missing "./daemon-react-sdk" specifier in "@qwen-code/webui" package,main上两个启动用例全红。本 PR 把 mock 指向启动时真正加载的模块。为什么需要
每个 PR 的
Test (ubuntu)job 都跑 web-shell 单测;这个文件在main上收集即失败,一旦 self-hosted 池跑到它,任何 PR 的该 job 都无法变绿。审查测试计划
如何验证
cd packages/web-shell && npx vitest run client/main-boot.test.tsx——main上 2 失败(上述缺失说明符错误),本改动后 2 通过。文件中其它内容不变,mock 的形状不变。证据(改动前后)
N/A(仅测试;见上述 vitest 输出)。
测试平台
环境(可选)
N/A
风险与范围
关联 Issue
#9811 与 #10215 的后续(两者叠加后 mock 指向了已移除的导出)。