Skip to content

test(layout): drop the stale switchedBuild warm-up left by two crossing merges - #33931

Closed
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/main-fix-layout-test
Closed

brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/main-fix-layout-test

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6
Prod 0 0 0 0

Main's static analysis and typecheck job fails on every PR because workspace-layout-paired-desktop.test.ts calls a function that no longer exists.

What broke. Two test-only changes to this file merged minutes apart and crossed. #33919 replaced the per-test switchedBuild() helper with static imports that load the runtime and RPC registry once. #33923, written against the older file, added a beforeAll that warms the runtime by calling switchedBuild(true). After both merged, that call refers to a deleted function, so typecheck fails.

Fix. Remove the stale beforeAll and its comment. #33919's static imports already pay the runtime graph's cold transform once, at module load and outside any test's timeout, which is what #33923's warm-up was for. Test-only; no shipped file changes.

Verified: the file's 25 tests pass locally through the named-test runner.

Author: @BrennanKB5

Plan: remove the obsolete hook while keeping the static runtime and RPC-registry imports, then verify the named tests and exact-head CI.

Light-path classification:

QA classification: only tests change, so the review skill's table gives qa=none; no running-app QA or screenshot is required.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: be83c250-5fb5-484d-96ab-2df9ec985f5d
Commits

Reviewing files that changed from the base of the PR and between 1187abd and 6b5bd4c.

Files selected for processing: 1
  • src/main/runtime/rpc/methods/workspace-layout-paired-desktop.test.ts

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

Walkthrough

The test file removes the beforeAll import and the suite-level hook that called switchedBuild(true) with a 120-second timeout.

Priority: Normal

Merge Risk: Minimal · up to 6b5bd

No actionable merge-blocking risk remains from this test-only cleanup.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning 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 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description clearly explains the failure, root cause, fix, scope, and testing. However, it does not follow the required template and omits or does not explicitly complete required sections, includ… Reformat the description using the repository template. Add an explicit issue link in the required Fixes # section, state N/A with a reason for Visual Proof, complete the Testing and checklist items, and address the remaining required s…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the removal of the stale switchedBuild warm-up from the layout test.
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.
Full details: Description check

Explanation

The description clearly explains the failure, root cause, fix, scope, and testing. However, it does not follow the required template and omits or does not explicitly complete required sections, including ELI5, What Changed, Why, a Fixes # issue link, Visual Proof with N/A, AI Disclosure, Review, Notes, the upstream boundary checkbox, and the checklist.

Resolution

Reformat the description using the repository template. Add an explicit issue link in the required Fixes # section, state N/A with a reason for Visual Proof, complete the Testing and checklist items, and address the remaining required sections or mark them not applicable.

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.

❤️ Share

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Main's static analysis and typecheck failed because two crossing test-only merges left a beforeAll calling the removed switchedBuild() helper. This PR deletes that stale hook and its unused import. No shipped behavior changes.

Light-path review: one independent adversarial reviewer checked the complete diff and recent history; no findings, fixes or deferrals. Static imports still load the runtime and RPC registry before test execution, outside each test's timeout, so the cold-import protection is retained. All 25 tests and their assertions remain.

Verified on 6b5bd4c265d50ea643467d633d4114954e743ed1: exact-head CI passed, including static analysis/typecheck, all five unit-test shards and the aggregate PR check. CI's first unit-test shard explicitly passed this file's 25/25 tests (107 ms); the lane owner's named-file run also passed 25/25. Current main is already the branch's parent; no sync commit was needed. No running-app QA or screenshots apply: the QA table's only-tests row is qa=none. No native chat or agent code changed, so a reference-mechanism comparison is inapplicable.

For Brennan's review: the retained static imports preserve the once-per-file runtime load. No visual change.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing: #33934 merged the same fix (main's workspace-layout-paired-desktop.test.ts now matches this branch exactly).

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