Skip to content

Commit 9929bb0

Browse files
authored
fix(watcher-tests): await ready + harden Windows cleanup (#55)
* fix(watcher-tests): await ready + harden Windows cleanup * test(windows): reuse rmWithRetries in watcher suites
1 parent dd0cca8 commit 9929bb0

File tree

6 files changed

+80
-19
lines changed

6 files changed

+80
-19
lines changed

src/core/file-watcher.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ export interface FileWatcherOptions {
66
rootPath: string;
77
/** ms after last change before triggering. Default: 2000 */
88
debounceMs?: number;
9+
/** Called once chokidar finishes initial scan and starts emitting change events */
10+
onReady?: () => void;
911
/** Called once the debounce window expires after the last detected change */
1012
onChanged: () => void;
1113
}
@@ -28,7 +30,7 @@ function isTrackedSourcePath(filePath: string): boolean {
2830
* Returns a stop() function that cancels the debounce timer and closes the watcher.
2931
*/
3032
export function startFileWatcher(opts: FileWatcherOptions): () => void {
31-
const { rootPath, debounceMs = 2000, onChanged } = opts;
33+
const { rootPath, debounceMs = 2000, onReady, onChanged } = opts;
3234
let debounceTimer: ReturnType<typeof setTimeout> | undefined;
3335

3436
const trigger = (filePath: string) => {
@@ -59,6 +61,7 @@ export function startFileWatcher(opts: FileWatcherOptions): () => void {
5961
});
6062

6163
watcher
64+
.on('ready', () => onReady?.())
6265
.on('add', trigger)
6366
.on('change', trigger)
6467
.on('unlink', trigger)

tests/auto-refresh-e2e.test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import path from 'path';
55
import { startFileWatcher } from '../src/core/file-watcher.js';
66
import { createAutoRefreshController } from '../src/core/auto-refresh.js';
77
import { CodebaseIndexer } from '../src/core/indexer.js';
8+
import { rmWithRetries } from './test-helpers.js';
89
import {
910
CODEBASE_CONTEXT_DIRNAME,
1011
KEYWORD_INDEX_FILENAME
@@ -72,7 +73,7 @@ describe('Auto-refresh E2E', () => {
7273
});
7374

7475
afterEach(async () => {
75-
await fs.rm(tempDir, { recursive: true, force: true });
76+
await rmWithRetries(tempDir);
7677
});
7778

7879
it('updates index after a file edit without manual refresh_index', async () => {
@@ -111,9 +112,15 @@ describe('Auto-refresh E2E', () => {
111112
}
112113
};
113114

115+
let resolveReady!: () => void;
116+
const watcherReady = new Promise<void>((resolve) => {
117+
resolveReady = resolve;
118+
});
119+
114120
const stopWatcher = startFileWatcher({
115121
rootPath: tempDir,
116122
debounceMs: 200,
123+
onReady: () => resolveReady(),
117124
onChanged: () => {
118125
const shouldRunNow = autoRefresh.onFileChange(indexStatus === 'indexing');
119126
if (!shouldRunNow) return;
@@ -123,7 +130,7 @@ describe('Auto-refresh E2E', () => {
123130
});
124131

125132
try {
126-
await sleep(250);
133+
await watcherReady;
127134
await fs.writeFile(path.join(tempDir, 'src', 'app.ts'), 'export const token = "UPDATED_TOKEN";\n');
128135

129136
await waitFor(

tests/file-watcher.test.ts

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { promises as fs } from 'fs';
33
import path from 'path';
44
import os from 'os';
55
import { startFileWatcher } from '../src/core/file-watcher.js';
6+
import { rmWithRetries } from './test-helpers.js';
67

78
describe('FileWatcher', () => {
89
let tempDir: string;
@@ -12,22 +13,27 @@ describe('FileWatcher', () => {
1213
});
1314

1415
afterEach(async () => {
15-
await fs.rm(tempDir, { recursive: true, force: true });
16+
await rmWithRetries(tempDir);
1617
});
1718

1819
it('triggers onChanged after debounce window', async () => {
1920
const debounceMs = 400;
2021
let callCount = 0;
2122

23+
let resolveReady!: () => void;
24+
const ready = new Promise<void>((resolve) => {
25+
resolveReady = resolve;
26+
});
27+
2228
const stop = startFileWatcher({
2329
rootPath: tempDir,
2430
debounceMs,
31+
onReady: () => resolveReady(),
2532
onChanged: () => { callCount++; },
2633
});
2734

2835
try {
29-
// Give chokidar a moment to finish initializing before the first write
30-
await new Promise((resolve) => setTimeout(resolve, 100));
36+
await ready;
3137
await fs.writeFile(path.join(tempDir, 'test.ts'), 'export const x = 1;');
3238
// Wait for chokidar to pick up the event (including awaitWriteFinish stabilityThreshold)
3339
// + debounce window + OS scheduling slack
@@ -39,25 +45,31 @@ describe('FileWatcher', () => {
3945
}, 8000);
4046

4147
it('debounces rapid changes into a single callback', async () => {
42-
const debounceMs = 300;
48+
const debounceMs = 800;
4349
let callCount = 0;
4450

51+
let resolveReady!: () => void;
52+
const ready = new Promise<void>((resolve) => {
53+
resolveReady = resolve;
54+
});
55+
4556
const stop = startFileWatcher({
4657
rootPath: tempDir,
4758
debounceMs,
59+
onReady: () => resolveReady(),
4860
onChanged: () => { callCount++; },
4961
});
5062

5163
try {
5264
// Give chokidar a moment to finish initializing before the first write
53-
await new Promise((resolve) => setTimeout(resolve, 100));
65+
await ready;
5466
// Write 5 files in quick succession — all within the debounce window
5567
for (let i = 0; i < 5; i++) {
5668
await fs.writeFile(path.join(tempDir, `file${i}.ts`), `export const x${i} = ${i};`);
57-
await new Promise((resolve) => setTimeout(resolve, 50));
69+
await new Promise((resolve) => setTimeout(resolve, 20));
5870
}
5971
// Wait for debounce to settle
60-
await new Promise((resolve) => setTimeout(resolve, debounceMs + 400));
72+
await new Promise((resolve) => setTimeout(resolve, debounceMs + 1200));
6173
expect(callCount).toBe(1);
6274
} finally {
6375
stop();
@@ -68,14 +80,20 @@ describe('FileWatcher', () => {
6880
const debounceMs = 500;
6981
let callCount = 0;
7082

83+
let resolveReady!: () => void;
84+
const ready = new Promise<void>((resolve) => {
85+
resolveReady = resolve;
86+
});
87+
7188
const stop = startFileWatcher({
7289
rootPath: tempDir,
7390
debounceMs,
91+
onReady: () => resolveReady(),
7492
onChanged: () => { callCount++; },
7593
});
7694

7795
// Give chokidar a moment to finish initializing before the first write
78-
await new Promise((resolve) => setTimeout(resolve, 100));
96+
await ready;
7997
await fs.writeFile(path.join(tempDir, 'cancel.ts'), 'export const y = 99;');
8098
// Let chokidar detect the event (including awaitWriteFinish stabilityThreshold)
8199
// but stop before the debounce window expires.
@@ -90,16 +108,22 @@ describe('FileWatcher', () => {
90108
const debounceMs = 250;
91109
let callCount = 0;
92110

111+
let resolveReady!: () => void;
112+
const ready = new Promise<void>((resolve) => {
113+
resolveReady = resolve;
114+
});
115+
93116
const stop = startFileWatcher({
94117
rootPath: tempDir,
95118
debounceMs,
119+
onReady: () => resolveReady(),
96120
onChanged: () => {
97121
callCount++;
98122
}
99123
});
100124

101125
try {
102-
await new Promise((resolve) => setTimeout(resolve, 100));
126+
await ready;
103127
await fs.writeFile(path.join(tempDir, 'notes.txt'), 'this should be ignored');
104128
await new Promise((resolve) => setTimeout(resolve, debounceMs + 700));
105129
expect(callCount).toBe(0);
@@ -112,16 +136,22 @@ describe('FileWatcher', () => {
112136
const debounceMs = 250;
113137
let callCount = 0;
114138

139+
let resolveReady!: () => void;
140+
const ready = new Promise<void>((resolve) => {
141+
resolveReady = resolve;
142+
});
143+
115144
const stop = startFileWatcher({
116145
rootPath: tempDir,
117146
debounceMs,
147+
onReady: () => resolveReady(),
118148
onChanged: () => {
119149
callCount++;
120150
}
121151
});
122152

123153
try {
124-
await new Promise((resolve) => setTimeout(resolve, 100));
154+
await ready;
125155
await fs.writeFile(path.join(tempDir, '.gitignore'), 'dist/\n');
126156
await new Promise((resolve) => setTimeout(resolve, debounceMs + 700));
127157
expect(callCount).toBe(1);

tests/impact-2hop.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
22
import { promises as fs } from 'fs';
3+
import os from 'os';
34
import path from 'path';
45
import { CodebaseIndexer } from '../src/core/indexer.js';
56
import { dispatchTool } from '../src/tools/index.js';
67
import type { ToolContext } from '../src/tools/types.js';
8+
import { rmWithRetries } from './test-helpers.js';
79
import {
810
CODEBASE_CONTEXT_DIRNAME,
911
INTELLIGENCE_FILENAME,
@@ -18,8 +20,7 @@ describe('Impact candidates (2-hop)', () => {
1820
const token = 'UNIQUETOKEN123';
1921

2022
beforeEach(async () => {
21-
// Keep test artifacts under CWD (mirrors other indexer tests and avoids OS tmp quirks)
22-
tempRoot = await fs.mkdtemp(path.join(process.cwd(), '.tmp-impact-2hop-'));
23+
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'impact-2hop-'));
2324
const srcDir = path.join(tempRoot, 'src');
2425
await fs.mkdir(srcDir, { recursive: true });
2526
await fs.writeFile(path.join(tempRoot, 'package.json'), JSON.stringify({ name: 'impact-2hop' }));
@@ -40,7 +41,7 @@ describe('Impact candidates (2-hop)', () => {
4041

4142
afterEach(async () => {
4243
if (tempRoot) {
43-
await fs.rm(tempRoot, { recursive: true, force: true });
44+
await rmWithRetries(tempRoot);
4445
tempRoot = null;
4546
}
4647
});

tests/search-snippets.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { promises as fs } from 'fs';
33
import os from 'os';
44
import path from 'path';
55
import { CodebaseIndexer } from '../src/core/indexer.js';
6+
import { rmWithRetries } from './test-helpers.js';
67

78
describe('Search Snippets with Scope Headers', () => {
89
let tempRoot: string | null = null;
@@ -91,15 +92,15 @@ export const VERSION = '1.0.0';
9192
config: { skipEmbedding: true }
9293
});
9394
await indexer.index();
94-
});
95+
}, 30000);
9596

9697
afterEach(async () => {
9798
if (tempRoot) {
98-
await fs.rm(tempRoot, { recursive: true, force: true });
99+
await rmWithRetries(tempRoot);
99100
tempRoot = null;
100101
}
101102
delete process.env.CODEBASE_ROOT;
102-
});
103+
}, 30000);
103104

104105
it('returns snippets when includeSnippets=true', async () => {
105106
if (!tempRoot) throw new Error('tempRoot not initialized');

tests/test-helpers.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { promises as fs } from 'fs';
2+
3+
export async function rmWithRetries(targetPath: string): Promise<void> {
4+
const maxAttempts = 8;
5+
let delayMs = 25;
6+
7+
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
8+
try {
9+
await fs.rm(targetPath, { recursive: true, force: true });
10+
return;
11+
} catch (error) {
12+
const code = (error as { code?: string }).code;
13+
const retryable = code === 'ENOTEMPTY' || code === 'EPERM' || code === 'EBUSY';
14+
if (!retryable || attempt === maxAttempts) throw error;
15+
await new Promise((resolve) => setTimeout(resolve, delayMs));
16+
delayMs *= 2;
17+
}
18+
}
19+
}

0 commit comments

Comments
 (0)