Repository navigation
test(console): name the focused element when the mobile drawer check fails - #2938
Conversation
…fails Finding 2016: console.test.mjs:846 failed once as 'false !== true', so the element that held focus was unknown. The opened-drawer check now waits up to 1 s for focus to reach the Agents link (or Settings item), logs a diagnostic when it settles after starting elsewhere, and names the focused element, the first sample and the drawer state on failure. The post-release check keeps its strict single sample and names the focused element too.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesThis PR adds focused-element descriptions and bounded initial focus polling to two existing mobile-drawer browser tests. Example: The mobile drawer opens but focus remains on the page body.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it provides useful browser-test diagnostics, preserves the strict post-render focus check, and introduces no identified defect. Priority: P3 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe browser suite opens the controller-backed Console with fixture storage and Drivers, holds a Namespaces response, and checks drawer state and keyboard focus around the resulting render. flowchart LR
A[Browser test] --> B[Controller fixture]
B --> C[Console mobile drawer]
C --> D[Focus samples]
D --> E[Assertion diagnostics]
Technical reviewBest possible solution: Retain the focused diagnostics and bounded initial wait while preserving strict focus verification after the held response completes. Do we have a high-confidence way to reproduce the issue? The original intermittent focus failure has no reliable reproduction: the author reports unsuccessful constrained-CPU attempts, and the available job metadata establishes only that the cited browser job failed. Is this the best way to solve the issue? Extending the existing behavioral checks is proportionate, and the drawer contributor explicitly accepts bounded initial settling while preserving the consequential post-render assertion. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 40676a7fed34. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 19, 2026, 9:45 AM ET / 13:45 UTC (Revision 2). |
…drawer state only on failure Review nits on finding 2016's helper: an SVG element's className is an object, and the drawer's aria-expanded was read even when the check passed.
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Problem
Finding 2016: Console Browser went red once on #2255 (run 39250245319, job 118052053777) at
tests/browser/console.test.mjs:846, the unlabelled check that the reopened mobile drawer's Agents link holds focus right after the toggle click. It failed asfalse !== true, so nothing says what held focus instead. The test failed once in 1,734 runs, during the 10-19 runner slowdown.Change
Test-only. A
focusSettles(locator, settleMs)helper samplesdocument.activeElementand describes it (tag, role, class, accessible text, detached or not).aria-expanded.openDrawerfocuses the first nav link synchronously, so the 1 s wait only covers a click that lands while the shell is being replaced. A real focus move would still fail, and the message now says where focus went.Verification
openDrawerwith no focus call fails withThe opened drawer focuses its navigation link <a> "◇Agents"; focus is on <body> (first check: <button class="mobile-toggle"> "Open navigation"; drawer true).console.test.mjs48/48. Repeated on the final head after the review nits: the class is read withgetAttribute(an SVG element'sclassNameis an object), and the drawer state is read only on failure.Risks
Low. Test-only. The opened-drawer check now tolerates up to 1 s of focus elsewhere before failing, and logs it when that happens. The post-release "keeps focus" check is unchanged.