Skip to content

Commit a038201

Browse files
committed
test: make withTempDir and withLinkedWorktreeFixture async-safe
Convert test helpers to properly await async callbacks before cleanup. This prevents race conditions where temp directories or worktrees are cleaned up before async operations complete. Adds dedicated tests validating cleanup waits for pending promises.
1 parent 2b8d4d1 commit a038201

5 files changed

Lines changed: 96 additions & 60 deletions

File tree

tests/bin/doctor/config.test.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ import { getConfigInfo } from '@/bin/doctor/config';
1010
import { withTempDir } from '../../helpers.ts';
1111

1212
describe('getConfigInfo', () => {
13-
test('handles missing config files', () => {
14-
withTempDir('doctor-test-', (tmpDir) => {
13+
test('handles missing config files', async () => {
14+
await withTempDir('doctor-test-', (tmpDir) => {
1515
const info = getConfigInfo(tmpDir);
1616
expect(info.projectConfig.exists).toBe(false);
1717
expect(info.effectiveRules).toEqual([]);
1818
expect(info.shadowedRules).toEqual([]);
1919
});
2020
});
2121

22-
test('detects valid project config', () => {
23-
withTempDir('doctor-test-', (tmpDir) => {
22+
test('detects valid project config', async () => {
23+
await withTempDir('doctor-test-', (tmpDir) => {
2424
writeFileSync(
2525
join(tmpDir, '.safety-net.json'),
2626
JSON.stringify({
@@ -44,8 +44,8 @@ describe('getConfigInfo', () => {
4444
});
4545
});
4646

47-
test('detects invalid project config', () => {
48-
withTempDir('doctor-test-', (tmpDir) => {
47+
test('detects invalid project config', async () => {
48+
await withTempDir('doctor-test-', (tmpDir) => {
4949
writeFileSync(join(tmpDir, '.safety-net.json'), '{ "version": 2 }');
5050
const info = getConfigInfo(tmpDir);
5151
expect(info.projectConfig.exists).toBe(true);
@@ -54,8 +54,8 @@ describe('getConfigInfo', () => {
5454
});
5555
});
5656

57-
test('excludes rules from invalid config (wrong version)', () => {
58-
withTempDir('doctor-test-', (tmpDir) => {
57+
test('excludes rules from invalid config (wrong version)', async () => {
58+
await withTempDir('doctor-test-', (tmpDir) => {
5959
writeFileSync(
6060
join(tmpDir, '.safety-net.json'),
6161
JSON.stringify({
@@ -77,24 +77,24 @@ describe('getConfigInfo', () => {
7777
});
7878
});
7979

80-
test('handles malformed JSON in config', () => {
81-
withTempDir('doctor-test-', (tmpDir) => {
80+
test('handles malformed JSON in config', async () => {
81+
await withTempDir('doctor-test-', (tmpDir) => {
8282
writeFileSync(join(tmpDir, '.safety-net.json'), '{ invalid json }');
8383
const info = getConfigInfo(tmpDir);
8484
expect(info.effectiveRules).toEqual([]);
8585
});
8686
});
8787

88-
test('handles empty config file', () => {
89-
withTempDir('doctor-test-', (tmpDir) => {
88+
test('handles empty config file', async () => {
89+
await withTempDir('doctor-test-', (tmpDir) => {
9090
writeFileSync(join(tmpDir, '.safety-net.json'), ' ');
9191
const info = getConfigInfo(tmpDir);
9292
expect(info.effectiveRules).toEqual([]);
9393
});
9494
});
9595

96-
test('handles config without rules array', () => {
97-
withTempDir('doctor-test-', (tmpDir) => {
96+
test('handles config without rules array', async () => {
97+
await withTempDir('doctor-test-', (tmpDir) => {
9898
writeFileSync(join(tmpDir, '.safety-net.json'), '{ "version": 1 }');
9999
const info = getConfigInfo(tmpDir);
100100
expect(info.effectiveRules).toEqual([]);

tests/bin/explain/command.test.ts

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ function expectDangerousTextStep(command: string): void {
3434
).toBeDefined();
3535
}
3636

37-
function expectWorktreeExplainBlocked(command: (mainWorktree: string) => string, reason: string) {
38-
withLinkedWorktreeFixture((fixture) => {
37+
async function expectWorktreeExplainBlocked(
38+
command: (mainWorktree: string) => string,
39+
reason: string,
40+
) {
41+
await withLinkedWorktreeFixture((fixture) => {
3942
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
4043
const result = explainCommand(command(toShellPath(fixture.mainWorktree)), {
4144
cwd: fixture.linkedWorktree,
@@ -394,22 +397,14 @@ describe('explainCommand shell wrapper edge cases', () => {
394397

395398
describe('explainCommand max recursion depth', () => {
396399
test('deeply nested command hits max recursion', () => {
397-
const deepNested =
398-
'bash -c "bash -c \\"bash -c \\\\\\"bash -c \\\\\\\\\\\\\\"bash -c \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"echo deep\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\"\\\\\\"\\"" ';
399-
const steps = getTraceSteps(explainCommand(deepNested));
400-
expect(
401-
steps.filter((s) => s.type === 'recurse').length +
402-
(recursionLimitErrorStep(deepNested) ? 1 : 0),
403-
).toBeGreaterThan(0);
400+
const deepNested = nestedBashCommand('echo deep', 10);
401+
expect(recursionLimitErrorStep(deepNested)).toBeTruthy();
404402
});
405403

406404
test('hits exact max recursion depth of 5', () => {
407405
const level5 =
408406
'bash -c "bash -c \\"bash -c \\\\\\"bash -c \\\\\\\\\\\\\\"bash -c \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"echo hi\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\"\\\\\\"\\"" ';
409-
const steps = getTraceSteps(explainCommand(level5));
410-
expect(
411-
steps.filter((s) => s.type === 'recurse').length >= 3 || recursionLimitErrorStep(level5),
412-
).toBeTruthy();
407+
expect(recursionLimitErrorStep(level5)).toBeFalsy();
413408
});
414409

415410
test('hits max recursion depth with 10 nested bash -c calls', () => {
@@ -581,54 +576,57 @@ describe('explainCommand fallback scan with find', () => {
581576
});
582577

583578
describe('explainCommand worktree parity', () => {
584-
test('uses wrapper cwd when explaining worktree relaxation', () => {
585-
expectWorktreeExplainBlocked((main) => `env -C ${main} git reset --hard`, 'git reset --hard');
579+
test('uses wrapper cwd when explaining worktree relaxation', async () => {
580+
await expectWorktreeExplainBlocked(
581+
(main) => `env -C ${main} git reset --hard`,
582+
'git reset --hard',
583+
);
586584
});
587585

588-
test('carries exported git context overrides into later segments', () => {
589-
expectWorktreeExplainBlocked(
586+
test('carries exported git context overrides into later segments', async () => {
587+
await expectWorktreeExplainBlocked(
590588
(main) => `export GIT_WORK_TREE=${main}; git reset --hard`,
591589
'git reset --hard',
592590
);
593591
});
594592

595-
test('passes wrapper cwd into recursive explain analysis', () => {
596-
expectWorktreeExplainBlocked(
593+
test('passes wrapper cwd into recursive explain analysis', async () => {
594+
await expectWorktreeExplainBlocked(
597595
(main) => `env -C ${main} sh -c "git reset --hard"`,
598596
'git reset --hard',
599597
);
600598
});
601599

602-
test('passes stripped env into recursive explain analysis', () => {
603-
expectWorktreeExplainBlocked(
600+
test('passes stripped env into recursive explain analysis', async () => {
601+
await expectWorktreeExplainBlocked(
604602
(main) => `GIT_WORK_TREE=${main} sh -c "git reset --hard"`,
605603
'git reset --hard',
606604
);
607605
});
608606

609-
test('carries nested exported git context overrides across inner segments', () => {
610-
expectWorktreeExplainBlocked(
607+
test('carries nested exported git context overrides across inner segments', async () => {
608+
await expectWorktreeExplainBlocked(
611609
(main) => `sh -c "export GIT_WORK_TREE=${main}; git reset --hard"`,
612610
'git reset --hard',
613611
);
614612
});
615613

616-
test('includes keyword-export git context overrides in current segment', () => {
617-
expectWorktreeExplainBlocked(
614+
test('includes keyword-export git context overrides in current segment', async () => {
615+
await expectWorktreeExplainBlocked(
618616
(main) => `set -k; git restore file.txt GIT_WORK_TREE=${main}`,
619617
'git restore',
620618
);
621619
});
622620

623-
test('includes nested keyword-export git context overrides in current segment', () => {
624-
expectWorktreeExplainBlocked(
621+
test('includes nested keyword-export git context overrides in current segment', async () => {
622+
await expectWorktreeExplainBlocked(
625623
(main) => `sh -c "set -k; git restore file.txt GIT_WORK_TREE=${main}"`,
626624
'git restore',
627625
);
628626
});
629627

630-
test('honors parallel nested overrides when explaining remote commands', () => {
631-
withLinkedWorktreeFixture((fixture) => {
628+
test('honors parallel nested overrides when explaining remote commands', async () => {
629+
await withLinkedWorktreeFixture((fixture) => {
632630
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
633631
const result = explainCommand('parallel -S host sh -c "git reset --hard" ::: x', {
634632
cwd: fixture.linkedWorktree,
@@ -640,8 +638,8 @@ describe('explainCommand worktree parity', () => {
640638
});
641639
});
642640

643-
test('does not report worktree relaxation for fallback embedded git', () => {
644-
withLinkedWorktreeFixture((fixture) => {
641+
test('does not report worktree relaxation for fallback embedded git', async () => {
642+
await withLinkedWorktreeFixture((fixture) => {
645643
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
646644
const result = explainCommand('ssh host git clean -f', { cwd: fixture.linkedWorktree });
647645
const worktreeStep = result.trace.segments

tests/core/analyze/analyze-coverage.test.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111

1212
const EMPTY_CONFIG: Config = { version: 1, rules: [] };
1313

14-
function analyzeInLinkedWorktree(command: (mainWorktree: string) => string) {
14+
async function analyzeInLinkedWorktree(command: (mainWorktree: string) => string) {
1515
return withLinkedWorktreeFixture((fixture) =>
1616
withEnv({ SAFETY_NET_WORKTREE: '1' }, () =>
1717
analyzeCommand(command(toShellPath(fixture.mainWorktree)), {
@@ -180,15 +180,15 @@ describe('analyzeCommand (coverage)', () => {
180180
});
181181

182182
describe('shell git context env state branches', () => {
183-
test('command -- export target is tracked across segments', () => {
184-
const result = analyzeInLinkedWorktree(
183+
test('command -- export target is tracked across segments', async () => {
184+
const result = await analyzeInLinkedWorktree(
185185
(main) => `command -- export GIT_WORK_TREE=${main}; git reset --hard`,
186186
);
187187
expect(result?.reason).toContain('git reset --hard');
188188
});
189189

190-
test('command inspection with no executable target leaves later git context unchanged', () => {
191-
withLinkedWorktreeFixture((fixture) => {
190+
test('command inspection with no executable target leaves later git context unchanged', async () => {
191+
await withLinkedWorktreeFixture((fixture) => {
192192
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
193193
expect(
194194
analyzeCommand(
@@ -213,8 +213,8 @@ describe('analyzeCommand (coverage)', () => {
213213
});
214214
});
215215

216-
test('export option parsing tracks only valid export operands', () => {
217-
withLinkedWorktreeFixture((fixture) => {
216+
test('export option parsing tracks only valid export operands', async () => {
217+
await withLinkedWorktreeFixture((fixture) => {
218218
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
219219
expect(
220220
analyzeCommand(
@@ -227,16 +227,21 @@ describe('analyzeCommand (coverage)', () => {
227227
),
228228
).toBeNull();
229229

230-
const result = analyzeInLinkedWorktree(
231-
(main) => `export -- GIT_WORK_TREE=${main}; git reset --hard`,
230+
const result = analyzeCommand(
231+
`export -- GIT_WORK_TREE=${toShellPath(fixture.mainWorktree)}; git reset --hard`,
232+
{
233+
cwd: fixture.linkedWorktree,
234+
config: EMPTY_CONFIG,
235+
worktreeMode: true,
236+
},
232237
);
233238
expect(result?.reason).toContain('git reset --hard');
234239
});
235240
});
236241
});
237242

238-
test('exporting an unset tracked name uses an empty effective value', () => {
239-
withLinkedWorktreeFixture((fixture) => {
243+
test('exporting an unset tracked name uses an empty effective value', async () => {
244+
await withLinkedWorktreeFixture((fixture) => {
240245
withEnv({ SAFETY_NET_WORKTREE: '1' }, () => {
241246
const result = analyzeCommand('export GIT_WORK_TREE; git reset --hard', {
242247
cwd: fixture.linkedWorktree,

tests/helpers.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import { existsSync } from 'node:fs';
3+
import { withLinkedWorktreeFixture, withTempDir } from './helpers.ts';
4+
5+
describe('test helpers', () => {
6+
test('withTempDir waits for async callbacks before cleanup', async () => {
7+
let tempDir = '';
8+
9+
await withTempDir('safety-net-helper-', async (dir) => {
10+
tempDir = dir;
11+
await Promise.resolve();
12+
expect(existsSync(dir)).toBe(true);
13+
});
14+
15+
expect(existsSync(tempDir)).toBe(false);
16+
});
17+
18+
test('withLinkedWorktreeFixture waits for async callbacks before cleanup', async () => {
19+
let rootDir = '';
20+
21+
await withLinkedWorktreeFixture(async (fixture) => {
22+
rootDir = fixture.rootDir;
23+
await Promise.resolve();
24+
expect(existsSync(fixture.rootDir)).toBe(true);
25+
});
26+
27+
expect(existsSync(rootDir)).toBe(false);
28+
});
29+
});

tests/helpers.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ export function withEnv<T>(env: Record<string, string>, fn: () => T): T {
6464
}
6565
}
6666

67-
export function withTempDir<T>(prefix: string, fn: (dir: string) => T): T {
67+
export async function withTempDir<T>(prefix: string, fn: (dir: string) => T | Promise<T>) {
6868
const dir = mkdtempSync(join(tmpdir(), prefix));
6969
try {
70-
return fn(dir);
70+
const result = await fn(dir);
71+
return result;
7172
} finally {
7273
rmSync(dir, { recursive: true, force: true });
7374
}
@@ -202,10 +203,13 @@ export function createLinkedWorktreeFixture(): LinkedWorktreeFixture {
202203
};
203204
}
204205

205-
export function withLinkedWorktreeFixture<T>(fn: (fixture: LinkedWorktreeFixture) => T): T {
206+
export async function withLinkedWorktreeFixture<T>(
207+
fn: (fixture: LinkedWorktreeFixture) => T | Promise<T>,
208+
) {
206209
const fixture = createLinkedWorktreeFixture();
207210
try {
208-
return fn(fixture);
211+
const result = await fn(fixture);
212+
return result;
209213
} finally {
210214
fixture.cleanup();
211215
}

0 commit comments

Comments
 (0)