Repository navigation
test: avoid repeated RPC catalog loads for layout schemas - #29663
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: reducing repeated RPC catalog loads in layout schema tests. |
| Description check | |
The description covers the required explanation, changed files, rationale, testing, visual proof, scope, compatibility considerations, and checklist. It also records the remaining full-workflow and ho… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| 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. |
Full details: Description check
-
Explanation
The description covers the required explanation, changed files, rationale, testing, visual proof, scope, compatibility considerations, and checklist. It also records the remaining full-workflow and hosted-measurement qualification before merge.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
✨ 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.
Comment @coderabbitai help to get the list of available commands.
ELI5
Layout command tests reload every RPC method for each group of parser inputs. They now check build registration twice and validate the other commands through the same production layout method directly.
What Changed
Use the existing
LAYOUT_APPLY_METHODSexport for eleven command-schema cases. Keep both full-catalog registration cases, all 19 cases and assertions, and all six real runtime-routing cases, including SSH and paired-server ownership. No production changes.Why
The catalog includes this method without changing its schema or handler. Repeating catalog startup adds cost without distinct proof. Keeping separate false/true build cases preserves registration coverage; direct owner lookups avoid caching the catalog or sharing mutable runtime fixtures.
Linked Issue
Internal CI/test audit; no new issue. Existing layout command work began in #29185.
Visual Proof
N/A — test setup only; no visual or interaction change.
Testing
Three alternating local pairs with Node24.21.0/Bun1.4.3, one worker and background launch preserve all 19 case identities with zero unhandled errors. Median wall 18.844 → 12.224s, module 17.200 → 10.668s, test execution 9.418 → 2.793s. Collection remains similar (7.741 → 7.826s). Hosted source-qualified measurement: selected suite module 13.020 → 4.000s, test execution 10.849 → 1.769s, collection 2.159 → 2.221s. Both retain all 19 cases. This is selected-suite evidence from one comparison, not a whole-CI elapsed claim.
All 51 cases across five owner/sibling suites pass with shuffle seed104729, including real runtime partition routing, stream registration, renderer delivery and editor workspace movement. Node typecheck, changed-code quality, scoped lint/format and whitespace checks pass.
Four deliberate production defects fail at the intended guards: registering in a window-owned build, omitting main-owned registration, allowing caller close semantics through the parser, and routing an SSH split to local. Production restores byte-for-byte. Reversing the eleven lookups and import reconstructs the entire original test exactly.
AI Disclosure
Review
Independent full-lane qualification approved this boundary split, including the editor workspace-move case added on current main. Independent final preservation and hosted source/inventory reviews passed. Full PR workflow 38900629919 and its aggregate 116975326668 passed for exact head 4b62ab5; all 30 separate/current checks succeeded or intentionally skipped. All five inventories reconcile: 12,358 → 12,362 files, four unrelated source-qualified additions, no removals, errors or omissions.
Agent skill upstream boundary
Notes
Production 0 LOC; one test +34/-11 after formatting. No platform, wire, provider, SSH execution or folder-workspace behavior changes. Real runtime partition tests and both build-switch catalog tests remain unchanged.
Checklist