Repository navigation
test(orcad): retry launcher-isolation teardown when Windows holds node.exe (EBUSY) - #28061
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
📥 Commits
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This change only adds retries to integration-test cleanup. No concrete merge-blocking risk is established, so it is ready to merge subject to normal checks. Pre-merge checks ·
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the test-only change and the Windows EBUSY teardown issue it addresses. |
| Description check | |
The description is detailed and covers the problem, change, rationale, testing, scope, visual proof, and checklist. It marks the linked issue as N/A and leaves some review or full-suite validation det… |
| 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. |
✨ Finishing Touches 💡 1
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
-
- 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.
An exited node.exe copy can stay locked for a moment on Windows, so the afterAll rm failed the hourly build's windows-11-arm persistence job.
61d4c89 to
3403501
Compare
ELI5
The hourly macOS dev build on main went red. Its Windows ARM test job passed all 1060 tests, then failed while deleting its temporary folder: Windows briefly keeps a just-exited
node.exelocked. The cleanup now retries for a moment instead of failing right away.What Changed
Problem. Hourly macOS Dev Build run 38295856465 (main at
8a7d19e3300) failed inorcad-template / persistence (windows-11-arm). Every test insrc/main/orcad/orcad-launcher-isolation.integration.test.tspassed. The file was then marked failed because itsafterAllcleanup threw:The "refuses changed server bytes" test runs a copied
node.exefrom that folder.runProcesswaits for the process to exit, but Windows can keep the executable's file locked for a short time after that (for example, while antivirus scans it). Thermright afterwards loses that race now and then.Fix. The cleanup calls
rm(root, { recursive: true, force: true, maxRetries: 8, retryDelay: 100 }). Node retriesEBUSY/EPERMfor up to about a second before giving up. Other temp-directory teardowns in this repo use the same options, for examplelocal-worktree-removal-recovery-live.test.tsandfish-xdg-data-dirs-handoff.test.ts.User-facing change: none. This only changes test cleanup.
Why
Retrying the delete is the existing fix for this Windows lock in the repo. The other options were to wait a fixed time after the process exits, which is slower and still a guess, or to stop cleaning up, which leaves temp folders behind on CI runners.
Another part of main's red CI, the
check:main-sync-spawn-ratchetcrash that fails static analysis on every PR, is already fixed in #28054 and #28053, so it isn't repeated here.Linked Issue
N/A (main CI flake)
Visual Proof
N/A: test-only change, nothing visible changes.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
pnpm test src/main/orcad/orcad-launcher-isolation.integration.test.tspasses on macOS. The tests that need the bundled Node runtime skip there, so the Windows race itself isn't reproduced locally. The windows-11-arm job covers it.orca-ci-checks: lint, code quality, localization andpnpm run typecheckpass. The native lint audit and the sync-spawn ratchet fail on current main until fix(ci): point the main sync-spawn ratchet at the shared ratchet library #28054 lands; they don't involve this file.Review
Agent skill upstream boundary
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)