From c27aa9c3701caf53d10f6253ce950ed4ccc199ae Mon Sep 17 00:00:00 2001 From: Smartsheet-JB-Brown Date: Wed, 16 Apr 2025 21:11:51 -0700 Subject: [PATCH] heap cleanup --- package.json | 3 +- src/core/__tests__/Cline.test.ts | 21 ++-- .../package-manager/PackageManagerManager.ts | 19 ++- .../__tests__/PackageManagerManager.test.ts | 119 +++++++++--------- 4 files changed, 91 insertions(+), 71 deletions(-) diff --git a/package.json b/package.json index f2fb0a50eb..d0f570b4fd 100644 --- a/package.json +++ b/package.json @@ -402,7 +402,8 @@ "pretest": "npm run compile", "dev": "cd webview-ui && npm run dev", "test": "node scripts/run-tests.js", - "test:extension": "node --max-old-space-size=8192 ./node_modules/.bin/jest -w=40%", + "test:extension": "node ./node_modules/.bin/jest -w=40% --detectOpenHandles --testTimeout=10000", + "test:extension:debug-memory": "node --max-old-space-size=8192 --trace-gc --expose-gc --heap-prof ./node_modules/.bin/jest --runInBand --logHeapUsage --detectOpenHandles --testTimeout=10000", "test:webview": "cd webview-ui && npm run test", "prepare": "husky", "publish:marketplace": "vsce publish && ovsx publish", diff --git a/src/core/__tests__/Cline.test.ts b/src/core/__tests__/Cline.test.ts index 90e365caf1..c7c06ac855 100644 --- a/src/core/__tests__/Cline.test.ts +++ b/src/core/__tests__/Cline.test.ts @@ -621,15 +621,22 @@ describe("Cline", () => { }, ] - clineWithImages.abandoned = true - await taskWithImages.catch(() => {}) + try { + clineWithImages.abandoned = true + await taskWithImages.catch(() => {}) - clineWithoutImages.abandoned = true - await taskWithoutImages.catch(() => {}) + clineWithoutImages.abandoned = true + await taskWithoutImages.catch(() => {}) - // Trigger API requests - await clineWithImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]) - await clineWithoutImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]) + // Trigger API requests + await Promise.all([ + clineWithImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]), + clineWithoutImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }]), + ]) + } finally { + // Clean up + await Promise.all([clineWithImages.abortTask(true), clineWithoutImages.abortTask(true)]) + } // Get the calls const imagesCalls = imagesSpy.mock.calls diff --git a/src/services/package-manager/PackageManagerManager.ts b/src/services/package-manager/PackageManagerManager.ts index 92b704bf50..186b2766d2 100644 --- a/src/services/package-manager/PackageManagerManager.ts +++ b/src/services/package-manager/PackageManagerManager.ts @@ -160,19 +160,26 @@ export class PackageManagerManager { const fetchPromise = this.gitFetcher.fetchRepository(url, forceRefresh, sourceName) // Create a timeout promise + let timeoutId: NodeJS.Timeout | undefined const timeoutPromise = new Promise((_, reject) => { - setTimeout(() => { + timeoutId = setTimeout(() => { reject(new Error(`Repository fetch timed out after 30 seconds: ${url}`)) }, 30000) // 30 second timeout }) - // Race the fetch against the timeout - const data = await Promise.race([fetchPromise, timeoutPromise]) + try { + // Race the fetch against the timeout + const result = await Promise.race([fetchPromise, timeoutPromise]) - // Cache the result - this.cache.set(url, { data, timestamp: Date.now() }) + // Cache the result + this.cache.set(url, { data: result, timestamp: Date.now() }) - return data + return result + } finally { + if (timeoutId) { + clearTimeout(timeoutId) + } + } } catch (error) { console.error(`PackageManagerManager: Error fetching repository data for ${url}:`, error) diff --git a/src/services/package-manager/__tests__/PackageManagerManager.test.ts b/src/services/package-manager/__tests__/PackageManagerManager.test.ts index c7af660af9..9c1b0e8575 100644 --- a/src/services/package-manager/__tests__/PackageManagerManager.test.ts +++ b/src/services/package-manager/__tests__/PackageManagerManager.test.ts @@ -610,16 +610,14 @@ describe("Concurrency Control", () => { enabled: true, } - // Mock getRepositoryData to be slow - const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) - const slowGetRepositoryData = jest.spyOn(manager as any, "getRepositoryData").mockImplementation(async () => { - await delay(100) // Simulate slow operation - return { + // Mock getRepositoryData to return a resolved promise immediately + const getRepoSpy = jest.spyOn(manager as any, "getRepositoryData").mockImplementation(() => + Promise.resolve({ metadata: { name: "test", description: "test", version: "1.0.0" }, items: [], url: source.url, - } as PackageManagerRepository - }) + } as PackageManagerRepository), + ) // Start two concurrent operations const operation1 = manager.getPackageManagerItems([source]) @@ -629,47 +627,53 @@ describe("Concurrency Control", () => { const [result1, result2] = await Promise.all([operation1, operation2]) // Verify getRepositoryData was only called once - expect(slowGetRepositoryData).toHaveBeenCalledTimes(1) + expect(getRepoSpy).toHaveBeenCalledTimes(1) + + // Clean up + getRepoSpy.mockRestore() }) it("should not allow metadata scanning during git operations", async () => { - const source1: PackageManagerSource = { - url: "https://github.com/test/repo1", - enabled: true, - } - const source2: PackageManagerSource = { - url: "https://github.com/test/repo2", - enabled: true, - } - - let isGitOperationActive = false - let metadataScanDuringGit = false - - // Mock git operation to be slow and set flag - jest.spyOn(GitFetcher.prototype, "fetchRepository").mockImplementation(async () => { - isGitOperationActive = true - await new Promise((resolve) => setTimeout(resolve, 100)) - isGitOperationActive = false - return { - metadata: { name: "test", description: "test", version: "1.0.0" }, - items: [], - url: source1.url, + try { + const source1: PackageManagerSource = { + url: "https://github.com/test/repo1", + enabled: true, } - }) - - // Mock metadata scanner to check if git operation is active - jest.spyOn(MetadataScanner.prototype, "scanDirectory").mockImplementation(async () => { - if (isGitOperationActive) { - metadataScanDuringGit = true + const source2: PackageManagerSource = { + url: "https://github.com/test/repo2", + enabled: true, } - return [] - }) - // Process both sources - await manager.getPackageManagerItems([source1, source2]) + let isGitOperationActive = false + let metadataScanDuringGit = false - // Verify metadata scanning didn't occur during git operations - expect(metadataScanDuringGit).toBe(false) + // Mock git operation to resolve immediately + const fetchRepoSpy = jest.spyOn(GitFetcher.prototype, "fetchRepository").mockImplementation(async () => { + isGitOperationActive = true + isGitOperationActive = false + return { + metadata: { name: "test", description: "test", version: "1.0.0" }, + items: [], + url: source1.url, + } + }) + + // Mock metadata scanner to check if git operation is active + const scanDirSpy = jest.spyOn(MetadataScanner.prototype, "scanDirectory").mockImplementation(async () => { + if (isGitOperationActive) { + metadataScanDuringGit = true + } + return [] + }) + + // Process both sources + await manager.getPackageManagerItems([source1, source2]) + + // Verify metadata scanning didn't occur during git operations + expect(metadataScanDuringGit).toBe(false) + } finally { + jest.clearAllTimers() + } }) it("should queue metadata scans and process them sequentially", async () => { @@ -681,18 +685,14 @@ describe("Concurrency Control", () => { let activeScans = 0 let maxConcurrentScans = 0 - const scanPromises: Promise[] = [] - // Create a mock MetadataScanner + // Create a mock MetadataScanner that resolves immediately const mockScanner = new MetadataScanner() const scanDirectorySpy = jest.spyOn(mockScanner, "scanDirectory").mockImplementation(async () => { activeScans++ maxConcurrentScans = Math.max(maxConcurrentScans, activeScans) - const promise = new Promise((resolve) => setTimeout(resolve, 50)) - scanPromises.push(promise) - await promise activeScans-- - return [] + return Promise.resolve([]) }) // Create a mock GitFetcher that uses our mock scanner @@ -704,26 +704,31 @@ describe("Concurrency Control", () => { ;(mockGitFetcher as any).metadataScanner = mockScanner // Mock GitFetcher's fetchRepository to trigger metadata scanning - jest.spyOn(mockGitFetcher, "fetchRepository").mockImplementation(async (repoUrl: string) => { - // Call scanDirectory through our mock scanner - await mockScanner.scanDirectory("/test/path", repoUrl) + const fetchRepoSpy = jest + .spyOn(mockGitFetcher, "fetchRepository") + .mockImplementation(async (repoUrl: string) => { + // Call scanDirectory through our mock scanner + await mockScanner.scanDirectory("/test/path", repoUrl) - return { - metadata: { name: "test", description: "test", version: "1.0.0" }, - items: [], - url: repoUrl, - } - }) + return Promise.resolve({ + metadata: { name: "test", description: "test", version: "1.0.0" }, + items: [], + url: repoUrl, + }) + }) // Replace the GitFetcher instance in the manager ;(manager as any).gitFetcher = mockGitFetcher // Process all sources await manager.getPackageManagerItems(sources) - await Promise.all(scanPromises) // Verify scans were called and only one was active at a time expect(scanDirectorySpy).toHaveBeenCalledTimes(sources.length) expect(maxConcurrentScans).toBe(1) + + // Clean up + scanDirectorySpy.mockRestore() + fetchRepoSpy.mockRestore() }) })