Merge pull request #4415 from github/cklin/clean-discovery-disposal

Fix flaky Windows tests: dispose file watchers and use retried cleanup

Author: cklin

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

@@ -20,6 +20,7 @@ class WatcherCollection extends DisposableObject {
    */
   public addWatcher(pattern: GlobPattern, listener: (e: Uri) => void): void {
     const watcher = workspace.createFileSystemWatcher(pattern);
+    this.push(watcher);
     this.push(watcher.onDidCreate(listener));
     this.push(watcher.onDidChange(listener));
     this.push(watcher.onDidDelete(listener));

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

@@ -7,6 +7,7 @@ import { EventEmitter, Uri, workspace } from "vscode";
 import { FilePathDiscovery } from "../../../../../src/common/vscode/file-path-discovery";
 import { basename, dirname, join } from "path";
 import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
+import { rm } from "fs/promises";
 import { dirSync } from "tmp";
 import { normalizePath } from "../../../../../src/common/files";
 import { extLogger } from "../../../../../src/common/logging/vscode/loggers";
@@ -60,7 +61,6 @@ class TestFilePathDiscovery extends FilePathDiscovery<TestData> {
 
 describe("FilePathDiscovery", () => {
   let tmpDir: string;
-  let tmpDirRemoveCallback: (() => void) | undefined;
 
   let workspaceFolder: WorkspaceFolder;
   let workspacePath: string;
@@ -81,11 +81,7 @@ describe("FilePathDiscovery", () => {
   let discovery: TestFilePathDiscovery;
 
   beforeEach(() => {
-    const t = dirSync({
-      unsafeCleanup: true,
-    });
-    tmpDir = normalizePath(t.name);
-    tmpDirRemoveCallback = t.removeCallback;
+    tmpDir = normalizePath(dirSync().name);
 
     workspaceFolder = {
       uri: Uri.file(join(tmpDir, "workspace")),
@@ -117,9 +113,14 @@ describe("FilePathDiscovery", () => {
     discovery = new TestFilePathDiscovery();
   });
 
-  afterEach(() => {
-    tmpDirRemoveCallback?.();
+  afterEach(async () => {
     discovery.dispose();
+    await rm(tmpDir, {
+      recursive: true,
+      force: true,
+      maxRetries: 10,
+      retryDelay: 200,
+    });
   });
 
   describe("initialRefresh", () => {

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

@@ -4,6 +4,7 @@ import { QueryDiscovery } from "../../../../src/queries-panel/query-discovery";
 import { createMockApp } from "../../../__mocks__/appMock";
 import { basename, dirname, join } from "path";
 import { dirSync } from "tmp";
+import { rm } from "fs/promises";
 import {
   FileTreeDirectory,
   FileTreeLeaf,
@@ -15,7 +16,6 @@ import { LanguageContextStore } from "../../../../src/language-context-store";
 
 describe("Query pack discovery", () => {
   let tmpDir: string;
-  let tmpDirRemoveCallback: (() => void) | undefined;
 
   let workspacePath: string;
 
@@ -29,11 +29,7 @@ describe("Query pack discovery", () => {
   let discovery: QueryDiscovery;
 
   beforeEach(() => {
-    const t = dirSync({
-      unsafeCleanup: true,
-    });
-    tmpDir = t.name;
-    tmpDirRemoveCallback = t.removeCallback;
+    tmpDir = dirSync().name;
 
     const workspaceFolder = {
       uri: Uri.file(join(tmpDir, "workspace")),
@@ -52,9 +48,14 @@ describe("Query pack discovery", () => {
     discovery = new QueryDiscovery(app, queryPackDiscoverer, languageContext);
   });
 
-  afterEach(() => {
-    tmpDirRemoveCallback?.();
+  afterEach(async () => {
     discovery.dispose();
+    await rm(tmpDir, {
+      recursive: true,
+      force: true,
+      maxRetries: 10,
+      retryDelay: 200,
+    });
   });
 
   describe("buildQueryTree", () => {

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

@@ -1,24 +1,20 @@
 import { Uri, workspace } from "vscode";
 import { QueryPackDiscovery } from "../../../../src/queries-panel/query-pack-discovery";
 import { dirSync } from "tmp";
+import { rm } from "fs/promises";
 import { dirname, join } from "path";
 import { ensureDir, writeJSON } from "fs-extra";
 import { QueryLanguage } from "../../../../src/common/query-language";
 
 describe("Query pack discovery", () => {
   let tmpDir: string;
-  let tmpDirRemoveCallback: (() => void) | undefined;
 
   let workspacePath: string;
 
   let discovery: QueryPackDiscovery;
 
   beforeEach(() => {
-    const t = dirSync({
-      unsafeCleanup: true,
-    });
-    tmpDir = t.name;
-    tmpDirRemoveCallback = t.removeCallback;
+    tmpDir = dirSync().name;
 
     const workspaceFolder = {
       uri: Uri.file(join(tmpDir, "workspace")),
@@ -33,9 +29,14 @@ describe("Query pack discovery", () => {
     discovery = new QueryPackDiscovery();
   });
 
-  afterEach(() => {
-    tmpDirRemoveCallback?.();
+  afterEach(async () => {
     discovery.dispose();
+    await rm(tmpDir, {
+      recursive: true,
+      force: true,
+      maxRetries: 10,
+      retryDelay: 200,
+    });
   });
 
   describe("findQueryPack", () => {