Skip to content

Commit 5ba5584

Browse files
cklinCopilot
andcommitted
Fix flaky Windows tests: dispose file watchers and use retried cleanup
The root cause was twofold: 1. MultiFileSystemWatcher.addWatcher() never disposed the FileSystemWatcher object itself — only its event subscriptions. This leaked OS file handles on Windows, preventing temp directory deletion. 2. The tests used tmp's synchronous rimrafSync for cleanup, which has no retry logic for Windows' asynchronous handle release. Fix: - Dispose the FileSystemWatcher itself in MultiFileSystemWatcher - Replace removeCallback with async fs.rm with maxRetries, giving Windows time to release handles between retries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bf5f763 commit 5ba5584

4 files changed

Lines changed: 28 additions & 24 deletions

File tree

extensions/ql-vscode/src/common/vscode/multi-file-system-watcher.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class WatcherCollection extends DisposableObject {
2020
*/
2121
public addWatcher(pattern: GlobPattern, listener: (e: Uri) => void): void {
2222
const watcher = workspace.createFileSystemWatcher(pattern);
23+
this.push(watcher);
2324
this.push(watcher.onDidCreate(listener));
2425
this.push(watcher.onDidChange(listener));
2526
this.push(watcher.onDidDelete(listener));

extensions/ql-vscode/test/vscode-tests/minimal-workspace/common/vscode/file-path-discovery.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { EventEmitter, Uri, workspace } from "vscode";
77
import { FilePathDiscovery } from "../../../../../src/common/vscode/file-path-discovery";
88
import { basename, dirname, join } from "path";
99
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
10+
import { rm } from "fs/promises";
1011
import { dirSync } from "tmp";
1112
import { normalizePath } from "../../../../../src/common/files";
1213
import { extLogger } from "../../../../../src/common/logging/vscode/loggers";
@@ -60,7 +61,6 @@ class TestFilePathDiscovery extends FilePathDiscovery<TestData> {
6061

6162
describe("FilePathDiscovery", () => {
6263
let tmpDir: string;
63-
let tmpDirRemoveCallback: (() => void) | undefined;
6464

6565
let workspaceFolder: WorkspaceFolder;
6666
let workspacePath: string;
@@ -81,11 +81,7 @@ describe("FilePathDiscovery", () => {
8181
let discovery: TestFilePathDiscovery;
8282

8383
beforeEach(() => {
84-
const t = dirSync({
85-
unsafeCleanup: true,
86-
});
87-
tmpDir = normalizePath(t.name);
88-
tmpDirRemoveCallback = t.removeCallback;
84+
tmpDir = normalizePath(dirSync().name);
8985

9086
workspaceFolder = {
9187
uri: Uri.file(join(tmpDir, "workspace")),
@@ -117,9 +113,14 @@ describe("FilePathDiscovery", () => {
117113
discovery = new TestFilePathDiscovery();
118114
});
119115

120-
afterEach(() => {
121-
tmpDirRemoveCallback?.();
116+
afterEach(async () => {
122117
discovery.dispose();
118+
await rm(tmpDir, {
119+
recursive: true,
120+
force: true,
121+
maxRetries: 10,
122+
retryDelay: 200,
123+
});
123124
});
124125

125126
describe("initialRefresh", () => {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-discovery.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { QueryDiscovery } from "../../../../src/queries-panel/query-discovery";
44
import { createMockApp } from "../../../__mocks__/appMock";
55
import { basename, dirname, join } from "path";
66
import { dirSync } from "tmp";
7+
import { rm } from "fs/promises";
78
import {
89
FileTreeDirectory,
910
FileTreeLeaf,
@@ -15,7 +16,6 @@ import { LanguageContextStore } from "../../../../src/language-context-store";
1516

1617
describe("Query pack discovery", () => {
1718
let tmpDir: string;
18-
let tmpDirRemoveCallback: (() => void) | undefined;
1919

2020
let workspacePath: string;
2121

@@ -29,11 +29,7 @@ describe("Query pack discovery", () => {
2929
let discovery: QueryDiscovery;
3030

3131
beforeEach(() => {
32-
const t = dirSync({
33-
unsafeCleanup: true,
34-
});
35-
tmpDir = t.name;
36-
tmpDirRemoveCallback = t.removeCallback;
32+
tmpDir = dirSync().name;
3733

3834
const workspaceFolder = {
3935
uri: Uri.file(join(tmpDir, "workspace")),
@@ -52,9 +48,14 @@ describe("Query pack discovery", () => {
5248
discovery = new QueryDiscovery(app, queryPackDiscoverer, languageContext);
5349
});
5450

55-
afterEach(() => {
56-
tmpDirRemoveCallback?.();
51+
afterEach(async () => {
5752
discovery.dispose();
53+
await rm(tmpDir, {
54+
recursive: true,
55+
force: true,
56+
maxRetries: 10,
57+
retryDelay: 200,
58+
});
5859
});
5960

6061
describe("buildQueryTree", () => {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/queries-panel/query-pack-discovery.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,20 @@
11
import { Uri, workspace } from "vscode";
22
import { QueryPackDiscovery } from "../../../../src/queries-panel/query-pack-discovery";
33
import { dirSync } from "tmp";
4+
import { rm } from "fs/promises";
45
import { dirname, join } from "path";
56
import { ensureDir, writeJSON } from "fs-extra";
67
import { QueryLanguage } from "../../../../src/common/query-language";
78

89
describe("Query pack discovery", () => {
910
let tmpDir: string;
10-
let tmpDirRemoveCallback: (() => void) | undefined;
1111

1212
let workspacePath: string;
1313

1414
let discovery: QueryPackDiscovery;
1515

1616
beforeEach(() => {
17-
const t = dirSync({
18-
unsafeCleanup: true,
19-
});
20-
tmpDir = t.name;
21-
tmpDirRemoveCallback = t.removeCallback;
17+
tmpDir = dirSync().name;
2218

2319
const workspaceFolder = {
2420
uri: Uri.file(join(tmpDir, "workspace")),
@@ -33,9 +29,14 @@ describe("Query pack discovery", () => {
3329
discovery = new QueryPackDiscovery();
3430
});
3531

36-
afterEach(() => {
37-
tmpDirRemoveCallback?.();
32+
afterEach(async () => {
3833
discovery.dispose();
34+
await rm(tmpDir, {
35+
recursive: true,
36+
force: true,
37+
maxRetries: 10,
38+
retryDelay: 200,
39+
});
3940
});
4041

4142
describe("findQueryPack", () => {

0 commit comments

Comments
 (0)