Skip to content

test(boot): fix Windows TempDir cleanup in legacy-session migration test#3371

Merged
esengine merged 1 commit into
main-v2from
fix/windows-ci-3247
Jun 6, 2026
Merged

test(boot): fix Windows TempDir cleanup in legacy-session migration test#3371
esengine merged 1 commit into
main-v2from
fix/windows-ci-3247

Conversation

@esengine

@esengine esengine commented Jun 6, 2026

Copy link
Copy Markdown
Owner

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.

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 esengine requested a review from SivanCola as a code owner June 6, 2026 15:01
@github-actions github-actions Bot added v2 Go rewrite (1.x) — main-v2 branch, active development config Configuration & setup (internal/config) labels Jun 6, 2026
@esengine esengine merged commit 2384eb4 into main-v2 Jun 6, 2026
10 checks passed
@esengine esengine deleted the fix/windows-ci-3247 branch June 6, 2026 15:05
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Configuration & setup (internal/config) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant