Skip to content

Commit afb70ef

Browse files
committed
Test optimizations to make it much faster to run them
1 parent 47047dd commit afb70ef

20 files changed

+105
-181
lines changed

CLAUDE.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,32 @@ expect(mockStdin.setRawMode).toHaveBeenCalledWith(true) // Setup
132132
expect(mockStdin.setRawMode).toHaveBeenCalledWith(false) // Cleanup
133133
```
134134

135+
**Test Configuration Best Practices**:
136+
137+
- **Leverage vitest.config.ts**: The global config already has `mockReset: true`, `clearMocks: true`, and `restoreMocks: true`. Do NOT manually call `vi.clearAllMocks()` or `vi.restoreAllMocks()` in `afterEach` hooks - these are redundant and hurt performance.
138+
139+
```typescript
140+
// ❌ Redundant: Already done by vitest.config.ts
141+
afterEach(() => {
142+
vi.clearAllMocks()
143+
vi.restoreAllMocks()
144+
})
145+
146+
// ✅ Correct: Let the global config handle it
147+
// No afterEach needed for basic mock cleanup
148+
```
149+
150+
- **Avoid Dynamic Imports**: Do NOT use dynamic imports in tests unless absolutely necessary. They significantly slow down test execution and add complexity. Use static imports with proper mocking instead.
151+
152+
```typescript
153+
// ❌ Slow: Dynamic import
154+
const { someFunction } = await import('../utils/helpers.js')
155+
156+
// ✅ Fast: Static import with mocking
157+
import { someFunction } from '../utils/helpers.js'
158+
vi.mock('../utils/helpers.js')
159+
```
160+
135161
**Mock Factories Required**:
136162

137163
```typescript

src/commands/enhance.test.ts

Lines changed: 11 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,21 @@ import type { GitHubService } from '../lib/GitHubService.js'
44
import type { AgentManager } from '../lib/AgentManager.js'
55
import type { SettingsManager, IloomSettings } from '../lib/SettingsManager.js'
66
import type { Issue } from '../types/index.js'
7+
import { launchClaude } from '../utils/claude.js'
8+
import { openBrowser } from '../utils/browser.js'
9+
import { waitForKeypress } from '../utils/prompt.js'
710

811
// Mock dependencies
912
vi.mock('../utils/claude.js')
1013
vi.mock('../utils/browser.js')
11-
vi.mock('../utils/prompt.js')
14+
vi.mock('../utils/prompt.js', () => ({
15+
waitForKeypress: vi.fn(),
16+
promptConfirmation: vi.fn(),
17+
promptInput: vi.fn(),
18+
}))
19+
vi.mock('../utils/mcp.js', () => ({
20+
generateGitHubCommentMcpConfig: vi.fn().mockResolvedValue([]),
21+
}))
1222

1323
// Mock the logger to prevent console output during tests
1424
vi.mock('../utils/logger.js', () => ({
@@ -93,7 +103,6 @@ describe('EnhanceCommand', () => {
93103
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
94104
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
95105

96-
const { launchClaude } = await import('../utils/claude.js')
97106
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
98107

99108
await expect(
@@ -119,7 +128,6 @@ describe('EnhanceCommand', () => {
119128
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
120129
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
121130

122-
const { launchClaude } = await import('../utils/claude.js')
123131
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
124132

125133
await command.execute({ issueNumber: 123, options: {} })
@@ -166,7 +174,6 @@ describe('EnhanceCommand', () => {
166174
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
167175
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
168176

169-
const { launchClaude } = await import('../utils/claude.js')
170177
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
171178

172179
await command.execute({ issueNumber: 42, options: {} })
@@ -179,7 +186,6 @@ describe('EnhanceCommand', () => {
179186
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
180187
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
181188

182-
const { launchClaude } = await import('../utils/claude.js')
183189
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
184190

185191
await command.execute({ issueNumber: 42, options: {} })
@@ -198,7 +204,6 @@ describe('EnhanceCommand', () => {
198204
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
199205
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
200206

201-
const { launchClaude } = await import('../utils/claude.js')
202207
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
203208

204209
await command.execute({ issueNumber: 42, options: {} })
@@ -215,7 +220,6 @@ describe('EnhanceCommand', () => {
215220
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
216221
vi.mocked(mockAgentManager.formatForCli).mockReturnValue(mockAgents)
217222

218-
const { launchClaude } = await import('../utils/claude.js')
219223
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
220224

221225
await command.execute({ issueNumber: 42, options: {} })
@@ -231,7 +235,6 @@ describe('EnhanceCommand', () => {
231235
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
232236
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
233237

234-
const { launchClaude } = await import('../utils/claude.js')
235238
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
236239

237240
await command.execute({ issueNumber: 42, options: {} })
@@ -261,8 +264,6 @@ describe('EnhanceCommand', () => {
261264
})
262265

263266
it('should detect "No enhancement needed" response', async () => {
264-
const { launchClaude } = await import('../utils/claude.js')
265-
const { waitForKeypress } = await import('../utils/prompt.js')
266267
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
267268

268269
await command.execute({ issueNumber: 42, options: {} })
@@ -273,8 +274,6 @@ describe('EnhanceCommand', () => {
273274

274275
it('should detect comment URL in response', async () => {
275276
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
276-
const { launchClaude } = await import('../utils/claude.js')
277-
const { waitForKeypress } = await import('../utils/prompt.js')
278277
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
279278
vi.mocked(waitForKeypress).mockResolvedValue('a')
280279

@@ -286,9 +285,6 @@ describe('EnhanceCommand', () => {
286285

287286
it('should extract comment URL from response', async () => {
288287
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
289-
const { launchClaude } = await import('../utils/claude.js')
290-
const { waitForKeypress } = await import('../utils/prompt.js')
291-
const { openBrowser } = await import('../utils/browser.js')
292288
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
293289
vi.mocked(waitForKeypress).mockResolvedValue('a')
294290

@@ -299,7 +295,6 @@ describe('EnhanceCommand', () => {
299295
})
300296

301297
it('should handle malformed responses gracefully', async () => {
302-
const { launchClaude } = await import('../utils/claude.js')
303298
vi.mocked(launchClaude).mockResolvedValue('Some unexpected response format')
304299

305300
await expect(
@@ -308,7 +303,6 @@ describe('EnhanceCommand', () => {
308303
})
309304

310305
it('should throw error on empty response', async () => {
311-
const { launchClaude } = await import('../utils/claude.js')
312306
vi.mocked(launchClaude).mockResolvedValue('')
313307

314308
await expect(
@@ -317,7 +311,6 @@ describe('EnhanceCommand', () => {
317311
})
318312

319313
it('should detect and handle permission denied responses', async () => {
320-
const { launchClaude } = await import('../utils/claude.js')
321314
vi.mocked(launchClaude).mockResolvedValue('Permission denied: GitHub CLI not authenticated or not installed')
322315

323316
await expect(
@@ -326,7 +319,6 @@ describe('EnhanceCommand', () => {
326319
})
327320

328321
it('should handle permission denied with case insensitive matching', async () => {
329-
const { launchClaude } = await import('../utils/claude.js')
330322
vi.mocked(launchClaude).mockResolvedValue('permission denied: Cannot access repository or issue does not exist')
331323

332324
await expect(
@@ -335,7 +327,6 @@ describe('EnhanceCommand', () => {
335327
})
336328

337329
it('should handle permission denied for comment creation', async () => {
338-
const { launchClaude } = await import('../utils/claude.js')
339330
vi.mocked(launchClaude).mockResolvedValue('Permission denied: Cannot create comments on this repository')
340331

341332
await expect(
@@ -344,7 +335,6 @@ describe('EnhanceCommand', () => {
344335
})
345336

346337
it('should handle permission denied for API rate limits', async () => {
347-
const { launchClaude } = await import('../utils/claude.js')
348338
vi.mocked(launchClaude).mockResolvedValue('Permission denied: GitHub API rate limit exceeded')
349339

350340
await expect(
@@ -371,8 +361,6 @@ describe('EnhanceCommand', () => {
371361
})
372362

373363
it('should not prompt for browser when no enhancement needed', async () => {
374-
const { launchClaude } = await import('../utils/claude.js')
375-
const { waitForKeypress } = await import('../utils/prompt.js')
376364
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
377365

378366
await command.execute({ issueNumber: 42, options: {} })
@@ -382,8 +370,6 @@ describe('EnhanceCommand', () => {
382370

383371
it('should prompt "Press q to quit or any other key to view" when enhanced', async () => {
384372
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
385-
const { launchClaude } = await import('../utils/claude.js')
386-
const { waitForKeypress } = await import('../utils/prompt.js')
387373
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
388374
vi.mocked(waitForKeypress).mockResolvedValue('a')
389375

@@ -396,9 +382,6 @@ describe('EnhanceCommand', () => {
396382

397383
it('should open browser when user does not press q', async () => {
398384
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
399-
const { launchClaude } = await import('../utils/claude.js')
400-
const { waitForKeypress } = await import('../utils/prompt.js')
401-
const { openBrowser } = await import('../utils/browser.js')
402385
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
403386
vi.mocked(waitForKeypress).mockResolvedValue('a')
404387

@@ -409,9 +392,6 @@ describe('EnhanceCommand', () => {
409392

410393
it('should NOT open browser when user presses q', async () => {
411394
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
412-
const { launchClaude } = await import('../utils/claude.js')
413-
const { waitForKeypress } = await import('../utils/prompt.js')
414-
const { openBrowser } = await import('../utils/browser.js')
415395
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
416396
vi.mocked(waitForKeypress).mockResolvedValue('q')
417397

@@ -423,9 +403,6 @@ describe('EnhanceCommand', () => {
423403

424404
it('should NOT open browser when user presses Q (uppercase)', async () => {
425405
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
426-
const { launchClaude } = await import('../utils/claude.js')
427-
const { waitForKeypress } = await import('../utils/prompt.js')
428-
const { openBrowser } = await import('../utils/browser.js')
429406
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
430407
vi.mocked(waitForKeypress).mockResolvedValue('Q')
431408

@@ -437,9 +414,6 @@ describe('EnhanceCommand', () => {
437414

438415
it('should skip browser when --no-browser flag is set', async () => {
439416
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
440-
const { launchClaude } = await import('../utils/claude.js')
441-
const { waitForKeypress } = await import('../utils/prompt.js')
442-
const { openBrowser } = await import('../utils/browser.js')
443417
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
444418

445419
await command.execute({ issueNumber: 42, options: { noBrowser: true } })
@@ -450,9 +424,6 @@ describe('EnhanceCommand', () => {
450424

451425
it('should handle browser opening failures gracefully', async () => {
452426
const commentUrl = 'https://github.com/owner/repo/issues/42#issuecomment-123456'
453-
const { launchClaude } = await import('../utils/claude.js')
454-
const { waitForKeypress } = await import('../utils/prompt.js')
455-
const { openBrowser } = await import('../utils/browser.js')
456427
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
457428
vi.mocked(waitForKeypress).mockResolvedValue('a')
458429
vi.mocked(openBrowser).mockRejectedValue(new Error('Browser failed to open'))
@@ -498,19 +469,16 @@ describe('EnhanceCommand', () => {
498469
return {}
499470
})
500471

501-
const { launchClaude } = await import('../utils/claude.js')
502472
vi.mocked(launchClaude).mockImplementation(async () => {
503473
calls.push('launchClaude')
504474
return 'https://github.com/owner/repo/issues/42#issuecomment-123456'
505475
})
506476

507-
const { waitForKeypress } = await import('../utils/prompt.js')
508477
vi.mocked(waitForKeypress).mockImplementation(async () => {
509478
calls.push('waitForKeypress')
510479
return 'a'
511480
})
512481

513-
const { openBrowser } = await import('../utils/browser.js')
514482
vi.mocked(openBrowser).mockImplementation(async () => {
515483
calls.push('openBrowser')
516484
})
@@ -544,8 +512,6 @@ describe('EnhanceCommand', () => {
544512
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
545513
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
546514

547-
const { launchClaude } = await import('../utils/claude.js')
548-
const { openBrowser } = await import('../utils/browser.js')
549515
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
550516

551517
await command.execute({ issueNumber: 42, options: {} })
@@ -571,9 +537,6 @@ describe('EnhanceCommand', () => {
571537
vi.mocked(mockAgentManager.loadAgents).mockResolvedValue([])
572538
vi.mocked(mockAgentManager.formatForCli).mockReturnValue({})
573539

574-
const { launchClaude } = await import('../utils/claude.js')
575-
const { waitForKeypress } = await import('../utils/prompt.js')
576-
const { openBrowser } = await import('../utils/browser.js')
577540
vi.mocked(launchClaude).mockResolvedValue(commentUrl)
578541
vi.mocked(waitForKeypress).mockResolvedValue('a')
579542

@@ -602,7 +565,6 @@ describe('EnhanceCommand', () => {
602565
})
603566

604567
it('should include author in prompt when provided', async () => {
605-
const { launchClaude } = await import('../utils/claude.js')
606568
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
607569

608570
await command.execute({ issueNumber: 42, options: { author: 'testuser' } })
@@ -614,7 +576,6 @@ describe('EnhanceCommand', () => {
614576
})
615577

616578
it('should not include author reference in prompt when not provided', async () => {
617-
const { launchClaude } = await import('../utils/claude.js')
618579
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
619580

620581
await command.execute({ issueNumber: 42, options: {} })
@@ -624,7 +585,6 @@ describe('EnhanceCommand', () => {
624585
})
625586

626587
it('should work without author parameter for backwards compatibility', async () => {
627-
const { launchClaude } = await import('../utils/claude.js')
628588
vi.mocked(launchClaude).mockResolvedValue('No enhancement needed')
629589

630590
await expect(

src/commands/feedback.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,39 @@ import type { SettingsManager } from '../lib/SettingsManager.js'
77

88
// Mock dependencies
99
vi.mock('../lib/IssueEnhancementService.js')
10+
vi.mock('../utils/diagnostics.js', () => {
11+
const mockDiagnostics = {
12+
cliVersion: '1.0.0',
13+
nodeVersion: 'v18.0.0',
14+
osType: 'darwin',
15+
osVersion: '10.0.0',
16+
architecture: 'arm64',
17+
capabilities: [],
18+
claudeVersion: null,
19+
}
20+
const mockMarkdown = `<!-- CLI GENERATED FEEDBACK v1.0.0 -->
21+
22+
<details>
23+
<summary>Diagnostic Information</summary>
24+
25+
| Property | Value |
26+
|----------|-------|
27+
| CLI Version | 1.0.0 |
28+
| Node.js Version | v18.0.0 |
29+
| OS | darwin |
30+
| OS Version | 10.0.0 |
31+
| Architecture | arm64 |
32+
| Capabilities | none |
33+
| Claude CLI Version | not available |
34+
35+
</details>
36+
`
37+
38+
return {
39+
gatherDiagnosticInfo: vi.fn(async () => mockDiagnostics),
40+
formatDiagnosticsAsMarkdown: vi.fn(() => mockMarkdown),
41+
}
42+
})
1043

1144
// Mock the logger to prevent console output during tests
1245
vi.mock('../utils/logger.js', () => ({

0 commit comments

Comments
 (0)