From f2d5c01970e4a93dc23e72f7d9655120f58f055c Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Thu, 30 Oct 2025 12:15:35 +0000 Subject: [PATCH] test: add cleanup to prevent memory leaks in test suites Add DeregisterAllServices() calls in afterEach hooks across all test files to clear singleton registry between tests. Export DeregisterAllServices from DependencyService. Fix StatusBarService test timing issues with animation frames. --- Services/DependencyService.ts | 4 + __tests__/AIClasses/Claude.test.ts | 11 ++- .../ClaudeConversationNamingService.test.ts | 6 +- __tests__/AIClasses/Gemini.test.ts | 11 ++- .../GeminiConversationNamingService.test.ts | 6 +- __tests__/AIClasses/OpenAI.test.ts | 11 ++- .../OpenAIConversationNamingService.test.ts | 6 +- __tests__/Services/AIFunctionService.test.ts | 4 +- __tests__/Services/ChatService.test.ts | 4 +- .../ConversationFileSystemService.test.ts | 4 +- .../ConversationNamingService.test.ts | 10 +- __tests__/Services/StatusBarService.test.ts | 98 +++++++++++++------ __tests__/Services/UserInputService.test.ts | 4 +- __tests__/Services/VaultCacheService.test.ts | 4 +- __tests__/Services/VaultService.test.ts | 4 +- 15 files changed, 128 insertions(+), 59 deletions(-) diff --git a/Services/DependencyService.ts b/Services/DependencyService.ts index 06092d1..8f6ae6d 100644 --- a/Services/DependencyService.ts +++ b/Services/DependencyService.ts @@ -21,4 +21,8 @@ export function Resolve(type: symbol): T { // It's a singleton, return the existing instance return service as T; +} + +export function DeregisterAllServices(): void { + services.clear(); } \ No newline at end of file diff --git a/__tests__/AIClasses/Claude.test.ts b/__tests__/AIClasses/Claude.test.ts index 76ea0a8..d0793d3 100644 --- a/__tests__/AIClasses/Claude.test.ts +++ b/__tests__/AIClasses/Claude.test.ts @@ -1,6 +1,6 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { Claude } from '../../AIClasses/Claude/Claude'; -import { RegisterSingleton, Resolve } from '../../Services/DependencyService'; +import { RegisterSingleton, Resolve, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { StreamingService } from '../../Services/StreamingService'; import type { IPrompt } from '../../AIClasses/IPrompt'; @@ -18,8 +18,6 @@ describe('Claude', () => { let mockFunctionDefinitions: any; beforeEach(() => { - - // Mock IPrompt mockPrompt = { systemInstruction: vi.fn().mockReturnValue('System instruction'), @@ -62,6 +60,11 @@ describe('Claude', () => { claude = new Claude(); }); + afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); + }); + describe('Constructor and Dependencies', () => { it('should initialize with dependencies from DependencyService', () => { expect(claude).toBeDefined(); diff --git a/__tests__/AIClasses/ClaudeConversationNamingService.test.ts b/__tests__/AIClasses/ClaudeConversationNamingService.test.ts index e3b9759..23ee0e6 100644 --- a/__tests__/AIClasses/ClaudeConversationNamingService.test.ts +++ b/__tests__/AIClasses/ClaudeConversationNamingService.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { ClaudeConversationNamingService } from '../../AIClasses/Claude/ClaudeConversationNamingService'; -import { RegisterSingleton, ClearAll } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { AIProviderModel } from '../../Enums/ApiProvider'; import { Role } from '../../Enums/Role'; @@ -11,8 +11,6 @@ describe('ClaudeConversationNamingService', () => { let fetchMock: any; beforeEach(() => { - - mockPlugin = { settings: { apiKey: 'test-claude-key' @@ -28,6 +26,8 @@ describe('ClaudeConversationNamingService', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/AIClasses/Gemini.test.ts b/__tests__/AIClasses/Gemini.test.ts index 3f94883..15db36d 100644 --- a/__tests__/AIClasses/Gemini.test.ts +++ b/__tests__/AIClasses/Gemini.test.ts @@ -1,6 +1,6 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { Gemini } from '../../AIClasses/Gemini/Gemini'; -import { RegisterSingleton, Resolve } from '../../Services/DependencyService'; +import { RegisterSingleton, Resolve, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { StreamingService } from '../../Services/StreamingService'; import type { IPrompt } from '../../AIClasses/IPrompt'; @@ -18,8 +18,6 @@ describe('Gemini', () => { let mockFunctionDefinitions: any; beforeEach(() => { - - // Mock IPrompt mockPrompt = { systemInstruction: vi.fn().mockReturnValue('System instruction'), @@ -62,6 +60,11 @@ describe('Gemini', () => { gemini = new Gemini(); }); + afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); + }); + describe('Constructor and Dependencies', () => { it('should initialize with dependencies from DependencyService', () => { expect(gemini).toBeDefined(); diff --git a/__tests__/AIClasses/GeminiConversationNamingService.test.ts b/__tests__/AIClasses/GeminiConversationNamingService.test.ts index ecd22f9..daa829f 100644 --- a/__tests__/AIClasses/GeminiConversationNamingService.test.ts +++ b/__tests__/AIClasses/GeminiConversationNamingService.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { GeminiConversationNamingService } from '../../AIClasses/Gemini/GeminiConversationNamingService'; -import { RegisterSingleton, ClearAll } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { AIProviderModel } from '../../Enums/ApiProvider'; import { Role } from '../../Enums/Role'; @@ -11,8 +11,6 @@ describe('GeminiConversationNamingService', () => { let fetchMock: any; beforeEach(() => { - - mockPlugin = { settings: { apiKey: 'test-gemini-key' @@ -28,6 +26,8 @@ describe('GeminiConversationNamingService', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/AIClasses/OpenAI.test.ts b/__tests__/AIClasses/OpenAI.test.ts index bb0eda9..1f70ea3 100644 --- a/__tests__/AIClasses/OpenAI.test.ts +++ b/__tests__/AIClasses/OpenAI.test.ts @@ -1,6 +1,6 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { OpenAI } from '../../AIClasses/OpenAI/OpenAI'; -import { RegisterSingleton, Resolve } from '../../Services/DependencyService'; +import { RegisterSingleton, Resolve, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { StreamingService } from '../../Services/StreamingService'; import type { IPrompt } from '../../AIClasses/IPrompt'; @@ -18,8 +18,6 @@ describe('OpenAI', () => { let mockFunctionDefinitions: any; beforeEach(() => { - - // Mock IPrompt mockPrompt = { systemInstruction: vi.fn().mockReturnValue('System instruction'), @@ -62,6 +60,11 @@ describe('OpenAI', () => { openai = new OpenAI(); }); + afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); + }); + describe('Constructor and Dependencies', () => { it('should initialize with dependencies from DependencyService', () => { expect(openai).toBeDefined(); diff --git a/__tests__/AIClasses/OpenAIConversationNamingService.test.ts b/__tests__/AIClasses/OpenAIConversationNamingService.test.ts index 5ab8a4a..0b0b2e4 100644 --- a/__tests__/AIClasses/OpenAIConversationNamingService.test.ts +++ b/__tests__/AIClasses/OpenAIConversationNamingService.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { OpenAIConversationNamingService } from '../../AIClasses/OpenAI/OpenAIConversationNamingService'; -import { RegisterSingleton, ClearAll } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { AIProviderModel } from '../../Enums/ApiProvider'; import { Role } from '../../Enums/Role'; @@ -11,8 +11,6 @@ describe('OpenAIConversationNamingService', () => { let fetchMock: any; beforeEach(() => { - - mockPlugin = { settings: { apiKey: 'test-openai-key' @@ -28,6 +26,8 @@ describe('OpenAIConversationNamingService', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/Services/AIFunctionService.test.ts b/__tests__/Services/AIFunctionService.test.ts index 691e307..86cea03 100644 --- a/__tests__/Services/AIFunctionService.test.ts +++ b/__tests__/Services/AIFunctionService.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { AIFunctionService } from '../../Services/AIFunctionService'; import { FileSystemService } from '../../Services/FileSystemService'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { AIFunction } from '../../Enums/AIFunction'; import { TFile, TFolder } from 'obsidian'; @@ -44,6 +44,8 @@ describe('AIFunctionService - Integration Tests', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/Services/ChatService.test.ts b/__tests__/Services/ChatService.test.ts index 6776530..dd49755 100644 --- a/__tests__/Services/ChatService.test.ts +++ b/__tests__/Services/ChatService.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ChatService } from '../../Services/ChatService'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { Conversation } from '../../Conversations/Conversation'; import { ConversationContent } from '../../Conversations/ConversationContent'; @@ -64,6 +64,8 @@ describe('ChatService - Integration Tests (Sync Methods Only)', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/Services/ConversationFileSystemService.test.ts b/__tests__/Services/ConversationFileSystemService.test.ts index 21e3654..fcca84a 100644 --- a/__tests__/Services/ConversationFileSystemService.test.ts +++ b/__tests__/Services/ConversationFileSystemService.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { ConversationFileSystemService } from '../../Services/ConversationFileSystemService'; import { FileSystemService } from '../../Services/FileSystemService'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { Conversation } from '../../Conversations/Conversation'; import { ConversationContent } from '../../Conversations/ConversationContent'; @@ -45,6 +45,8 @@ describe('ConversationFileSystemService - Integration Tests', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/Services/ConversationNamingService.test.ts b/__tests__/Services/ConversationNamingService.test.ts index 6ce172b..a262064 100644 --- a/__tests__/Services/ConversationNamingService.test.ts +++ b/__tests__/Services/ConversationNamingService.test.ts @@ -1,6 +1,6 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { ConversationNamingService } from '../../Services/ConversationNamingService'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { Conversation } from '../../Conversations/Conversation'; import { Path } from '../../Enums/Path'; @@ -12,7 +12,6 @@ describe('ConversationNamingService', () => { let mockVaultService: any; beforeEach(() => { - // Mock IConversationNamingService mockNamingProvider = { generateName: vi.fn().mockResolvedValue('Generated Title') @@ -35,6 +34,11 @@ describe('ConversationNamingService', () => { service = new ConversationNamingService(); }); + afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); + }); + describe('Constructor and Dependencies', () => { it('should initialize with dependencies', () => { expect(service).toBeDefined(); diff --git a/__tests__/Services/StatusBarService.test.ts b/__tests__/Services/StatusBarService.test.ts index 3efbffa..7aa6cfd 100644 --- a/__tests__/Services/StatusBarService.test.ts +++ b/__tests__/Services/StatusBarService.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { StatusBarService } from '../../Services/StatusBarService'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; describe('StatusBarService', () => { @@ -49,6 +49,16 @@ describe('StatusBarService', () => { }); afterEach(() => { + // Clean up service resources (cancel animations, remove DOM elements) + service.removeStatusBarMessage(); + + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); + + // Clear RAF callback references + rafCallbacks = []; + + // Restore all mocks vi.restoreAllMocks(); }); @@ -77,7 +87,8 @@ describe('StatusBarService', () => { // Should only create once expect(mockPlugin.addStatusBarItem).toHaveBeenCalledTimes(1); - expect(mockStatusBarItem.empty).toHaveBeenCalledTimes(2); + // empty() is called: createStatusBarMessage(1) + setStatusBarMessage first call(2) + second call(3) = 3 times + expect(mockStatusBarItem.empty).toHaveBeenCalledTimes(3); expect(mockStatusBarItem.createEl).toHaveBeenCalledTimes(2); }); @@ -156,19 +167,25 @@ describe('StatusBarService', () => { it('should complete animation after 1000ms', () => { service.animateTokens(100, 200); - // Advance through animation + // Simulate animation frames until completion currentTime = 0; - let frameIndex = 0; - while (frameIndex < rafCallbacks.length && currentTime <= 1000) { - rafCallbacks[frameIndex](currentTime); - frameIndex++; + let iterations = 0; + const maxIterations = 100; // Safety limit + + while (currentTime < 1000 && iterations < maxIterations) { + const callbackIndex = rafCallbacks.length - 1; // Get most recent callback + if (callbackIndex >= 0) { + rafCallbacks[callbackIndex](currentTime); + } currentTime += 16; // ~60fps + iterations++; } - // Run final frame at exactly 1000ms + // Run final frame at exactly 1000ms to ensure completion currentTime = 1000; - if (frameIndex < rafCallbacks.length) { - rafCallbacks[frameIndex](currentTime); + const finalCallbackIndex = rafCallbacks.length - 1; + if (finalCallbackIndex >= 0) { + rafCallbacks[finalCallbackIndex](currentTime); } // Should reach target values @@ -272,15 +289,11 @@ describe('StatusBarService', () => { // Should cancel previous animations expect(global.cancelAnimationFrame).toHaveBeenCalled(); - // Should have latest animation running + // Run the latest animation to completion currentTime = 1000; - const lastIndex = rafCallbacks.length - 1; - let frameIndex = 0; - while (frameIndex <= lastIndex && currentTime >= 0) { - if (rafCallbacks[frameIndex]) { - rafCallbacks[frameIndex](currentTime); - } - frameIndex++; + const latestCallback = rafCallbacks[rafCallbacks.length - 1]; + if (latestCallback) { + latestCallback(currentTime); } const lastCall = mockStatusBarItem.createEl.mock.calls[mockStatusBarItem.createEl.mock.calls.length - 1]; @@ -289,24 +302,51 @@ describe('StatusBarService', () => { }); it('should maintain state between animations', () => { - // First animation + // First animation - run to completion service.animateTokens(100, 200); - currentTime = 1000; - let frameIndex = 0; - while (frameIndex < rafCallbacks.length) { - rafCallbacks[frameIndex](currentTime); - frameIndex++; + + // Simulate animation to completion + currentTime = 0; + const startTime = currentTime; + let iterations = 0; + while (currentTime - startTime < 1000 && iterations < 100) { + const callbackIndex = rafCallbacks.length - 1; + if (callbackIndex >= 0) { + rafCallbacks[callbackIndex](currentTime); + } + currentTime += 16; + iterations++; + } + + // Final frame at exactly 1000ms + currentTime = startTime + 1000; + const finalIndex = rafCallbacks.length - 1; + if (finalIndex >= 0) { + rafCallbacks[finalIndex](currentTime); } // Second animation should start from previous end values rafCallbacks = []; // Reset RAF tracking service.animateTokens(200, 400); - currentTime = 1000; - frameIndex = 0; - while (frameIndex < rafCallbacks.length) { - rafCallbacks[frameIndex](currentTime); - frameIndex++; + // Simulate second animation to completion + currentTime = performance.now(); + const secondStartTime = currentTime; + iterations = 0; + while (currentTime - secondStartTime < 1000 && iterations < 100) { + const callbackIndex = rafCallbacks.length - 1; + if (callbackIndex >= 0) { + rafCallbacks[callbackIndex](currentTime); + } + currentTime += 16; + iterations++; + } + + // Final frame at exactly 1000ms + currentTime = secondStartTime + 1000; + const secondFinalIndex = rafCallbacks.length - 1; + if (secondFinalIndex >= 0) { + rafCallbacks[secondFinalIndex](currentTime); } const lastCall = mockStatusBarItem.createEl.mock.calls[mockStatusBarItem.createEl.mock.calls.length - 1]; diff --git a/__tests__/Services/UserInputService.test.ts b/__tests__/Services/UserInputService.test.ts index 00a89df..0f02322 100644 --- a/__tests__/Services/UserInputService.test.ts +++ b/__tests__/Services/UserInputService.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { UserInputService } from '../../Services/UserInputService'; import { SearchTrigger } from '../../Enums/SearchTrigger'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { writable, get } from 'svelte/store'; import type { ISearchState } from '../../Stores/SearchStateStore'; @@ -53,6 +53,8 @@ describe('UserInputService', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); vi.restoreAllMocks(); }); diff --git a/__tests__/Services/VaultCacheService.test.ts b/__tests__/Services/VaultCacheService.test.ts index 76e0fc7..bd768ad 100644 --- a/__tests__/Services/VaultCacheService.test.ts +++ b/__tests__/Services/VaultCacheService.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { VaultCacheService } from '../../Services/VaultCacheService'; import { VaultService } from '../../Services/VaultService'; import { TFile, TFolder, MetadataCache } from 'obsidian'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { FileEvent } from '../../Enums/FileEvent'; @@ -108,6 +108,8 @@ describe('VaultCacheService - Integration Tests', () => { let fileEventHandler: any; beforeEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); // Reset all mocks vi.clearAllMocks(); diff --git a/__tests__/Services/VaultService.test.ts b/__tests__/Services/VaultService.test.ts index f83a89d..33dddfa 100644 --- a/__tests__/Services/VaultService.test.ts +++ b/__tests__/Services/VaultService.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { VaultService } from '../../Services/VaultService'; import { TFile, TFolder, TAbstractFile, FileManager } from 'obsidian'; import { Path } from '../../Enums/Path'; -import { RegisterSingleton } from '../../Services/DependencyService'; +import { RegisterSingleton, DeregisterAllServices } from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; import { SanitiserService } from '../../Services/SanitiserService'; @@ -99,6 +99,8 @@ describe('VaultService - Integration Tests', () => { }); afterEach(() => { + // Clear singleton registry to prevent memory leaks + DeregisterAllServices(); consoleErrorSpy.mockRestore(); });