Skip to content

test: share floating panel behavior suite startup - #29304

Merged
OrcaWin merged 8 commits into
mainfrom
OrcaWin/test-audit-floating-startup
Oct 16, 2026
Merged

OrcaWin merged 8 commits into
mainfrom
OrcaWin/test-audit-floating-startup

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​159 $\color{#cf222e}{\Huge{\mathbf{−}}}$​689 $\color{#cf222e}{\Huge{\mathbf{−}}}$​530
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​4 $\color{#cf222e}{\Huge{\mathbf{−}}}$​11 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7

ELI5

Floating-panel tests repeatedly load the same component and register the same mocks. Run their 73 cases together so the PR suite does that work once.

What Changed

Four behavior suites become imported specs behind one test entry point. The assertions stay with their existing behavior areas; 35 common mock declarations register once. Setup resets the markdown creator and gives each case a fresh focus IPC function, using the production cache's existing identity boundary. Remove the cache-reset function and re-export used only by tests.

Why

The panel behaves the same, with the same bounds, focus, shortcuts, and tab-lifecycle regressions protected. Sharing setup avoids four isolated import graphs without reducing assertions or adding a production testing hook. Test/support drops 531 lines; production drops six.

Linked Issue

Maintainer-requested CI test audit. #29298 has landed. This PR now includes current main and targets main.

Visual Proof

N/A — test setup consolidation and removal of an unused product export; no rendered behavior changes.

Testing

  • I manually tested these changes locally
  • Automated tests updated

Fresh verification after merging main 9c177fb: all 108 owner/sibling cases pass, including all 73 panel cases. Actual dynamic collection and all six measurement reports preserve the exact original 73 full names, once each. Shuffled panel suite passes all 73 (seed 104729). Web typecheck, all changed-code-quality checks, formatting and whitespace checks pass. No native-focus/visible-window tests were run.

Three alternating full-file pairs on this reconciled source, Linux, Bun 1.4.3 / Node 24.21.0, one worker, module cache off and background launch:

Metric Baseline median (range) Candidate median (range)
command wall time 3.843 s (3.734–4.099) 2.779 s (2.708–3.336)
summed module time 2.121 s (2.081–2.123) 1.260 s (1.223–1.285)

Every run passes the same 73 cases; baseline restores only this PR's paths from current main while the candidate uses the consolidated entry. This is a controlled local observation, not a whole-PR wall claim. Historical hosted selected module time was 4,944 → 981 ms (runs 38827086842 / 38828680027; verified actual sources), with all five reports clean and matching Node/Bun versions/four workers on ARM. That unpaired older comparison has runner/base/cache/workload confounders and does not validate the reconciled head. Fresh reconciled-head full PR run 38855578573 passed all five unit shards with zero unhandled errors across 12,216 executed files. The actual tested merge tree matches the reviewed head; its wrapper passed all 73 cases, with 568 ms official module cost versus 5,202 ms summed across the four original suites in run 38848025584. This historical, unpaired comparison changed from GitHub ARM / Node 24.21 to Blacksmith X64 / Node 24.19; base code, dependencies, workload, caches and scheduling also differ. It does not isolate this refactor or establish whole-PR savings. Full required CI remains pending before merge.

Review

All original cases retained. Shared browser-close mock preserves the lifecycle suite's behavior; other suites do not exercise a nonempty browser close. Per-case reset prevents shared mock implementation leakage. Independent preservation review passed: no missing contracts, changed mock behavior, or discovery gaps.

Agent skill upstream boundary

  • Not applicable; no upstream skill source copied.

Checklist

  • Small owner-focused change
  • Behavior, mechanism, and rationale explained
  • N/A visual proof explained
  • Self-reviewed; cross-platform and SSH behavior preserved
  • Independent preservation review passed
  • Main-based full required CI passed; hosted timing comparison recorded above

Refresh validation: head 1ed0341 includes current main 98457f6 and merged Kitty fixture repair #29519. The complete floating subtree, dependency lock, package and Vitest configuration are unchanged from independently approved bb2cde0. Eight affected owner/drag/layout/producer suites pass 127 cases; renderer typecheck, changed-code quality and whitespace checks pass. The prior dynamic collection/shuffle proof still covers identical floating/runner bytes.

Previous head bb2cde0 full PR run 38868611124 passed aggregate 116869988453 and all five clean unit shards; actual-source qualification confirms all 73 wrapper cases passed, with official module duration 797 ms. That run skipped real WSL, so it did not resolve the earlier image failure. Separate #29519 subsequently passed all 12 Windows/WSL/GPU image cases plus nine WSL participation cases, with retained screenshots reviewed, before merging. Fresh full CI and explicit Windows/WSL image validation at this refreshed head remain required. Prior timing comparisons stay historical; no new causal speed claim.

Final exact-head qualification: full PR workflow 38870300944 (.github/workflows/pr.yml) passed aggregate verify / PR checks passed 116875467802 for 1ed0341. All five clean unit reports contain 12,321 unique files exactly once; all 73 panel cases passed (424 ms official module time, 273 ms raw execution, separate metrics). The tested merge and reviewed head have identical trees. Separate exact-SHA Windows run 38870298789 passed 12 native/WSL/GPU image cases and nine WSL participation cases with zero skips or retries. All 36 screenshots were independently reviewed. This supersedes the pending status above; the earlier image failure was not waived. Current measurements verify execution and do not establish causal hosted or whole-PR savings.

@OrcaWin
OrcaWin changed the base branch from OrcaWin/test-audit-owner-coverage to main October 16, 2026 05:24
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: df4a9eb0-1d31-4be1-b27c-059bfda8bc2f
📥 Commits

Reviewing files that changed from the base of the PR and between 9c177fb and 9a4acb1.

📒 Files selected for processing: 8
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.behavior.test.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.bounds.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.focus.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.shortcuts.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tab-lifecycle.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx
  • src/renderer/src/components/floating-terminal/floating-terminal-focus-reporting.ts
  • src/renderer/src/components/floating-terminal/floating-terminal-panel-test-harness.ts
💤 Files with no reviewable changes: 5
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.shortcuts.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.focus.spec.tsx
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx
  • src/renderer/src/components/floating-terminal/floating-terminal-focus-reporting.ts
  • src/renderer/src/components/floating-terminal/FloatingTerminalPanel.bounds.spec.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The floating-terminal specs now use shared mocks declared in a behavior test setup instead of file-local mock declarations. The tab-lifecycle test checks the imported close function through vi.mocked. The test harness now resets mocks synchronously and no longer clears the reported-focus cache. The cache-reset helper and its panel re-export were removed.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 9a4ac

The consolidated panel tests remain covered by the normal test entrypoint, and no merge-blocking issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: sharing startup work for the floating-panel behavior test suite.
Description check Passed The description follows the required template and explains the change, rationale, testing, visual impact, review notes, and checklist status. It provides detailed validation results and clearly states…
Full details: Description check

Explanation

The description follows the required template and explains the change, rationale, testing, visual impact, review notes, and checklist status. It provides detailed validation results and clearly states the remaining limitations and CI evidence.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OrcaWin
OrcaWin merged commit 46779ca into main Oct 16, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant