-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Description
🧹 Test Infrastructure Issue
File: src/lib/analytics.test.ts
Severity: LOW
Origin: Copilot review comment in PR #100
Problem
The OfflineAnalytics singleton is created at module load time and persists across all test runs:
// In analytics.ts
let analyticsInstance: OfflineAnalytics | null = null;
try {
analyticsInstance = new OfflineAnalytics(); // ← Created once at module load
} catch (error) {
console.error("Failed to initialize analytics singleton:", error);
}Issue: The singleton:
- Registers global event listeners (
window.addEventListener('online', ...)) - Starts periodic sync interval (
setInterval) - Persists across test runs with active listeners/intervals
- Can cause test interference and memory leaks
Expected Behavior
Either:
- Tests should clean up the singleton after each test, OR
- Document that singleton persistence is intentional for production
Proposed Solution
Option A: Add cleanup in tests (Recommended)
// In analytics.test.ts
afterEach(() => {
vi.restoreAllMocks();
// Clean up singleton to prevent memory leaks and test interference
if (analytics && typeof analytics.destroy === "function") {
analytics.destroy();
}
});Option B: Document intentional behavior
If singleton persistence is intentional for production (event listeners should live for entire app lifetime), add comment:
// NOTE: Singleton persists across tests by design
// Event listeners remain active for app lifetime in production
// Test isolation is not affected as long as vi.clearAllMocks() is usedContext
- Related to PR feat: Implement PWA Phase 3 features (Push Notifications, Share Target API, Offline Analytics) #100 (PWA Phase 3 implementation)
- Copilot comment: "Potential memory leak: analytics singleton never cleaned up between tests"
- Not causing test failures currently, but good practice to address
Acceptance Criteria
- Choose Option A (cleanup) or Option B (document)
- If Option A: Add
afterEachcleanup hook - If Option A: Verify all tests still pass after cleanup
- If Option B: Add explanatory comment in both analytics.ts and analytics.test.ts
- Consider refactoring to factory pattern if cleanup becomes complex
Metadata
Metadata
Assignees
Type
Projects
Status
✅ Done