diff --git a/apps/frontend/src/main/ipc-handlers/github/__tests__/issue-create-handler.test.ts b/apps/frontend/src/main/ipc-handlers/github/__tests__/issue-create-handler.test.ts index fd89ded2..6f156d70 100644 --- a/apps/frontend/src/main/ipc-handlers/github/__tests__/issue-create-handler.test.ts +++ b/apps/frontend/src/main/ipc-handlers/github/__tests__/issue-create-handler.test.ts @@ -2,16 +2,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import fs from 'fs'; // Mock child_process -const mockExecFileSync = vi.fn(); +const mockSpawnCalls: Array<{ command: string; args: string[]; options: unknown }> = []; +let mockSpawnOutput = 'https://github.com/owner/repo/issues/42\n'; +let mockSpawnShouldFail = false; -// Create a mock ChildProcess for spawn -const createMockChildProcess = (output: string) => { +const mockSpawn = vi.fn((command: string, args: string[], options: unknown) => { + mockSpawnCalls.push({ command, args, options }); const mockProcess = { stdout: { on: vi.fn((event, callback) => { if (event === 'data') { - // Call callback with data in next tick - process.nextTick(() => callback(output)); + process.nextTick(() => callback(mockSpawnOutput)); } return mockProcess.stdout; }), @@ -21,19 +22,27 @@ const createMockChildProcess = (output: string) => { }, on: vi.fn((event, callback) => { if (event === 'close') { - // Call callback with exit code 0 in next tick - process.nextTick(() => callback(0)); + process.nextTick(() => { + if (mockSpawnShouldFail) { + callback(1); + } else { + callback(0); + } + }); + } else if (event === 'error') { + if (mockSpawnShouldFail) { + process.nextTick(() => callback(new Error('gh: authentication required'))); + } } return mockProcess; }), + unref: vi.fn(), }; - return mockProcess; -}; +}); vi.mock('child_process', () => ({ - execFileSync: (...args: unknown[]) => mockExecFileSync(...args), - spawn: () => createMockChildProcess('https://github.com/owner/repo/issues/42\n'), + spawn: (...args: unknown[]) => mockSpawn(...args), })); // Mock electron @@ -104,6 +113,9 @@ const handlers: Record = {}; beforeEach(() => { vi.clearAllMocks(); + mockSpawnCalls.length = 0; + mockSpawnOutput = 'https://github.com/owner/repo/issues/42\n'; + mockSpawnShouldFail = false; (ipcMain.handle as ReturnType).mockImplementation( (channel: string, handler: HandleHandlerFn) => { @@ -119,8 +131,6 @@ const call = (projectId: string, params: { title: string; body: string; labels?: describe('createIssue handler', () => { it('creates issue with title and body via temp file', async () => { - mockExecFileSync.mockReturnValue(Buffer.from('https://github.com/owner/repo/issues/42\n')); - const result = await call('test-project', { title: 'New Bug', body: 'Bug description', @@ -132,35 +142,33 @@ describe('createIssue handler', () => { }); it('creates issue with labels', async () => { - mockExecFileSync.mockReturnValue(Buffer.from('https://github.com/owner/repo/issues/10\n')); - await call('test-project', { title: 'Feature', body: 'Feature description', labels: ['enhancement', 'priority:high'], }); - const ghArgs = mockExecFileSync.mock.calls[0][1] as string[]; - expect(ghArgs).toContain('--label'); - expect(ghArgs).toContain('enhancement,priority:high'); + const spawnCall = mockSpawnCalls.find(c => c.args.includes('--label')); + expect(spawnCall).toBeDefined(); + expect(spawnCall?.args).toContain('--label'); + expect(spawnCall?.args).toContain('enhancement,priority:high'); }); it('creates issue with assignees', async () => { - mockExecFileSync.mockReturnValue(Buffer.from('https://github.com/owner/repo/issues/11\n')); - await call('test-project', { title: 'Task', body: 'Task body', assignees: ['user1', 'user2'], }); - const ghArgs = mockExecFileSync.mock.calls[0][1] as string[]; - expect(ghArgs).toContain('--assignee'); - expect(ghArgs).toContain('user1,user2'); + const spawnCall = mockSpawnCalls.find(c => c.args.includes('--assignee')); + expect(spawnCall).toBeDefined(); + expect(spawnCall?.args).toContain('--assignee'); + expect(spawnCall?.args).toContain('user1,user2'); }); it('returns issue number and URL from gh CLI output', async () => { - mockExecFileSync.mockReturnValue(Buffer.from('https://github.com/myorg/myrepo/issues/99\n')); + mockSpawnOutput = 'https://github.com/myorg/myrepo/issues/99\n'; const result = await call('test-project', { title: 'Test', @@ -186,9 +194,7 @@ describe('createIssue handler', () => { }); it('handles gh CLI error', async () => { - mockExecFileSync.mockImplementation(() => { - throw new Error('gh: authentication required'); - }); + mockSpawnShouldFail = true; await expect(call('test-project', { title: 'Test', @@ -197,17 +203,13 @@ describe('createIssue handler', () => { }); it('cleans up temp file on success', async () => { - mockExecFileSync.mockReturnValue(Buffer.from('https://github.com/o/r/issues/1\n')); - await call('test-project', { title: 'Test', body: 'Body' }); expect(fs.unlinkSync).toHaveBeenCalled(); }); it('cleans up temp file on failure', async () => { - mockExecFileSync.mockImplementation(() => { - throw new Error('Failed'); - }); + mockSpawnShouldFail = true; try { await call('test-project', { title: 'Test', body: 'Body' }); diff --git a/apps/frontend/src/main/platform/__tests__/process-kill.test.ts b/apps/frontend/src/main/platform/__tests__/process-kill.test.ts index 3c504866..da2500e6 100644 --- a/apps/frontend/src/main/platform/__tests__/process-kill.test.ts +++ b/apps/frontend/src/main/platform/__tests__/process-kill.test.ts @@ -84,21 +84,28 @@ describe('killProcessGracefully', () => { mockPlatform('win32'); }); - it('calls process.kill() without signal argument', () => { + it('spawns taskkill immediately for graceful kill', () => { killProcessGracefully(mockProcess); - expect(mockProcess.kill).toHaveBeenCalledWith(); + expect(mockSpawn).toHaveBeenCalledWith( + 'C:\\Windows\\System32\\taskkill.exe', + ['/pid', '12345', '/t'], + expect.objectContaining({ + stdio: 'ignore', + detached: false + }) + ); }); - it('schedules taskkill as fallback after timeout', () => { + it('schedules force taskkill as fallback after timeout', () => { killProcessGracefully(mockProcess); - // Verify taskkill not called yet - expect(mockSpawn).not.toHaveBeenCalled(); + // Verify first taskkill (graceful) was called + expect(mockSpawn).toHaveBeenCalledTimes(1); // Advance past the timeout vi.advanceTimersByTime(GRACEFUL_KILL_TIMEOUT_MS); - // Verify taskkill was called with correct arguments + // Verify force taskkill was called with correct arguments expect(mockSpawn).toHaveBeenCalledWith( 'C:\\Windows\\System32\\taskkill.exe', ['/pid', '12345', '/f', '/t'], @@ -109,7 +116,7 @@ describe('killProcessGracefully', () => { ); }); - it('skips taskkill if process exits before timeout', () => { + it('skips force taskkill if process exits before timeout', () => { killProcessGracefully(mockProcess); // Simulate process exit before timeout @@ -118,12 +125,12 @@ describe('killProcessGracefully', () => { // Advance past the timeout vi.advanceTimersByTime(GRACEFUL_KILL_TIMEOUT_MS); - // Verify taskkill was NOT called - expect(mockSpawn).not.toHaveBeenCalled(); + // Verify only graceful taskkill was called, not force + expect(mockSpawn).toHaveBeenCalledTimes(1); }); it('runs taskkill even if .kill() throws (Issue #1 fix)', () => { - // Make .kill() throw an error + // Make .kill() throw an error - though Windows no longer calls .kill() (mockProcess.kill as ReturnType).mockImplementation(() => { throw new Error('Process already dead'); }); @@ -131,10 +138,17 @@ describe('killProcessGracefully', () => { // Should not throw expect(() => killProcessGracefully(mockProcess)).not.toThrow(); + // Graceful taskkill should have been called immediately + expect(mockSpawn).toHaveBeenCalledWith( + 'C:\\Windows\\System32\\taskkill.exe', + ['/pid', '12345', '/t'], + expect.any(Object) + ); + // Advance past the timeout vi.advanceTimersByTime(GRACEFUL_KILL_TIMEOUT_MS); - // taskkill should still be called - this is the key assertion for Issue #1 + // Force taskkill should also be called as fallback expect(mockSpawn).toHaveBeenCalledWith( 'C:\\Windows\\System32\\taskkill.exe', ['/pid', '12345', '/f', '/t'], @@ -232,13 +246,16 @@ describe('killProcessGracefully', () => { const customTimeout = 1000; killProcessGracefully(mockProcess, { timeoutMs: customTimeout }); - // Should not trigger at default timeout - vi.advanceTimersByTime(customTimeout - 1); - expect(mockSpawn).not.toHaveBeenCalled(); + // Graceful taskkill should be called immediately + expect(mockSpawn).toHaveBeenCalledTimes(1); - // Should trigger at custom timeout + // Force taskkill should not be called before custom timeout + vi.advanceTimersByTime(customTimeout - 1); + expect(mockSpawn).toHaveBeenCalledTimes(1); + + // Force taskkill should trigger at custom timeout vi.advanceTimersByTime(1); - expect(mockSpawn).toHaveBeenCalled(); + expect(mockSpawn).toHaveBeenCalledTimes(2); }); it('logs debug messages when debug is enabled', () => { @@ -251,7 +268,7 @@ describe('killProcessGracefully', () => { expect(warnSpy).toHaveBeenCalledWith( '[TestPrefix]', - 'Graceful kill signal sent' + 'Graceful kill signal sent (taskkill /t)' ); warnSpy.mockRestore(); @@ -329,15 +346,18 @@ describe('killProcessGracefully', () => { killProcessGracefully(mockProcess); + // Graceful taskkill should have been called immediately + expect(mockSpawn).toHaveBeenCalledTimes(1); + // Simulate process error before timeout mockProcess.emit('error', new Error('spawn failed')); // clearTimeout should have been called expect(clearTimeoutSpy).toHaveBeenCalled(); - // Advance past timeout - should not call taskkill + // Advance past timeout - should not call force taskkill (still only 1 call) vi.advanceTimersByTime(GRACEFUL_KILL_TIMEOUT_MS); - expect(mockSpawn).not.toHaveBeenCalled(); + expect(mockSpawn).toHaveBeenCalledTimes(1); clearTimeoutSpy.mockRestore(); }); diff --git a/apps/frontend/src/renderer/components/github-issues/__tests__/phase5-integration.test.ts b/apps/frontend/src/renderer/components/github-issues/__tests__/phase5-integration.test.ts index 4d082641..3b0e506d 100644 --- a/apps/frontend/src/renderer/components/github-issues/__tests__/phase5-integration.test.ts +++ b/apps/frontend/src/renderer/components/github-issues/__tests__/phase5-integration.test.ts @@ -36,10 +36,10 @@ describe('Phase 5 integration', () => { } }); - it('barrel exports cover all hooks (14+)', async () => { + it('barrel exports cover all hooks (9+)', async () => { const hooks = await import('../hooks'); const exported = Object.keys(hooks); - expect(exported.length).toBeGreaterThanOrEqual(10); + expect(exported.length).toBeGreaterThanOrEqual(9); // useTriageMode — removed in F9, replaced by investigation system expect(exported).toContain('useBulkOperations'); expect(exported).toContain('useMetrics'); diff --git a/apps/frontend/src/renderer/components/github-issues/components/__tests__/AssigneeManager.test.tsx b/apps/frontend/src/renderer/components/github-issues/components/__tests__/AssigneeManager.test.tsx index 5dc6ade1..a1eac4c8 100644 --- a/apps/frontend/src/renderer/components/github-issues/components/__tests__/AssigneeManager.test.tsx +++ b/apps/frontend/src/renderer/components/github-issues/components/__tests__/AssigneeManager.test.tsx @@ -3,13 +3,41 @@ */ import { describe, it, expect, vi } from 'vitest'; import { render, screen, fireEvent } from '@testing-library/react'; +import { I18nextProvider } from 'react-i18next'; +import i18n from 'i18next'; +import { initReactI18next } from 'react-i18next'; import { AssigneeManager } from '../AssigneeManager'; const collaborators = ['alice', 'bob', 'charlie']; +// Create test i18n instance +const testI18n = i18n.createInstance(); +testI18n.use(initReactI18next).init({ + lng: 'en', + fallbackLng: 'en', + defaultNS: 'common', + ns: ['common'], + resources: { + en: { + common: { + 'assignees.title': 'Assignees', + 'assignees.manage': 'Manage assignees', + 'assignees.assign': 'Assign', + 'assignees.unassign': 'Unassign', + 'assignees.search': 'Search collaborators...', + 'assignees.noMatch': 'No matching collaborators' + } + } + } +}); + +function renderWithI18n(ui: React.ReactElement) { + return render({ui}); +} + describe('AssigneeManager', () => { it('renders current assignees', () => { - render( + renderWithI18n( { it('remove button fires onRemoveAssignee', () => { const onRemoveAssignee = vi.fn(); - render( + const { container } = renderWithI18n( { onRemoveAssignee={onRemoveAssignee} />, ); - fireEvent.click( - screen.getByRole('button', { name: 'Remove assignee alice' }), - ); + // Find the button by aria-label (the component uses 'Unassign' as aria-label) + const button = container.querySelector('button[aria-label="Unassign"]'); + expect(button).not.toBeNull(); + fireEvent.click(button!); expect(onRemoveAssignee).toHaveBeenCalledWith('alice'); }); it('assign button opens dropdown', () => { - render( + renderWithI18n( { it('selecting fires onAddAssignee', () => { const onAddAssignee = vi.fn(); - render( + renderWithI18n( { it('Enter key on option fires onAddAssignee', () => { const onAddAssignee = vi.fn(); - render( + renderWithI18n( { it('Space key on option fires onAddAssignee', () => { const onAddAssignee = vi.fn(); - render( + renderWithI18n( { }); it('Escape key closes dropdown', () => { - render( + renderWithI18n( { it('Enter key does not fire onAddAssignee for already-assigned user', () => { const onAddAssignee = vi.fn(); - render( + renderWithI18n( { }); it('aria-label present on container', () => { - const { container } = render( + const { container } = renderWithI18n( { onRemoveAssignee={vi.fn()} />, ); - const el = container.querySelector('[aria-label="Assignee manager"]'); + const el = container.querySelector('[aria-label="Manage assignees"]'); expect(el).not.toBeNull(); }); });