From f354bb17759b8f7251f4c7779493f54bec5af2cf Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:24:24 -0700 Subject: [PATCH] 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. --- tests/e2e/helpers/golden-source-control.ts | 38 +++++++----- .../golden-source-control.unit.test.ts | 58 +++++++++++++++++++ 2 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 tests/e2e/helpers/golden-source-control.unit.test.ts diff --git a/tests/e2e/helpers/golden-source-control.ts b/tests/e2e/helpers/golden-source-control.ts index 251cbdf8f35..a0c2cf99300 100644 --- a/tests/e2e/helpers/golden-source-control.ts +++ b/tests/e2e/helpers/golden-source-control.ts @@ -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 { diff --git a/tests/e2e/helpers/golden-source-control.unit.test.ts b/tests/e2e/helpers/golden-source-control.unit.test.ts new file mode 100644 index 00000000000..c8e069bf49a --- /dev/null +++ b/tests/e2e/helpers/golden-source-control.unit.test.ts @@ -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) + }) +})