test: fix remaining 4 failing test files
Fixed 43 tests across 4 files: - AssigneeManager.test.tsx: Added i18n wrapper - process-kill.test.ts: Updated tests for current implementation - issue-create-handler.test.ts: Fixed mock setup for spawnAsync - phase5-integration.test.ts: Updated export count after useTriageMode removed Co-Authored-By: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
e1df2904cd
commit
fb67f3fbfc
@@ -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<string, HandleHandlerFn> = {};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockSpawnCalls.length = 0;
|
||||
mockSpawnOutput = 'https://github.com/owner/repo/issues/42\n';
|
||||
mockSpawnShouldFail = false;
|
||||
|
||||
(ipcMain.handle as ReturnType<typeof vi.fn>).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' });
|
||||
|
||||
@@ -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<typeof vi.fn>).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();
|
||||
});
|
||||
|
||||
+2
-2
@@ -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');
|
||||
|
||||
+42
-13
@@ -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(<I18nextProvider i18n={testI18n}>{ui}</I18nextProvider>);
|
||||
}
|
||||
|
||||
describe('AssigneeManager', () => {
|
||||
it('renders current assignees', () => {
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[
|
||||
{ login: 'alice', avatarUrl: 'https://example.com/alice.png' },
|
||||
@@ -26,7 +54,7 @@ describe('AssigneeManager', () => {
|
||||
|
||||
it('remove button fires onRemoveAssignee', () => {
|
||||
const onRemoveAssignee = vi.fn();
|
||||
render(
|
||||
const { container } = renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[{ login: 'alice' }]}
|
||||
collaborators={collaborators}
|
||||
@@ -34,14 +62,15 @@ describe('AssigneeManager', () => {
|
||||
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(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -56,7 +85,7 @@ describe('AssigneeManager', () => {
|
||||
|
||||
it('selecting fires onAddAssignee', () => {
|
||||
const onAddAssignee = vi.fn();
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -71,7 +100,7 @@ describe('AssigneeManager', () => {
|
||||
|
||||
it('Enter key on option fires onAddAssignee', () => {
|
||||
const onAddAssignee = vi.fn();
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -87,7 +116,7 @@ describe('AssigneeManager', () => {
|
||||
|
||||
it('Space key on option fires onAddAssignee', () => {
|
||||
const onAddAssignee = vi.fn();
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -102,7 +131,7 @@ describe('AssigneeManager', () => {
|
||||
});
|
||||
|
||||
it('Escape key closes dropdown', () => {
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -119,7 +148,7 @@ describe('AssigneeManager', () => {
|
||||
|
||||
it('Enter key does not fire onAddAssignee for already-assigned user', () => {
|
||||
const onAddAssignee = vi.fn();
|
||||
render(
|
||||
renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[{ login: 'alice' }]}
|
||||
collaborators={collaborators}
|
||||
@@ -134,7 +163,7 @@ describe('AssigneeManager', () => {
|
||||
});
|
||||
|
||||
it('aria-label present on container', () => {
|
||||
const { container } = render(
|
||||
const { container } = renderWithI18n(
|
||||
<AssigneeManager
|
||||
currentAssignees={[]}
|
||||
collaborators={collaborators}
|
||||
@@ -142,7 +171,7 @@ describe('AssigneeManager', () => {
|
||||
onRemoveAssignee={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
const el = container.querySelector('[aria-label="Assignee manager"]');
|
||||
const el = container.querySelector('[aria-label="Manage assignees"]');
|
||||
expect(el).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user