Add rollback to createGoldenWorktree on setup failure

Cleanup callbacks only register after setup succeeds. When a config command fails, the half-built worktree and branch leak into later test runs, causing flakiness. Now we roll back immediately and re-throw the setup error.
2026-10-22 08:03:37 +00:00 · 2026-08-13 09:45:48 -07:00
parent abd5d55420
commit f354bb1775
2 changed files with 83 additions and 13 deletions
@@ -26,19 +26,31 @@ export function createGoldenWorktree(repoPath: string, label: string): GoldenWor
    cwd: repoPath,
    stdio: 'pipe'
  })
-  execFileSync('git', ['config', 'extensions.worktreeConfig', 'true'], {
-    cwd: worktreePath,
-    stdio: 'pipe'
-  })
-  execFileSync('git', ['config', '--worktree', 'user.name', GOLDEN_GIT_AUTHOR_NAME], {
-    cwd: worktreePath,
-    stdio: 'pipe'
-  })
-  execFileSync('git', ['config', '--worktree', 'user.email', GOLDEN_GIT_AUTHOR_EMAIL], {
-    cwd: worktreePath,
-    stdio: 'pipe'
-  })
-  return { branchName, worktreePath }
+  const fixture: GoldenWorktree = { branchName, worktreePath }
+  // Callers only register cleanup once this returns, so roll back here or the
+  // half-built worktree and branch leak into every later run.
+  try {
+    execFileSync('git', ['config', 'extensions.worktreeConfig', 'true'], {
+      cwd: worktreePath,
+      stdio: 'pipe'
+    })
+    execFileSync('git', ['config', '--worktree', 'user.name', GOLDEN_GIT_AUTHOR_NAME], {
+      cwd: worktreePath,
+      stdio: 'pipe'
+    })
+    execFileSync('git', ['config', '--worktree', 'user.email', GOLDEN_GIT_AUTHOR_EMAIL], {
+      cwd: worktreePath,
+      stdio: 'pipe'
+    })
+  } catch (setupError) {
+    try {
+      cleanupGoldenWorktree(repoPath, fixture)
+    } catch {
+      // Keep the setup failure as the reported cause.
+    }
+    throw setupError
+  }
+  return fixture
 }

 export function cleanupGoldenWorktree(repoPath: string, fixture: GoldenWorktree): void {
@@ -0,0 +1,58 @@
+import { execFileSync } from 'node:child_process'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { createGoldenWorktree } from './golden-source-control'
+
+vi.mock('node:child_process', () => ({ execFileSync: vi.fn() }))
+
+const execFileSyncMock = vi.mocked(execFileSync)
+const gitArgsFor = (): string[][] =>
+  execFileSyncMock.mock.calls.map(([, args]) => (args ?? []) as string[])
+
+describe('createGoldenWorktree', () => {
+  beforeEach(() => {
+    execFileSyncMock.mockReset()
+  })
+
+  it('rolls back the worktree and branch when a configuration command fails', () => {
+    const setupError = new Error('git config --worktree unsupported')
+    execFileSyncMock.mockImplementation(((_file: string, args: string[]) => {
+      if (args[0] === 'config' && args[1] === 'extensions.worktreeConfig') {
+        throw setupError
+      }
+      return ''
+    }) as unknown as typeof execFileSync)
+
+    expect(() => createGoldenWorktree('/repo', 'rollback')).toThrow(setupError)
+
+    const worktreePath = gitArgsFor().find((args) => args[0] === 'worktree')?.[2]
+    expect(worktreePath).toBeDefined()
+    const branchName = gitArgsFor().find((args) => args[0] === 'branch')?.[2]
+    expect(gitArgsFor()).toContainEqual(['worktree', 'remove', '--force', worktreePath])
+    expect(branchName).toMatch(/^e2e-golden-rollback-/)
+  })
+
+  it('keeps the setup error when rollback itself fails', () => {
+    const setupError = new Error('git config --worktree unsupported')
+    execFileSyncMock.mockImplementation(((_file: string, args: string[]) => {
+      if (args[0] === 'config' && args[1] === 'extensions.worktreeConfig') {
+        throw setupError
+      }
+      if (args[0] === 'branch') {
+        throw new Error('branch is still checked out')
+      }
+      return ''
+    }) as unknown as typeof execFileSync)
+
+    expect(() => createGoldenWorktree('/repo', 'rollback-fails')).toThrow(setupError)
+  })
+
+  it('returns the fixture when every setup command succeeds', () => {
+    execFileSyncMock.mockReturnValue('')
+
+    const fixture = createGoldenWorktree('/repo', 'happy')
+
+    expect(fixture.branchName).toMatch(/^e2e-golden-happy-/)
+    expect(gitArgsFor().some((args) => args[0] === 'worktree' && args[1] === 'remove')).toBe(false)
+    expect(gitArgsFor().some((args) => args[0] === 'branch')).toBe(false)
+  })
+})