Repository navigation
test(console): consolidate the console browser tests added since #2903 - #2947
Conversation
…le browser tests (wip)
…wn scale and add the missing ones
…ep the repositories note by its check
|
🦞👀 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. |
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. What this changesThis PR shares console browser fixtures and assertion helpers, batches long-name measurements, and refreshes CI scheduling estimates. Example: Run the console status-block browser suite.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep open for normal maintainer handling: this PR provides useful test consolidation, and the reviewed diff has no actionable correctness findings. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherConsole browser suites exercise served UI and controller routes through an in-memory application fixture; shared helpers collect observations, and duration estimates order CI test files. flowchart LR
A[CI duration estimates] --> B[Test runner]
B --> C[Browser test cases]
D[Shared browser helpers] --> C
C --> E[Console application fixture]
E --> F[Rendered console]
F --> G[Behavior assertions]
Technical reviewBest possible solution: Land the focused consolidation through the ordinary protected merge process once required CI passes. Do we have a high-confidence way to reproduce the issue? Source inspection confirms repeated fixture setup and duplicated observation helpers; this PR does not report a new product bug. Is this the best way to solve the issue? The change reuses established browser and fixture owners, preserves the changed scenarios' behavioral assertions, and keeps scheduling changes confined to measured estimates. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against def181770d66. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel 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 Reviewed October 19, 2026, 11:29 AM ET / 15:29 UTC. |
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
Free's clanker here. One small coverage loss in the batched Agents-list measurement; keep the batch, but select links rather than all <a> elements.
I ran the exact measurement and assertions from c4fa5e07 in sandboxed Chromium with two synthetic rows. With both links intact, the PR and a role-based batch passed. Removing only the first link’s href (same name, class, and geometry; last link still valid), the old exact link-role selector found zero matches, but the PR’s full measurement still passed. A batch using table.getByRole("link").evaluateAll(links => …) correctly failed “is listed once.” This tests what the regression would catch; I’m not claiming the product currently emits a broken link.
The small change is to measure links in that one call and use link.closest("table").after(box) while measuring. It preserves the link-role coverage without restoring one browser round trip per name. Separately, our source comparison found all 187 prior named test declarations/templates retained in their files and the new grouping parents; that’s a discovery check, not a claim we reran the whole matrix. No CI or bot rerun requested.
2 participants
Problem
Since #2903 about 25 PRs added console browser tests. Most new cases booted their own console fixture (about 0.3 s each before the page loads), several files carried their own copy of the same helper (a status line's class/tone/role, the browser
invalidevent counter, hand-written request holds), and the long-name test looked up each of ~90 names by accessible name, twice. console-agents-list-return also launched a new Chromium for every page. The duration rows for the grown files were stale, and console-form-fields-2 and console-agent-name-runs had none (flake-sweep-1019b).Change
Test-only. No assertion is removed or loosened; every former test is still a named test or subtest.
t.testsubtest with its own Agent (and its own Namespace where it had one) and its own browser context. The binding-removal case gets a Namespace of its own, because it really shares and leaves a Role that a later share would reuse. Hand-written holds use the sharedholdRequests.fileBrowser, as the other console files do). The long-name test measures every listed name in oneevaluateper width and asserts each case as before, plus that each name is listed exactly once.console-agents-browser-helpers.mjs):statusTone(locator, { text }),hasFocus(locator)andcountInvalidEvents(page). They replace threestatusTonecopies, five inline copies and twocountInvalidEventscopies. console-namespace-not-ready's warning check now compares the exact class (wasclassList.contains).Verification
Mutants, before (main def1817) and after (this head). 120 mutants listed by the lanes that added these tests since #2903 (from their PR bodies and lane logs; "derived" rows rebuild a PR's "fails on main" claim as a source mutant), plus 10 of my own aimed at the cases this PR regroups. Each mutant was run against the test files that should kill it, at 2 CPUs. The console-status-block mutants were run again on the final head after the removal-Namespace fix.
The kill sets are identical, mutant by mutant. The same two survive on both sides:
detail_stop_notice_warning(the Stop notice's tone,console-agent-detail, which this PR changes only by importingstatusTone) andns_old_form_keeps_polling(a derived #2937 row inconsole-namespace-not-ready). Neither is a regression here; both are recorded as board findings. One mutant a lane reported as surviving (ln_a4_lookbehind_dropped, #2935) is killed on both sides.Pins at 2 CPUs (taskset), 20 runs each on this branch:
Wall time, local, 2 CPUs. Five paired runs, main and this branch side by side at the same load:
The lanes run 8 files at a time, so the per-file savings show up as about 1% of lane wall time, within noise. Both checks-browser runs fail only
docs-compute-matrixlocally (the docs preview cannot loadgithub-sluggerin a worktree), on main as on the branch.Duration rows. Runners have been slower since about 08:45Z (15 to 90% per run). Each main run's time for a file is divided by its lane's slowdown factor (median over the lane's files that no PR changed since #2903 of time / row; e.g. 1.86 for checks-browser-2 on def1817), and the row is the median over the main runs since the file last changed. Files this PR changes are then scaled by the local ratio above.
The drafts-kept growth flake-sweep-1019b saw (34 to 42 s) was the runner slowdown, not the file.
Lint and format: eslint (
--max-warnings 0) andprettier --checkon every touched file pass;node scripts/ci/run-tests.mjs auditreports no issues. No new test file, so no suite mapping changes.Open PRs: a 3-way
git merge-treeof this head with every open PR touching these files (#2922, #906, #2518, #2596, #2258, #878, #115) gives the same conflicted files and hunks as the same merge with main: no new conflict hunk. #2922's new form-field tests usecountInvalidEvents,errorPlacement,assertFieldErrorandassertShownError, which keep their names here.Internal review: no assertion weakened. Fixed: a prettier line in console-status-block, the removal case's shared-Namespace order dependence, and a comment left without its code in console-agent-repositories.
Risks