From 75a8de83fc3492baada7c0722793474b4255c325 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Mon, 4 May 2026 15:08:02 -0400 Subject: [PATCH] test(cleanup): fix temporary directory leaks in test suites (#26217) --- .../src/commands/extensions/configure.test.ts | 10 ++++++-- .../config/extension-manager-scope.test.ts | 6 +++-- packages/core/src/tools/mcp-client.test.ts | 22 ++++++++++++++---- .../src/file-system-test-helpers.ts | 23 +++++++++++++++++-- 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/extensions/configure.test.ts b/packages/cli/src/commands/extensions/configure.test.ts index cf86d6cc71..dffd3fee37 100644 --- a/packages/cli/src/commands/extensions/configure.test.ts +++ b/packages/cli/src/commands/extensions/configure.test.ts @@ -20,8 +20,11 @@ import { getScopedEnvContents, type ExtensionSetting, } from '../../config/extensions/extensionSettings.js'; +import { cleanupTmpDir } from '@google/gemini-cli-test-utils'; import prompts from 'prompts'; import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; const { mockExtensionManager, mockGetExtensionManager, mockLoadSettings } = vi.hoisted(() => { @@ -84,7 +87,9 @@ describe('extensions configure command', () => { vi.spyOn(debugLogger, 'error'); vi.clearAllMocks(); - tempWorkspaceDir = fs.mkdtempSync('gemini-cli-test-workspace'); + tempWorkspaceDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-workspace-'), + ); vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir); // Default behaviors mockLoadSettings.mockReturnValue({ merged: {} }); @@ -94,7 +99,8 @@ describe('extensions configure command', () => { ); }); - afterEach(() => { + afterEach(async () => { + await cleanupTmpDir(tempWorkspaceDir); vi.restoreAllMocks(); }); diff --git a/packages/cli/src/config/extension-manager-scope.test.ts b/packages/cli/src/config/extension-manager-scope.test.ts index f88673e692..5e93face28 100644 --- a/packages/cli/src/config/extension-manager-scope.test.ts +++ b/packages/cli/src/config/extension-manager-scope.test.ts @@ -10,6 +10,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { ExtensionManager } from './extension-manager.js'; import { createTestMergedSettings } from './settings.js'; +import { cleanupTmpDir } from '@google/gemini-cli-test-utils'; import { loadAgentsFromDirectory, loadSkillsFromDir, @@ -87,8 +88,9 @@ describe('ExtensionManager Settings Scope', () => { ); }); - afterEach(() => { - // Clean up files if needed, or rely on temp dir cleanup + afterEach(async () => { + await cleanupTmpDir(currentTempHome); + await cleanupTmpDir(tempWorkspace); vi.clearAllMocks(); }); diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 50b17aa735..fdfbbd23de 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -45,6 +45,7 @@ import type { ResourceRegistry } from '../resources/resource-registry.js'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { cleanupTmpDir } from '@google/gemini-cli-test-utils'; import { coreEvents } from '../utils/events.js'; import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; @@ -105,9 +106,11 @@ describe('mcp-client', () => { workspaceContext = new WorkspaceContext(testWorkspace); }); - afterEach(() => { - vi.restoreAllMocks(); + afterEach(async () => { vi.useRealTimers(); + await cleanupTmpDir(testWorkspace); + workspaceContext = null as unknown as WorkspaceContext; + vi.restoreAllMocks(); }); describe('McpClient', () => { @@ -2410,7 +2413,10 @@ describe('connectToMcpServer with OAuth', () => { vi.mocked(MCPOAuthProvider).mockReturnValue(mockAuthProvider); }); - afterEach(() => { + afterEach(async () => { + vi.useRealTimers(); + await cleanupTmpDir(testWorkspace); + workspaceContext = null as unknown as WorkspaceContext; vi.clearAllMocks(); }); @@ -2617,7 +2623,10 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { vi.spyOn(console, 'error').mockImplementation(() => {}); }); - afterEach(() => { + afterEach(async () => { + vi.useRealTimers(); + await cleanupTmpDir(testWorkspace); + workspaceContext = null as unknown as WorkspaceContext; vi.clearAllMocks(); }); @@ -2780,7 +2789,10 @@ describe('connectToMcpServer - OAuth with transport fallback', () => { }); }); - afterEach(() => { + afterEach(async () => { + vi.useRealTimers(); + await cleanupTmpDir(testWorkspace); + workspaceContext = null as unknown as WorkspaceContext; vi.clearAllMocks(); vi.unstubAllGlobals(); }); diff --git a/packages/test-utils/src/file-system-test-helpers.ts b/packages/test-utils/src/file-system-test-helpers.ts index 43ce6a5d1b..ba1778d88c 100644 --- a/packages/test-utils/src/file-system-test-helpers.ts +++ b/packages/test-utils/src/file-system-test-helpers.ts @@ -93,6 +93,25 @@ export async function createTmpDir( * Cleans up (deletes) a temporary directory and its contents. * @param dir The absolute path to the temporary directory to clean up. */ -export async function cleanupTmpDir(dir: string) { - await fs.rm(dir, { recursive: true, force: true }); +export async function cleanupTmpDir(dir: string | undefined) { + if (!dir) { + return; + } + + try { + const exists = await fs + .access(dir) + .then(() => true) + .catch(() => false); + + if (exists) { + if (process.platform === 'win32') { + // Give Windows a moment to release file handles + await new Promise((resolve) => setTimeout(resolve, 100)); + } + await fs.rm(dir, { recursive: true, force: true }); + } + } catch { + // Ignore errors during cleanup (e.g., directory already deleted) + } }