Repository navigation
test: share saved terminal title runtime setup - #29314
Conversation
…audit-runtime-construction
…ntime-construction
…test-audit-runtime-title-startup
…ntime-title-startup
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the main change: sharing runtime setup for saved terminal title tests. Description check The description is complete and specific. It explains the moved tests, rationale, preserved coverage, assertion repair, testing results, and validation status. The linked issue is referenced as #29309…Full details: Description check
-
Explanation
The description is complete and specific. It explains the moved tests, rationale, preserved coverage, assertion repair, testing results, and validation status. The linked issue is referenced as
#29309but is not formatted as a direct issue link.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 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.
Comment @coderabbitai help to get the list of available commands.
ELI5
Three terminal-title regressions start the whole runtime separately. Keep them with the existing terminal-creation tests so that setup runs once.
What Changed
Move the three saved-title cases into the already imported terminal-creation spec and delete their standalone test entry. Keep named creation persistence, the background-terminal title exception, and saved-title fallback after live title loss. Require the background tab to exist before checking its missing custom name, closing an assertion that previously passed if the tab disappeared entirely.
Why
All three are useful runtime contracts. Sharing the established runtime setup removes repeated imports without dropping coverage or adding a harness. The existing spec owns terminal creation and already uses the same lifecycle and session fixture. Production code is unchanged.
Linked Issue
Maintainer-requested CI test audit. Stacked after #29309; retarget main and rerun required checks once its base lands.
Visual Proof
N/A — test organization and assertion repair; product behavior unchanged.
Testing
Linux, Bun 1.4.3 / Node 24.21.0, two workers: all six alternating before/after runs pass 1,365 tests / one skipped. All three title cases remain. Independent preservation review passed.
Each comparison runs the existing runtime aggregator plus the original standalone title suite (baseline), or the aggregator containing the moved cases (candidate). Command: CI Bun
--bun node_modules/vitest/vitest.mjs run --config config/vitest.config.ts --maxWorkers=2with default and timing reporters,ORCA_BACKGROUND_LAUNCH=1, and CI Node executable supplied to the pool. This measures removed duplicate imports; hosted results are recorded separately below.Hosted before/after: parent #29309 run 38829650565 versus this PR run 38830208115. Selected module worker time totaled 40.612 → 37.057s (8.8% lower): the retained runtime module rose 36.804 → 37.057s, while the separate 3.808s title module was eliminated. All five artifacts per variant passed with zero unhandled errors; measured files passed. Both used Node 24.21.0/Bun 1.4.3, five ARM shards and four workers per shard. Actual merge-checkout trees match the respective PR heads, with runner/routing sources unchanged. This historical, unpaired sum is separate from local elapsed time and does not establish a whole-workflow speedup. Main-based full required CI remains pending.
Three deliberate production mutations fail the intended persisted-title, background-title, and list-fallback assertions; a missing-background-row control fails the new existence assertion. Source restored byte for byte afterward. Node typecheck, changed-code-quality, formatting, and whitespace checks pass.
Review
No test declarations removed. The moved fixture executes the actual admission reducer and session persistence; it does not supply expected titles through mocks. The existing aggregator imports the target spec. Test entry/spec drops five lines; no production change.
Agent skill upstream boundary
Checklist
Fresh refresh head c90ad00 incorporates the current constructor parent c866091. Its exact two-file +66/-71 title delta matches the original candidate, and independent preservation review confirms all three contracts, the new existence guard and upstream aggregator imports. Three alternating Linux Node 24.21.0 / Bun 1.4.3 one-worker pairs all pass the same 1,385 cases plus one skip. Median command wall 28.751 → 21.529 seconds (25.1% lower); summed module 27.016 → 19.764 seconds. This is controlled local observation, not a hosted or whole-workflow speed claim. Current node typecheck, changed-code-quality against the actual parent, lint, format and whitespace checks all pass. Fresh stacked full run 38872648005 is pending; main-based qualification remains required after the parent lands. Prior measurements above remain historical.
Main reconciliation after constructor squash: final head6bbc1fa59bfe1c762eecefcc25d0751c8e31a9cf merges main50de2bc966. PR diff is now only the original two title paths +66/-71. Fresh complete runtime owner passes1,385 cases/one skip; node typecheck, actual-main changed-quality and whitespace pass. The moved case bodies remain identical, so the three controlled pair measurements above remain source-qualified local evidence for this unchanged title delta; no timing claim is made from the concurrent final validation. A new exact-head full gate and separate headless validation are required; previous stacked run is historical.
Final qualification at6bbc1fa59bfe1c762eecefcc25d0751c8e31a9cf: full
.github/workflows/pr.ymlrun38873657354 passed verify / PR checks passed116886660689. Separate Headless Node server38873657297 passed. All current checks are completed success/skipped. Five clean reports contain12,329 unique paths with zero unhandled errors. All1,385cases/one skip run in the retained aggregator, preserving the three moved title contracts; actual source and all24 checked blobs match the reviewed head. Hosted parent aggregator+standalone27,506ms→candidate24,650ms (10.38% lower selected module sum). Same Node24.19/Bun1.4.3 BlacksmithX64/five shards/four workers, but upstream source/workload and runner variance confound this unpaired observation; no causal or workflow elapsed-time claim. This supersedes pending status above.