Skip to content

fix(watcher-tests): await ready + harden Windows cleanup #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PatrickSys merged 2 commits into from
Mar 1, 2026
Merged

fix(watcher-tests): await ready + harden Windows cleanup #55

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87e7434
fix(watcher-tests): await ready + harden Windows cleanup
PatrickSys Mar 1, 2026
ebf15db
test(windows): reuse rmWithRetries in watcher suites
PatrickSys Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/core/file-watcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export interface FileWatcherOptions {
rootPath: string;
/** ms after last change before triggering. Default: 2000 */
debounceMs?: number;
/** Called once chokidar finishes initial scan and starts emitting change events */
onReady?: () => void;
/** Called once the debounce window expires after the last detected change */
onChanged: () => void;
}
Expand All @@ -28,7 +30,7 @@ function isTrackedSourcePath(filePath: string): boolean {
* Returns a stop() function that cancels the debounce timer and closes the watcher.
*/
export function startFileWatcher(opts: FileWatcherOptions): () => void {
const { rootPath, debounceMs = 2000, onChanged } = opts;
const { rootPath, debounceMs = 2000, onReady, onChanged } = opts;
let debounceTimer: ReturnType<typeof setTimeout> | undefined;

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

watcher
.on('ready', () => onReady?.())
.on('add', trigger)
.on('change', trigger)
.on('unlink', trigger)
Expand Down
8 changes: 7 additions & 1 deletion tests/auto-refresh-e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,15 @@ describe('Auto-refresh E2E', () => {
}
};

let resolveReady!: () => void;
const watcherReady = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stopWatcher = startFileWatcher({
rootPath: tempDir,
debounceMs: 200,
onReady: () => resolveReady(),
onChanged: () => {
const shouldRunNow = autoRefresh.onFileChange(indexStatus === 'indexing');
if (!shouldRunNow) return;
Expand All @@ -123,7 +129,7 @@ describe('Auto-refresh E2E', () => {
});

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

await waitFor(
Expand Down
47 changes: 38 additions & 9 deletions tests/file-watcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,20 @@ describe('FileWatcher', () => {
const debounceMs = 400;
let callCount = 0;

let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stop = startFileWatcher({
rootPath: tempDir,
debounceMs,
onReady: () => resolveReady(),
onChanged: () => { callCount++; },
});

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

it('debounces rapid changes into a single callback', async () => {
const debounceMs = 300;
const debounceMs = 800;
let callCount = 0;

let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stop = startFileWatcher({
rootPath: tempDir,
debounceMs,
onReady: () => resolveReady(),
onChanged: () => { callCount++; },
});

try {
// Give chokidar a moment to finish initializing before the first write
await new Promise((resolve) => setTimeout(resolve, 100));
await ready;
// Write 5 files in quick succession — all within the debounce window
for (let i = 0; i < 5; i++) {
await fs.writeFile(path.join(tempDir, `file${i}.ts`), `export const x${i} = ${i};`);
await new Promise((resolve) => setTimeout(resolve, 50));
await new Promise((resolve) => setTimeout(resolve, 20));
}
// Wait for debounce to settle
await new Promise((resolve) => setTimeout(resolve, debounceMs + 400));
await new Promise((resolve) => setTimeout(resolve, debounceMs + 1200));
expect(callCount).toBe(1);
} finally {
stop();
Expand All @@ -68,14 +79,20 @@ describe('FileWatcher', () => {
const debounceMs = 500;
let callCount = 0;

let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stop = startFileWatcher({
rootPath: tempDir,
debounceMs,
onReady: () => resolveReady(),
onChanged: () => { callCount++; },
});

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

let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stop = startFileWatcher({
rootPath: tempDir,
debounceMs,
onReady: () => resolveReady(),
onChanged: () => {
callCount++;
}
});

try {
await new Promise((resolve) => setTimeout(resolve, 100));
await ready;
await fs.writeFile(path.join(tempDir, 'notes.txt'), 'this should be ignored');
await new Promise((resolve) => setTimeout(resolve, debounceMs + 700));
expect(callCount).toBe(0);
Expand All @@ -112,16 +135,22 @@ describe('FileWatcher', () => {
const debounceMs = 250;
let callCount = 0;

let resolveReady!: () => void;
const ready = new Promise<void>((resolve) => {
resolveReady = resolve;
});

const stop = startFileWatcher({
rootPath: tempDir,
debounceMs,
onReady: () => resolveReady(),
onChanged: () => {
callCount++;
}
});

try {
await new Promise((resolve) => setTimeout(resolve, 100));
await ready;
await fs.writeFile(path.join(tempDir, '.gitignore'), 'dist/\n');
await new Promise((resolve) => setTimeout(resolve, debounceMs + 700));
expect(callCount).toBe(1);
Expand Down
24 changes: 21 additions & 3 deletions tests/impact-2hop.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { promises as fs } from 'fs';
import os from 'os';
import path from 'path';
import { CodebaseIndexer } from '../src/core/indexer.js';
import { dispatchTool } from '../src/tools/index.js';
Expand All @@ -17,9 +18,26 @@ describe('Impact candidates (2-hop)', () => {
let tempRoot: string | null = null;
const token = 'UNIQUETOKEN123';

async function rmWithRetries(targetPath: string): Promise<void> {
const maxAttempts = 8;
let delayMs = 25;

for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
await fs.rm(targetPath, { recursive: true, force: true });
return;
} catch (error) {
const code = (error as { code?: string }).code;
const retryable = code === 'ENOTEMPTY' || code === 'EPERM' || code === 'EBUSY';
if (!retryable || attempt === maxAttempts) throw error;
await new Promise((r) => setTimeout(r, delayMs));
delayMs *= 2;
}
}
}
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider extracting rmWithRetries to a shared test utility file since it's duplicated in search-snippets.test.ts and will likely be needed in other test files

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


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

afterEach(async () => {
if (tempRoot) {
await fs.rm(tempRoot, { recursive: true, force: true });
await rmWithRetries(tempRoot);
tempRoot = null;
}
});
Expand Down
24 changes: 21 additions & 3 deletions tests/search-snippets.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ import { CodebaseIndexer } from '../src/core/indexer.js';
describe('Search Snippets with Scope Headers', () => {
let tempRoot: string | null = null;

async function rmWithRetries(targetPath: string): Promise<void> {
const maxAttempts = 8;
let delayMs = 25;

for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
await fs.rm(targetPath, { recursive: true, force: true });
return;
} catch (error) {
const code = (error as { code?: string }).code;
const retryable = code === 'ENOTEMPTY' || code === 'EPERM' || code === 'EBUSY';
if (!retryable || attempt === maxAttempts) throw error;
await new Promise((r) => setTimeout(r, delayMs));
delayMs *= 2;
}
}
}

beforeEach(async () => {
vi.resetModules();
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'search-snippets-test-'));
Expand Down Expand Up @@ -91,15 +109,15 @@ export const VERSION = '1.0.0';
config: { skipEmbedding: true }
});
await indexer.index();
});
}, 30000);

afterEach(async () => {
if (tempRoot) {
await fs.rm(tempRoot, { recursive: true, force: true });
await rmWithRetries(tempRoot);
tempRoot = null;
}
delete process.env.CODEBASE_ROOT;
});
}, 30000);

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