test(boot): fix Windows TempDir cleanup in legacy-session migration test#3371
Merged
Conversation
TestBuildMigratesLegacySessionsFromConfigSessionDir used t.Chdir(proj) to point Build at a temp project dir, which makes that dir the process cwd. On Windows a cwd directory cannot be removed, so t.TempDir cleanup failed (being used by another process) even though all assertions passed — this reddened the main-v2 Windows CI. Use the Build WorkspaceRoot option instead: it loads the project config without changing the process cwd. Fixes the #3247 regression.
esengine
added a commit
that referenced
this pull request
Jun 7, 2026
Windows/Linux CI intermittently reddened on t.TempDir auto-cleanup (RemoveAll: "being used by another process" on Windows, "directory not empty" on Linux) even though every assertion passed — across the boot legacy-migration tests and the desktop capabilities tests. #3371 blamed t.Chdir and fixed one test, but that same test kept failing afterward, so the real cause is a teardown race, not the cwd. Two independent fixes: 1. jobs.Manager.Close only cancelled the session context and returned without waiting for job goroutines to observe the cancel and exit, so a caller tearing down a t.TempDir could race a goroutine still holding a file under it. Close now waits on a WaitGroup — shutdown is graceful. 2. Add a robustTempDir test helper (boot + desktop) that retries RemoveAll for a short window, absorbing the brief post-Close interval where a background resource still holds a file under the dir. A dir that never frees is logged, not fatal, so a genuine leak stays visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the Windows CI regression from #3247.
Problem
TestBuildMigratesLegacySessionsFromConfigSessionDir used t.Chdir(proj) to point Build at a temp project dir, which makes that dir the process cwd. On Windows a cwd directory cannot be removed, so t.TempDir cleanup failed (being used by another process) even though all assertions passed — reddening the main-v2 Windows CI for every Go PR.
Fix
Use Build’s WorkspaceRoot option (added for desktop multi-project sessions) instead of t.Chdir. It loads the same project config without changing the process cwd, so the temp dir is removable on cleanup. The migration assertions are unchanged. Verified on macOS; the Windows CI on this PR confirms the cleanup.