From bfeb94d972d4fde520efa8167edfff4e42ced96f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 29 Jul 2025 08:08:15 +0200 Subject: [PATCH] test fixes --- .../components/terminal-quick-keys.test.ts | 2 +- .../push-notification-service.test.ts | 117 ++++++++++-------- .../server/routes/events.claude-turn.test.ts | 59 +++++---- web/src/server/routes/events.test.ts | 53 ++++---- .../socket-protocol-integration.test.ts | 2 +- 5 files changed, 128 insertions(+), 105 deletions(-) diff --git a/web/src/client/components/terminal-quick-keys.test.ts b/web/src/client/components/terminal-quick-keys.test.ts index 343cbef9..bcee5417 100644 --- a/web/src/client/components/terminal-quick-keys.test.ts +++ b/web/src/client/components/terminal-quick-keys.test.ts @@ -95,7 +95,7 @@ describe('TerminalQuickKeys', () => { // Should have sent only the 'a' key expect(mockOnKeyPress).toHaveBeenCalledOnce(); - expect(mockOnKeyPress).toHaveBeenCalledWith('a', false, false); + expect(mockOnKeyPress).toHaveBeenCalledWith('a', false, false, false); }); it('should handle multiple Option+Arrow sequences', () => { diff --git a/web/src/client/services/push-notification-service.test.ts b/web/src/client/services/push-notification-service.test.ts index 4bf5775f..0ceae0bb 100644 --- a/web/src/client/services/push-notification-service.test.ts +++ b/web/src/client/services/push-notification-service.test.ts @@ -2,6 +2,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DEFAULT_NOTIFICATION_PREFERENCES } from '../../types/config.js'; import type { NotificationPreferences } from './push-notification-service.js'; import { pushNotificationService } from './push-notification-service.js'; +import { notificationEventService } from './notification-event-service.js'; +import { serverConfigService } from './server-config-service.js'; + +// Mock notification event service +vi.mock('./notification-event-service', () => ({ + notificationEventService: { + on: vi.fn(), + off: vi.fn(), + connect: vi.fn(), + disconnect: vi.fn(), + getConnectionStatus: vi.fn(() => true), + }, +})); + +// Mock server config service +vi.mock('./server-config-service', () => ({ + serverConfigService: { + getNotificationPreferences: vi.fn(), + updateNotificationPreferences: vi.fn(), + }, +})); // Mock the auth client vi.mock('./auth-client', () => ({ @@ -303,33 +324,7 @@ describe('PushNotificationService', () => { soundEnabled: true, vibrationEnabled: false, }; - - // Mock the config API response - global.fetch = vi.fn().mockImplementation((url: string) => { - if (url === '/api/push/vapid-public-key') { - return Promise.resolve({ - ok: true, - json: () => - Promise.resolve({ - publicKey: 'test-vapid-key', - enabled: true, - }), - }); - } - if (url === '/api/config') { - return Promise.resolve({ - ok: true, - json: () => - Promise.resolve({ - notificationPreferences: serverPrefs, - }), - }); - } - return Promise.resolve({ - ok: true, - json: () => Promise.resolve({}), - }); - }); + (serverConfigService.getNotificationPreferences as vi.Mock).mockResolvedValue(serverPrefs); await pushNotificationService.initialize(); @@ -501,27 +496,9 @@ describe('PushNotificationService', () => { vibrationEnabled: true, }; - global.fetch = vi.fn().mockResolvedValue({ - ok: true, - json: () => - Promise.resolve({ - notificationPreferences: preferences, - }), - }); - await pushNotificationService.savePreferences(preferences); - // Since we're now using serverConfigService, we expect a config API call - expect(fetch).toHaveBeenCalledWith( - '/api/config', - expect.objectContaining({ - method: 'PUT', - headers: expect.objectContaining({ - 'Content-Type': 'application/json', - }), - body: expect.stringContaining('notificationPreferences'), - }) - ); + expect(serverConfigService.updateNotificationPreferences).toHaveBeenCalledWith(preferences); }); it('should handle save failure gracefully', async () => { @@ -537,9 +514,8 @@ describe('PushNotificationService', () => { vibrationEnabled: true, }; - global.fetch = vi.fn().mockRejectedValue(new Error('Network error')); + (serverConfigService.updateNotificationPreferences as vi.Mock).mockRejectedValue(new Error('Network error')); - // Should throw error now since we don't fallback to localStorage await expect(pushNotificationService.savePreferences(preferences)).rejects.toThrow( 'Network error' ); @@ -555,7 +531,27 @@ describe('PushNotificationService', () => { await pushNotificationService.initialize(); - await pushNotificationService.testNotification(); + // Capture the event handler + let testNotificationHandler: (data: any) => void; + (notificationEventService.on as vi.Mock).mockImplementation((event, handler) => { + if (event === 'test-notification') { + testNotificationHandler = handler; + } + return () => {}; // Return an unsubscribe function + }); + + const testPromise = pushNotificationService.testNotification(); + + // Simulate receiving the event + await new Promise(resolve => setTimeout(resolve, 100)); // allow time for listener to be registered + + expect(testNotificationHandler!).toBeDefined(); + testNotificationHandler!({ + title: 'VibeTunnel Test', + body: 'Push notifications are working correctly!', + }); + + await testPromise; expect(mockServiceWorkerRegistration.showNotification).toHaveBeenCalledWith( 'VibeTunnel Test', @@ -563,7 +559,7 @@ describe('PushNotificationService', () => { body: 'Push notifications are working correctly!', icon: '/apple-touch-icon.png', badge: '/favicon-32.png', - tag: 'vibetunnel-test', + tag: 'vibetunnel-test-sse', }) ); }); @@ -576,9 +572,24 @@ describe('PushNotificationService', () => { await pushNotificationService.initialize(); - await expect(pushNotificationService.testNotification()).rejects.toThrow( - 'Notification permission not granted' - ); + // Capture the event handler + let testNotificationHandler: (data: any) => void; + (notificationEventService.on as vi.Mock).mockImplementation((event, handler) => { + if (event === 'test-notification') { + testNotificationHandler = handler; + } + return () => {}; // Return an unsubscribe function + }); + + const testPromise = pushNotificationService.testNotification(); + + // Simulate receiving the event + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(testNotificationHandler!).toBeDefined(); + testNotificationHandler!({}); + + await testPromise; expect(mockServiceWorkerRegistration.showNotification).not.toHaveBeenCalled(); }); diff --git a/web/src/server/routes/events.claude-turn.test.ts b/web/src/server/routes/events.claude-turn.test.ts index 6ec70f51..087b1ec6 100644 --- a/web/src/server/routes/events.claude-turn.test.ts +++ b/web/src/server/routes/events.claude-turn.test.ts @@ -16,7 +16,7 @@ vi.mock('../utils/logger', () => ({ })); describe('Claude Turn Events', () => { - let mockPtyManager: PtyManager & EventEmitter; + let mockSessionMonitor: SessionMonitor & EventEmitter; let mockRequest: Partial & { headers: Record; on: ReturnType; @@ -26,8 +26,8 @@ describe('Claude Turn Events', () => { let eventHandler: (req: Request, res: Response) => void; beforeEach(() => { - // Create a mock PtyManager that extends EventEmitter - mockPtyManager = new EventEmitter() as PtyManager & EventEmitter; + // Create a mock SessionMonitor that extends EventEmitter + mockSessionMonitor = new EventEmitter() as SessionMonitor & EventEmitter; // Create mock request mockRequest = { @@ -43,7 +43,7 @@ describe('Claude Turn Events', () => { } as unknown as Response; // Create router - eventsRouter = createEventsRouter(mockPtyManager); + eventsRouter = createEventsRouter(mockSessionMonitor); // Get the /events handler interface RouteLayer { @@ -75,7 +75,12 @@ describe('Claude Turn Events', () => { // Emit claude-turn event const sessionId = 'claude-session-123'; const sessionName = 'Claude Code Session'; - mockPtyManager.emit('claudeTurn', sessionId, sessionName); + mockSessionMonitor.emit('notification', { + type: 'claude-turn', + sessionId, + sessionName, + message: 'Claude has finished responding', + }); // Verify SSE was sent expect(mockResponse.write).toHaveBeenCalledWith( @@ -97,9 +102,9 @@ describe('Claude Turn Events', () => { vi.clearAllMocks(); // Emit multiple claude-turn events - mockPtyManager.emit('claudeTurn', 'session-1', 'First Claude Session'); - mockPtyManager.emit('claudeTurn', 'session-2', 'Second Claude Session'); - mockPtyManager.emit('claudeTurn', 'session-3', 'Third Claude Session'); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'session-1' }); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'session-2' }); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'session-3' }); // Should have written 3 events const writeCalls = (mockResponse.write as ReturnType).mock.calls; @@ -111,9 +116,13 @@ describe('Claude Turn Events', () => { await eventHandler(mockRequest, mockResponse); vi.clearAllMocks(); - const beforeTime = new Date().toISOString(); - mockPtyManager.emit('claudeTurn', 'test-session', 'Test Session'); - const afterTime = new Date().toISOString(); + const beforeTime = new Date(); + mockSessionMonitor.emit('notification', { + type: 'claude-turn', + sessionId: 'test-session', + timestamp: new Date().toISOString(), + }); + const afterTime = new Date(); // Get the event data const writeCall = (mockResponse.write as ReturnType).mock.calls[0][0]; @@ -121,8 +130,8 @@ describe('Claude Turn Events', () => { expect(eventData.timestamp).toBeDefined(); expect(new Date(eventData.timestamp).toISOString()).toEqual(eventData.timestamp); - expect(eventData.timestamp >= beforeTime).toBe(true); - expect(eventData.timestamp <= afterTime).toBe(true); + expect(new Date(eventData.timestamp).getTime()).toBeGreaterThanOrEqual(beforeTime.getTime()); + expect(new Date(eventData.timestamp).getTime()).toBeLessThanOrEqual(afterTime.getTime()); }); it('should unsubscribe from claude-turn events on disconnect', async () => { @@ -135,13 +144,13 @@ describe('Claude Turn Events', () => { expect(closeHandler).toBeTruthy(); // Verify claude-turn listener is attached - expect(mockPtyManager.listenerCount('claudeTurn')).toBe(1); + expect(mockSessionMonitor.listenerCount('notification')).toBe(1); // Simulate client disconnect closeHandler(); // Verify listener is removed - expect(mockPtyManager.listenerCount('claudeTurn')).toBe(0); + expect(mockSessionMonitor.listenerCount('notification')).toBe(0); }); it('should handle claude-turn alongside other events', async () => { @@ -149,15 +158,13 @@ describe('Claude Turn Events', () => { vi.clearAllMocks(); // Emit various events including claude-turn - mockPtyManager.emit('sessionStarted', 'session-1', 'New Session'); - mockPtyManager.emit('claudeTurn', 'session-1', 'New Session'); - mockPtyManager.emit('commandFinished', { + mockSessionMonitor.emit('notification', { type: 'session-start', sessionId: 'session-1' }); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'session-1' }); + mockSessionMonitor.emit('notification', { + type: 'command-finished', sessionId: 'session-1', - command: 'echo test', - duration: 100, - exitCode: 0, }); - mockPtyManager.emit('sessionExited', 'session-1', 'New Session', 0); + mockSessionMonitor.emit('notification', { type: 'session-exit', sessionId: 'session-1' }); // Verify all events were sent const writeCalls = (mockResponse.write as ReturnType).mock.calls; @@ -180,7 +187,13 @@ describe('Claude Turn Events', () => { await eventHandler(mockRequest, mockResponse); vi.clearAllMocks(); - mockPtyManager.emit('claudeTurn', 'session-123', 'My Claude Session'); + mockSessionMonitor.emit('notification', { + type: 'claude-turn', + sessionId: 'session-123', + sessionName: 'My Claude Session', + message: 'Claude has finished responding', + timestamp: new Date().toISOString(), + }); const writeCall = (mockResponse.write as ReturnType).mock.calls[0][0]; diff --git a/web/src/server/routes/events.test.ts b/web/src/server/routes/events.test.ts index 95cebe55..4f4b9e76 100644 --- a/web/src/server/routes/events.test.ts +++ b/web/src/server/routes/events.test.ts @@ -1,7 +1,7 @@ import { EventEmitter } from 'events'; import type { Response } from 'express'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { PtyManager } from '../pty/index.js'; +import type { SessionMonitor } from '../services/session-monitor.js'; import { createEventsRouter } from './events.js'; // Mock dependencies @@ -27,7 +27,7 @@ interface RouteLayer { type ExpressRouter = { stack: RouteLayer[] }; describe('Events Router', () => { - let mockPtyManager: PtyManager & EventEmitter; + let mockSessionMonitor: SessionMonitor & EventEmitter; let mockRequest: Partial & { headers: Record; on: ReturnType; @@ -36,8 +36,8 @@ describe('Events Router', () => { let eventsRouter: ReturnType; beforeEach(() => { - // Create a mock PtyManager that extends EventEmitter - mockPtyManager = new EventEmitter() as PtyManager & EventEmitter; + // Create a mock SessionMonitor that extends EventEmitter + mockSessionMonitor = new EventEmitter() as SessionMonitor & EventEmitter; // Create mock request mockRequest = { @@ -57,7 +57,7 @@ describe('Events Router', () => { } as unknown as Response; // Create router - eventsRouter = createEventsRouter(mockPtyManager); + eventsRouter = createEventsRouter(mockSessionMonitor); }); afterEach(() => { @@ -120,16 +120,13 @@ describe('Events Router', () => { // Emit a sessionExit event const eventData = { + type: 'session-exit', sessionId: 'test-123', sessionName: 'Test Session', exitCode: 0, + timestamp: new Date().toISOString(), }; - mockPtyManager.emit( - 'sessionExited', - eventData.sessionId, - eventData.sessionName, - eventData.exitCode - ); + mockSessionMonitor.emit('notification', eventData); // Verify SSE was sent - check that the data contains our expected fields const writeCall = (mockResponse.write as ReturnType).mock.calls[0][0]; @@ -165,12 +162,14 @@ describe('Events Router', () => { // Emit a commandFinished event const eventData = { + type: 'command-finished', sessionId: 'test-123', command: 'npm test', exitCode: 0, duration: 5432, + timestamp: new Date().toISOString(), }; - mockPtyManager.emit('commandFinished', eventData); + mockSessionMonitor.emit('notification', eventData); // Verify SSE was sent - check that the data contains our expected fields const writeCall = (mockResponse.write as ReturnType).mock.calls[0][0]; @@ -206,9 +205,13 @@ describe('Events Router', () => { vi.clearAllMocks(); // Emit multiple events - mockPtyManager.emit('sessionExited', 'session-1'); - mockPtyManager.emit('commandFinished', { sessionId: 'session-2', command: 'ls' }); - mockPtyManager.emit('claudeTurn', 'session-3', 'Session 3'); + mockSessionMonitor.emit('notification', { type: 'session-exit', sessionId: 'session-1' }); + mockSessionMonitor.emit('notification', { + type: 'command-finished', + sessionId: 'session-2', + command: 'ls', + }); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'session-3' }); // Should have written 3 events const writeCalls = (mockResponse.write as ReturnType).mock.calls; @@ -255,17 +258,13 @@ describe('Events Router', () => { expect(closeHandler).toBeTruthy(); // Verify listeners are attached - expect(mockPtyManager.listenerCount('sessionExited')).toBeGreaterThan(0); - expect(mockPtyManager.listenerCount('commandFinished')).toBeGreaterThan(0); - expect(mockPtyManager.listenerCount('claudeTurn')).toBeGreaterThan(0); + expect(mockSessionMonitor.listenerCount('notification')).toBeGreaterThan(0); // Simulate client disconnect closeHandler(); // Verify listeners are removed - expect(mockPtyManager.listenerCount('sessionExited')).toBe(0); - expect(mockPtyManager.listenerCount('commandFinished')).toBe(0); - expect(mockPtyManager.listenerCount('claudeTurn')).toBe(0); + expect(mockSessionMonitor.listenerCount('notification')).toBe(0); }); it('should handle response errors gracefully', async () => { @@ -287,7 +286,7 @@ describe('Events Router', () => { // Should not throw even if write fails expect(() => { - mockPtyManager.emit('claudeTurn', 'test', 'Test Session'); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'test' }); }).not.toThrow(); }); @@ -304,7 +303,7 @@ describe('Events Router', () => { vi.clearAllMocks(); // Emit an event - mockPtyManager.emit('claudeTurn', 'test-123', 'Test Session'); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'test-123' }); // Verify SSE format includes id const writeCalls = (mockResponse.write as ReturnType).mock.calls; @@ -337,11 +336,11 @@ describe('Events Router', () => { // Should not throw expect(() => { - mockPtyManager.emit('claudeTurn', 'test-123', 'Test Session'); + mockSessionMonitor.emit('notification', circularData); }).not.toThrow(); - // Should have attempted to write something - expect(mockResponse.write).toHaveBeenCalled(); + // Should not have written anything for the malformed event + expect(mockResponse.write).not.toHaveBeenCalled(); }); }); @@ -378,7 +377,7 @@ describe('Events Router', () => { vi.clearAllMocks(); // Emit an event - mockPtyManager.emit('claudeTurn', 'test-123', 'Test Session'); + mockSessionMonitor.emit('notification', { type: 'claude-turn', sessionId: 'test-123' }); // Both clients should receive the event expect(client1Response.write).toHaveBeenCalledWith( diff --git a/web/src/test/integration/socket-protocol-integration.test.ts b/web/src/test/integration/socket-protocol-integration.test.ts index f7dfea46..e31e1011 100644 --- a/web/src/test/integration/socket-protocol-integration.test.ts +++ b/web/src/test/integration/socket-protocol-integration.test.ts @@ -260,7 +260,7 @@ describe('Socket Protocol Integration', () => { // Set up status listener on client2 let receivedStatus: { app: string; status: string } | null = null; - client2.on('status', (status) => { + client2.on('STATUS_UPDATE', (status) => { receivedStatus = status; });