Skip to content

test(console): fold three single-case browser files into their homes - #2736

Merged
freeqaz-openai merged 1 commit into
mainfrom
test-consolidate-45
Oct 17, 2026
Merged

freeqaz-openai merged 1 commit into
mainfrom
test-consolidate-45

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Three browser test files added on 10-16/17 each hold one or two cases on a surface that an existing file already covers. Each costs its own Chromium launch, its own Console Browser 2 slot and its own suite entry.

Change

Each moved test is copied verbatim and keeps its own fixture and setup (Sandbox Driver, default Presets, routed Installation answers, viewport). No test is renamed.

Left alone, because their home files are being edited elsewhere:

Verification

  • Test names: the before set (3 removed files plus the 2 home files on main) and the after set (2 home files) are identical: 80 names each, including subtests, with no duplicates.
  • Mutants, before and after: every mutant the moved tests killed is still killed in the new file, and the same moved tests fail.
    • Create Agent mode: C19, C20, C21, C22, C23, C31.
    • Pod quota line: Q1 (lost access), Q4 (403).
    • Deploy gate order: S3.
  • node scripts/ci/run-tests.mjs audit: no issues. eslint and prettier are clean on the touched files.
  • 20/20 at 2 CPUs on CPUs 10-13: console-create-agent-provisioning-400 passed 20/20 (36 tests each, 38-40 s per run) and console-agent-saves passed 20/20 (44 tests each, 44-49 s per run).
  • Console Browser 2 lane time: the test window was 79 s on this PR's run (5b55cf1) against 82 s on main ac6a281, and 78-80 s on the four main runs before test: pin conditions from the 10-17 morning fixes #2732. That is the same within noise. The lane still runs 323 tests, now in 16 files instead of 19, and the summed file time fell from 496 s to 489 s. console-create-agent-provisioning-400 went from 29 s to 36 s and ends at 76 s, before console-agents-2 (79 s), which still sets the lane.

Risks

Test-only. In Console Browser 2, console-create-agent-provisioning-400 grew by 6 s in CI and still ends before console-agents-2. An internal review approved the change, with no changes requested. Its file-durations.json row (12.2 s) was already stale before this change, and it is not refreshed here.

Create Agent mode defaults and the Pod quota denied read move into
console-create-agent-provisioning-400; the missing-binding Deploy gate
moves into console-agent-saves. Each case keeps its own fixture and
setup. checks-browser-2 drops the three removed files.
@freeqaz
freeqaz requested a review from a team as a code owner October 17, 2026 10:58
@clawsweeper

clawsweeper Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 17, 2026 10:58 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 17, 2026 10:58 — with GitHub Actions Inactive

@freeqaz-openai freeqaz-openai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Free's clanker here. I checked the actual base Console Browser 2 results (artifact 12098193551) and this PR's Console Browser 2 artifact (ID 12098143997). Both ran 323 tests with no failures or skips. All 80 cases in the five affected files match across the move; that comparison also rejects a changed quota assertion or a missing test replaced by a duplicate. The three moved test bodies are unchanged.

The concrete saving is 19 file runs down to 16, removing three normal Chromium launches. Tests keep separate browser contexts and app fixtures, but no longer get the three extra browser-process resets. I wouldn't claim a wall-clock speedup from this pair of runs. The comparison is for GitHub's tested merge 77396d46, not a claim that every later main revision was tested.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 17, 2026
@clawsweeper

clawsweeper Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR moves execution-mode, quota-access, and Secret-gate tests into two existing browser test files and removes their former suite entries.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, useful consolidation with preserved regression assertions, independent contexts, and passing relevant CI.
Proof confidence 🌊 off-meta tidepool Not applicable: This maintainer-authored test consolidation changes no shipped behavior; relevant browser CI passes, and the supplied role-sharing screenshots concern a separate feature.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors pay separate browser-process and CI scheduling overhead for small files covering existing Console surfaces.
Reason: The maintainer-authored change removes redundant setup while preserving distinct regression cases; elapsed-time improvement is not established or required to justify this small cleanup.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it preserves the regression cases while removing three separate browser-file runs, and I found no introduced defect.

Priority: P3
Reviewed head: 5b55cf1a5cad8b026fc795234922fbf8e0e1d75d

Before merge

None.

Findings

None.

Agent review details

How this fits together

The Console Browser 2 manifest selects browser test files; each file shares a Chromium process while its tests retain separate browser contexts and application fixtures.

flowchart LR
  A[Console Browser 2 manifest] --> B[CI test runner]
  B --> C[Two destination test files]
  C --> D[Shared Chromium per file]
  D --> E[Separate browser contexts]
  E --> F[Console and application fixtures]
Loading

Technical review

Best possible solution:

Land the focused consolidation with its existing regression coverage and browser-context isolation preserved.

Do we have a high-confidence way to reproduce the issue?

No product bug is claimed; source inspection confirms the separate-file overhead addressed by this PR.

Is this the best way to solve the issue?

The move reuses the existing file-level browser owner, preserves each test's setup and assertions, and removes the replaced registrations without adding another abstraction.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3d33ba8b51a0.

Provenance checked

  • Execution-mode regression tests keeps the original intent (55659a6: Pin user-selected and credential-required execution modes while avoiding files under concurrent modification.)
  • Quota-access and missing-binding gate regressions keeps the original intent (ac6a281: Pin denied deployment reads and missing-binding precedence after mutation testing exposed uncovered conditions.)
  • File-level browser reuse keeps the original intent (3d9ad33: Reduce Chromium launch overhead while retaining independent contexts and fixtures; older follow-history traversal reached unavailable creation-adjacent blobs, without affecting inspection of this reuse contract.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Review metrics

Metric Value Why it matters
Browser file runs 19 → 16 The manifest removes three files while retaining their cases in existing files, eliminating three normal Chromium launches.
Change footprint Production +0/-0; tests +307/-360; manifest +0/-12 The patch removes duplicated file setup without changing product behavior.

Labels

Label changes:

  • add P3: This is a narrow test-organization cleanup with no production behavior change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is a narrow test-organization cleanup with no production behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 17, 2026, 7:11 AM ET / 11:11 UTC.

@freeqaz-openai freeqaz-openai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Free’s clanker here. Approved at 5b55cf1a. I compared the moved test bodies with current main: the assertions are preserved, the three old suite entries are gone, and the separately maintained focus test stays in place. The before-and-after browser results show the same 323 passing tests. The measurable improvement is three fewer Chromium launches, not a promised wall-clock speedup.

CI Required and Console Browser 2 passed. I read ClawSweeper’s complete review of this head; its “Before merge” and “Findings” sections are both empty.

@freeqaz-openai
freeqaz-openai merged commit 106dc57 into main Oct 17, 2026
48 of 49 checks passed
@freeqaz-openai
freeqaz-openai deleted the test-consolidate-45 branch October 17, 2026 11:13
freeqaz added a commit that referenced this pull request Oct 17, 2026
Review follow-up: the oldest of the five runs is #2736's own merge commit.
freeqaz-openai pushed a commit that referenced this pull request Oct 17, 2026
* test(ci): refresh file durations

Recompute every row from the five green main runs after #2736 (c085982 and
the four before it). provisioning-400 grew from 12.2 s to 35.3 s once #2736
folded cases into it, ci-openshell from 0.1 s to 7.3 s, and the shared lanes
run faster overall, so 145 of 209 rows moved. Add rows for the three shared
browser files that had none (agents-list-return, agent-deploy-focus,
refocus-focus). credential-source-api gets no row: it is not in
checks-baseline-1's parallelFiles, so it runs alone and its row would not be
read.

* test(ci): name the durations window from #2736 on

Review follow-up: the oldest of the five runs is #2736's own merge commit.

This branch had an error being deployed

1 failed deployment
integration-qa-pr — 5b55cf1a Deployed Oct 17, 2026 by freeqaz via QA advisory (OpenShell) #1865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants