test(web-shell): capture the git-mode new-branch sub-state in the visuals suite - #7672
Conversation
…uals suite The `git mode selector` visuals scenario captured the composer chip and the opened three-mode popover, but stopped there: selecting an option used to dismiss the popover ~100ms later (the click bubbled through the React tree out of the portaled content to the composer surface's onClick → core.focus() → Radix focus-outside close), so the branch-name sub-state couldn't be shot stably. #7668 fixed that dismissal, so it now can. Extend the scenario to click "New branch", fill a valid branch name, and capture the revealed input (validated) with its Create-branch affordance, in both themes. All six git-mode captures are byte-stable across runs (0% pixel diff). Match the option by role — its label is split across a name and a description span, so getByText('New branch') is ambiguous (also fixed in #7668). Beyond covering a state the preview never showed, this doubles as a visual regression guard for #7668: if the popover ever dismisses on option-click again, the input goes missing and the assertion fails here, not only in the screenshot.
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
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.
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
Thanks for the PR! Template looks good ✓ Problem: this fills a documented coverage gap — the new-branch sub-state was deliberately excluded from the visuals scenario in #7471 because the popover dismissed before it could be captured stably. #7668 (already on main) fixed that dismissal, making the sub-state capturable. The gap is real and clearly established by the prior PR's own comments. Direction: aligned — extending an existing visuals scenario to cover a state that was previously impossible to shoot is straightforward test maintenance. No product-direction concerns. Size: not applicable (single test file, no core paths). Approach: the scope is exactly right — 15 lines of test code appended to the existing scenario, using role-based selectors and data-testid locators consistent with the suite. Nothing to cut. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:填补了一个有记录的覆盖缺口——新建分支子态在 #7471 中被刻意排除,因为 popover 在截图前就会关闭。#7668(已在 main)修复了该关闭行为,使子态可被稳定捕获。缺口真实存在, prior PR 的注释已明确说明。 方向:对齐——扩展现有视觉场景以覆盖此前无法拍摄的状态,属于常规测试维护。无产品方向顾虑。 规模:不适用(单个测试文件,未触及核心路径)。 方案:范围恰当——在现有场景后追加 15 行测试代码,使用 role 选择器和 data-testid 定位符,与套件风格一致。无需裁剪。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: after the existing popover capture, click the "New branch" radio option, assert the branch input appears, fill a valid name, assert the confirm button is visible, then capture. That's exactly what this PR does. The implementation is clean:
No critical blockers. No convention violations. CI Test EvidenceAll checks on
The "Capture web-shell visuals" check is the exact suite this PR modifies — green. macOS/Windows test jobs are skipped per CI config for this change. Not verified: real-scenario tmux testing (CI run — not applicable for a test-only change with no TUI surface). 中文说明代码审查独立方案:在现有 popover 截图后,点击 "New branch" 单选项,断言分支输入框出现,填入合法名称,断言确认按钮可见,然后截图。PR 的实现与此完全一致。 实现干净:role 选择器避免了文本歧义;data-testid 定位符与套件一致;移除的旧注释被正确替换为解释当前捕获原因的新注释,兼作回归守卫的文档。无无关改动,无范围蔓延。 无关键阻塞项。无规范违反。 CI 测试证据
未验证:真实场景 tmux 测试(CI 运行——对纯测试变更不适用)。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, minimal test addition that fills a documented gap; would merge without hesitation. This is exactly what good test maintenance looks like: a sub-state that was previously impossible to capture (and the prior PR said so in a comment) becomes capturable after a fix lands, and this PR immediately closes that gap with the minimum code needed. The capture doubles as a regression guard for #7668's fix — if the popover dismissal ever returns, this assertion fails loudly rather than relying on eyeballed screenshots. The diff is 15 lines of test logic, no production code touched, selectors are idiomatic for the suite, and CI's own visuals capture job confirms it works. Nothing to maintain, nothing to curse in six months. 中文说明置信度:5/5 —— 干净、最小的测试新增,填补了有记录的覆盖缺口;毫不犹豫地合并。 这正是良好的测试维护:一个此前无法捕获的子态(prior PR 的注释已说明),在修复落地后变得可捕获,本 PR 立即以最小代码关闭了该缺口。捕获兼作 #7668 修复的回归守卫——如果 popover 关闭行为复现,断言会在此处响亮地失败,而非依赖人眼看截图。 diff 为 15 行测试逻辑,未触及生产代码,选择器符合套件惯例,CI 的视觉捕获任务确认其有效。无需维护,六个月后也不会骂人。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
There was a problem hiding this comment.
The visual coverage is useful and the main flow looks right. I left one test-strengthening comment and one non-blocking cleanup suggestion inline.
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Code review —
|
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed[rc:3647253861] P2 — Assert the validated state → ImplementedThe confirm-branch button is visible as soon as branch mode opens; only its disabled→enabled transition reflects validation. Changed [rc:3647253880] P3 — Trim the implementation history → ImplementedReplaced the 8-line comment (which repeated #7668's propagation history) with two concise lines: one for the "why this sub-state is capturable" and one for the non-obvious role-matching rationale (label spans multiple elements, so Verification
中文说明已处理的评审反馈[rc:3647253861] P2 — 断言验证后的状态 → 已实现确认分支按钮在进入分支模式时就已可见,只有从 disabled 到 enabled 的转变才能反映验证结果。将 [rc:3647253880] P3 — 精简实现历史注释 → 已实现将 8 行注释(重复了 #7668 的事件传播历史)替换为两行简洁注释:一行说明"为什么这个子状态可以被捕获",另一行说明非显而易见的 role 匹配原因(标签跨多个元素, 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review
Scope the New branch option to the popover locator and settle past the ~100ms dismissal window before re-asserting the popover and input stay visible, so a regression of #7668 hard-fails here instead of only producing a wrong (visually reviewed) screenshot. Mirrors the proven guard in web-shell.git-mode.spec.ts.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedThanks for the A/B run — both points are valid and both are now implemented. The change stays test-only and scoped to the
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM!✅
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
Released in v0.21.1. |
|
PR #7672 ( ✅ completed |


What this PR does
Extends the
git mode selectorvisuals scenario to capture the new-branch sub-state of the git-mode popover — the branch-name input (validated) plus the Create-branch affordance — in both themes. The scenario already captured the composer chip and the opened three-mode popover; this adds the state a reviewer actually looks at when judging the branch-creation UI.Why it's needed
Selecting an option in the popover used to dismiss it ~100ms later: the click bubbled through the React component tree (synthetic events don't follow the DOM tree, so the portaled popover content still reached the composer surface's
onClick) →core.focus()→ Radix focus-outside close. So the branch-name sub-state flashed and vanished, and couldn't be shot stably through the capture pipeline — it was deliberately left out when the scenario landed (#7471).#7668 fixed that dismissal (
onClick={(e) => e.stopPropagation()}onPopoverContent), so the sub-state is now stable and capturable. This PR captures it. Neatly, the state that demonstrates #7668's fix is exactly the one #7668 made shootable — so beyond filling the coverage gap, this capture doubles as a visual regression guard: if the popover ever dismisses on option-click again, the branch input goes missing and the assertion fails here, not only in the (eyeballed) screenshot.The option is matched by role (
getByRole('radio', { name: /New branch/ })) rather than text — its label is split across a name and a description span, sogetByText('New branch')is ambiguous (also fixed in #7668).Reviewer Test Plan
How to verify
Determinism (the point — proving the once-flaky branch state is now stable): render twice and pixel-diff. All six captures are
0.0000%(|ΔR|+|ΔG|+|ΔB| > 30per pixel, over the full frame). Full visuals suite stays green (26 passed), and prettier/eslint/scoped-tsc are clean.To confirm the capture is load-bearing on #7668's fix, revert only the
stopPropagationline inGitModePopover.tsxand re-run this scenario: the popover dismisses after the New-branch click,expect(branchInput).toBeVisible()fails, and thegit-mode-branch-*capture never happens.Evidence (Before & After)
The new capture — the popover stays open, "New branch" is selected (✓), the branch input holds a validated
feat/my-feature, and the$ git checkout -b …preview + Create branch button are shown:git-mode-branch-dark.png/git-mode-branch-light.png(head-only NEW in the preview bot).Before this PR: no capture of this state at all — the scenario stopped at the opened popover.
Tested on
Environment (optional)
Playwright against the visuals mock daemon (Vite dev server, Chromium). No real backend. Determinism measured with a canvas pixel-diff matching the visuals bot's own metric.
Risk & Scope
main(it is): without itsstopPropagationfix this capture would be flaky. That's intentional — the capture guards exactly that fix.web-shell.git-mode.spec.ts).Linked Issues
Follow-up to #7471 (added the scenario) and #7668 (fixed the dismissal that made this sub-state capturable).
中文说明
这个 PR 做了什么
扩展
git mode selector视觉场景,补拍 git-mode popover 的新建分支子态——branch 输入框(已校验)+ Create-branch 按钮,双主题。场景原本只拍了 composer chip 和打开态的三模式 popover;这一步补上了 reviewer 真正会看的那个"创建分支 UI"的状态。为什么需要
在 popover 里点选项以前会在 ~100ms 后把它关掉:点击沿 React 组件树冒泡(合成事件不走 DOM 树,所以 portal 出去的 popover 内容仍冒泡到 composer 表面的
onClick)→core.focus()→ 触发 Radix 的 focus-outside 关闭。于是 branch 输入子态一闪即逝,在截图管线里拍不稳,场景落地时(#7471)就故意没拍它。#7668 修好了这个关闭(给
PopoverContent加onClick={(e) => e.stopPropagation()}),这个子态现在稳定可拍,本 PR 就把它拍下来。巧妙之处:能展示 #7668 修复的那个状态,恰恰是 #7668 让它可拍的——所以除了补齐覆盖缺口,这个捕获还兼作视觉回归守卫:以后谁再让 popover 在点选项时关闭,branch 输入就会消失、断言在这里就挂,而不是只体现在(需人眼看的)截图里。选项用 role 定位(
getByRole('radio', { name: /New branch/ }))而非文字——它的 label 分在 name span 和 description span 两处,getByText('New branch')有歧义(这点 #7668 也修了)。如何验证
见上文英文命令。确定性(重点——证明曾经不稳的 branch 态现在稳了):双跑像素比对,6 张全部
0.0000%。全套件保持26 passed,prettier/eslint/tsc 干净。想确认这个捕获对 #7668 的修复是承重的:只回退GitModePopover.tsx里那行stopPropagation再跑本场景,popover 在点 New branch 后关闭,expect(branchInput).toBeVisible()失败,git-mode-branch-*根本拍不出来。风险与范围
main(已在):没有它的stopPropagation修复,这个捕获会 flaky。这是有意的——捕获守的正是那个修复。web-shell.git-mode.spec.ts覆盖)。