Skip to content

Commit 85b3ee4

Browse files
arittrarittr
committed
test: enable and fix skipped generator unit tests
- Enable previously skipped signature handling test suite by using mock agents instead of requiring real CLI - Enable previously skipped message validation tests with proper agent mocking - Enable previously skipped git command execution tests and fix argument inspection - Add agent instance injection support to CommitMessageGenerator for better testability - Update CommitMessageGeneratorConfig schema to accept Agent instance or agent name - Fix test assertions to properly inspect git command arguments as arrays 🤖 Generated with Claude via commitment
1 parent 32950c8 commit 85b3ee4

File tree

3 files changed

+114
-50
lines changed

3 files changed

+114
-50
lines changed

‎src/__tests__/generator.unit.test.ts‎

Lines changed: 68 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -329,13 +329,10 @@ describe('CommitMessageGenerator', () => {
329329
});
330330
});
331331

332-
describe.skip('signature handling (requires agent CLI)', () => {
333-
// NOTE: These tests require actual agent CLI to be installed
334-
// To properly test these, we would need to mock the agent factory
335-
// For now, these are covered by integration tests in the eval system
336-
332+
describe('signature handling', () => {
337333
it('should add default signature when none provided', async () => {
338334
const generator = new CommitMessageGenerator({
335+
agent: mockAgent,
339336
gitProvider: mockGitProvider,
340337
});
341338

@@ -346,6 +343,7 @@ describe('CommitMessageGenerator', () => {
346343

347344
it('should add custom signature when provided', async () => {
348345
const generator = new CommitMessageGenerator({
346+
agent: mockAgent,
349347
gitProvider: mockGitProvider,
350348
signature: 'Custom Test Signature',
351349
});
@@ -357,8 +355,13 @@ describe('CommitMessageGenerator', () => {
357355
});
358356

359357
it('should use codex signature for codex agent', async () => {
358+
const codexAgent: Agent = {
359+
generate: mock(async () => 'feat: add feature\n\nImplement new feature'),
360+
name: 'codex',
361+
};
362+
360363
const generator = new CommitMessageGenerator({
361-
agent: 'codex',
364+
agent: codexAgent,
362365
gitProvider: mockGitProvider,
363366
});
364367

@@ -368,8 +371,13 @@ describe('CommitMessageGenerator', () => {
368371
});
369372

370373
it('should use gemini signature for gemini agent', async () => {
374+
const geminiAgent: Agent = {
375+
generate: mock(async () => 'feat: add feature\n\nImplement new feature'),
376+
name: 'gemini',
377+
};
378+
371379
const generator = new CommitMessageGenerator({
372-
agent: 'gemini',
380+
agent: geminiAgent,
373381
gitProvider: mockGitProvider,
374382
});
375383

@@ -380,6 +388,7 @@ describe('CommitMessageGenerator', () => {
380388

381389
it('should skip signature when set to empty string', async () => {
382390
const generator = new CommitMessageGenerator({
391+
agent: mockAgent,
383392
gitProvider: mockGitProvider,
384393
signature: '',
385394
});
@@ -392,6 +401,7 @@ describe('CommitMessageGenerator', () => {
392401

393402
it('should format signature with double newline separator', async () => {
394403
const generator = new CommitMessageGenerator({
404+
agent: mockAgent,
395405
gitProvider: mockGitProvider,
396406
signature: 'TEST',
397407
});
@@ -402,85 +412,90 @@ describe('CommitMessageGenerator', () => {
402412
});
403413
});
404414

405-
describe.skip('message validation (requires agent CLI)', () => {
415+
describe('message validation', () => {
406416
it('should accept valid commit message (>= 5 chars)', async () => {
407-
mockAgent.generate = mock(async () => 'feat: test');
417+
const testAgent: Agent = {
418+
generate: mock(async () => 'feat: test'),
419+
name: 'claude',
420+
};
408421

409422
const generator = new CommitMessageGenerator({
423+
agent: testAgent,
410424
gitProvider: mockGitProvider,
411425
});
412426

413-
// Override private agent for this test - hack via constructor injection
414-
// Since we can't directly set private fields, we rely on factory creating real agent
415-
// and test will use real agent. To properly test, we'd need to mock the factory.
416-
// For now, this tests the validation logic via the actual flow.
417-
418427
const result = await generator.generateCommitMessage(validTask, validOptions);
419428

420429
expect(result.length).toBeGreaterThanOrEqual(5);
421430
});
422431

423432
it('should throw on message less than 5 chars', async () => {
424-
// Create a custom mock agent that returns short message
425-
const shortMessageGitProvider: GitProvider = {
426-
exec: mock(async () => 'diff'),
433+
const testAgent: Agent = {
434+
generate: mock(async () => 'abc'),
435+
name: 'claude',
427436
};
428437

429-
// We need to mock the agent factory to return our mock agent
430-
// Since that's complex, we'll test via the actual agent which should never return < 5 chars
431-
// This is more of an integration test, but validates the behavior
432-
433438
const generator = new CommitMessageGenerator({
434-
gitProvider: shortMessageGitProvider,
439+
agent: testAgent,
440+
gitProvider: mockGitProvider,
435441
});
436442

437-
// The real agent will either succeed (>5 chars) or fail with AgentError
438-
// If it somehow returns <5 chars, generator should throw
439-
await expect(generator.generateCommitMessage(validTask, validOptions)).rejects.toThrow();
443+
await expect(generator.generateCommitMessage(validTask, validOptions)).rejects.toThrow(
444+
'AI generation produced invalid commit message'
445+
);
440446
});
441447
});
442448

443-
describe.skip('git command execution (requires agent CLI)', () => {
444-
// NOTE: These tests require actual agent CLI - covered by integration tests
449+
describe('git command execution', () => {
445450
it('should execute git diff --stat command', async () => {
446451
const generator = new CommitMessageGenerator({
452+
agent: mockAgent,
447453
gitProvider: mockGitProvider,
448454
});
449455

450456
await generator.generateCommitMessage(validTask, validOptions);
451457

452458
expect(mockGitProvider.exec).toHaveBeenCalled();
453459
const calls = (mockGitProvider.exec as any).mock.calls;
454-
const statCall = calls.find((call: any[]) => call[0].includes('--stat'));
460+
const statCall = calls.find((call: any[]) =>
461+
call[0].some((arg: string) => arg.includes('--stat'))
462+
);
455463
expect(statCall).toBeDefined();
456464
});
457465

458466
it('should execute git diff --name-status command', async () => {
459467
const generator = new CommitMessageGenerator({
468+
agent: mockAgent,
460469
gitProvider: mockGitProvider,
461470
});
462471

463472
await generator.generateCommitMessage(validTask, validOptions);
464473

465474
const calls = (mockGitProvider.exec as any).mock.calls;
466-
const nameStatusCall = calls.find((call: any[]) => call[0].includes('--name-status'));
475+
const nameStatusCall = calls.find((call: any[]) =>
476+
call[0].some((arg: string) => arg.includes('--name-status'))
477+
);
467478
expect(nameStatusCall).toBeDefined();
468479
});
469480

470481
it('should execute git diff with unified context', async () => {
471482
const generator = new CommitMessageGenerator({
483+
agent: mockAgent,
472484
gitProvider: mockGitProvider,
473485
});
474486

475487
await generator.generateCommitMessage(validTask, validOptions);
476488

477489
const calls = (mockGitProvider.exec as any).mock.calls;
478-
const diffCall = calls.find((call: any[]) => call[0].includes('--unified'));
490+
const diffCall = calls.find((call: any[]) =>
491+
call[0].some((arg: string) => arg.includes('--unified'))
492+
);
479493
expect(diffCall).toBeDefined();
480494
});
481495

482496
it('should pass workdir to git commands', async () => {
483497
const generator = new CommitMessageGenerator({
498+
agent: mockAgent,
484499
gitProvider: mockGitProvider,
485500
});
486501

@@ -502,6 +517,7 @@ describe('CommitMessageGenerator', () => {
502517
};
503518

504519
const generator = new CommitMessageGenerator({
520+
agent: mockAgent,
505521
gitProvider: failingGitProvider,
506522
});
507523

@@ -511,40 +527,42 @@ describe('CommitMessageGenerator', () => {
511527
});
512528
});
513529

514-
describe.skip('error wrapping (requires agent CLI)', () => {
515-
it('should wrap AgentError with GeneratorError', async () => {
516-
const failingGitProvider: GitProvider = {
517-
exec: mock(async () => 'diff output'),
530+
describe('error wrapping', () => {
531+
it('should throw GeneratorError when agent returns invalid message', async () => {
532+
const failingAgent: Agent = {
533+
generate: mock(async () => ''),
534+
name: 'claude',
518535
};
519536

520537
const generator = new CommitMessageGenerator({
521-
gitProvider: failingGitProvider,
538+
agent: failingAgent,
539+
gitProvider: mockGitProvider,
522540
});
523541

524-
// Real agent will fail with AgentError (CLI not found)
525542
await expect(generator.generateCommitMessage(validTask, validOptions)).rejects.toThrow(
526543
GeneratorError
527544
);
528545
});
529546

530547
it('should include agent name in error context', async () => {
531-
const failingGitProvider: GitProvider = {
532-
exec: mock(async () => 'diff output'),
548+
const failingAgent: Agent = {
549+
generate: mock(async () => ''),
550+
name: 'codex',
533551
};
534552

535553
const generator = new CommitMessageGenerator({
536-
agent: 'codex',
537-
gitProvider: failingGitProvider,
554+
agent: failingAgent,
555+
gitProvider: mockGitProvider,
538556
});
539557

540558
try {
541559
await generator.generateCommitMessage(validTask, validOptions);
542560
throw new Error('Should have thrown');
543561
} catch (error) {
544562
expect(error).toBeInstanceOf(Error);
545-
// Error message should reference the agent
563+
// Error message should reference AI generation
546564
const message = (error as Error).message;
547-
expect(message.toLowerCase()).toMatch(/codex|ai generation/i);
565+
expect(message.toLowerCase()).toMatch(/ai generation/i);
548566
}
549567
});
550568

@@ -556,6 +574,7 @@ describe('CommitMessageGenerator', () => {
556574
};
557575

558576
const generator = new CommitMessageGenerator({
577+
agent: mockAgent,
559578
gitProvider: failingGitProvider,
560579
});
561580

@@ -570,9 +589,10 @@ describe('CommitMessageGenerator', () => {
570589
});
571590
});
572591

573-
describe.skip('end-to-end generation flow (requires agent CLI)', () => {
592+
describe('end-to-end generation flow', () => {
574593
it('should generate commit from task + git diff', async () => {
575594
const generator = new CommitMessageGenerator({
595+
agent: mockAgent,
576596
gitProvider: mockGitProvider,
577597
});
578598

@@ -597,6 +617,7 @@ describe('CommitMessageGenerator', () => {
597617
};
598618

599619
const generator = new CommitMessageGenerator({
620+
agent: mockAgent,
600621
gitProvider: multiFileGitProvider,
601622
});
602623

@@ -617,6 +638,7 @@ describe('CommitMessageGenerator', () => {
617638

618639
it('should include custom signature in output', async () => {
619640
const generator = new CommitMessageGenerator({
641+
agent: mockAgent,
620642
gitProvider: mockGitProvider,
621643
signature: '✨ Magic signature',
622644
});
@@ -629,6 +651,7 @@ describe('CommitMessageGenerator', () => {
629651

630652
it('should handle task with output context', async () => {
631653
const generator = new CommitMessageGenerator({
654+
agent: mockAgent,
632655
gitProvider: mockGitProvider,
633656
});
634657

@@ -647,6 +670,7 @@ describe('CommitMessageGenerator', () => {
647670

648671
it('should work with minimal task (no produces)', async () => {
649672
const generator = new CommitMessageGenerator({
673+
agent: mockAgent,
650674
gitProvider: mockGitProvider,
651675
});
652676

@@ -664,6 +688,7 @@ describe('CommitMessageGenerator', () => {
664688

665689
it('should work with minimal options (no files)', async () => {
666690
const generator = new CommitMessageGenerator({
691+
agent: mockAgent,
667692
gitProvider: mockGitProvider,
668693
});
669694

‎src/generator.ts‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ export type CommitTask = {
2828
* Configuration options for commit message generation
2929
*/
3030
export type CommitMessageGeneratorConfig = {
31-
/** AI agent to use ('claude' | 'codex' | 'gemini', default: 'claude') */
32-
agent?: AgentName;
31+
/** AI agent to use - either a string name or Agent instance for testing */
32+
agent?: Agent | AgentName;
3333
/** Custom git provider (default: RealGitProvider) */
3434
gitProvider?: GitProvider;
3535
/** Custom logger for debugging and diagnostics */
@@ -106,9 +106,35 @@ export class CommitMessageGenerator {
106106
// Store logger for internal use
107107
this.logger = validatedConfig.logger;
108108

109-
// Instantiate agent using factory (defaults to Claude) and pass logger
110-
const agentName = validatedConfig.agent ?? 'claude';
111-
this.agent = createAgent(agentName, this.logger);
109+
// Determine if agent is a string (use factory) or object (inject directly)
110+
const agentConfig = validatedConfig.agent ?? 'claude';
111+
let agentName: AgentName;
112+
113+
if (typeof agentConfig === 'string') {
114+
// String: use factory to create agent
115+
agentName = agentConfig;
116+
this.agent = createAgent(agentName, this.logger);
117+
} else {
118+
// Object: use injected agent directly (for testing)
119+
// Type assertion is safe because Zod schema validates the Agent interface
120+
this.agent = agentConfig as Agent;
121+
// Try to infer agent name from the agent's name property
122+
// Default to 'claude' if name doesn't match known agents
123+
agentName = match(agentConfig.name.toLowerCase())
124+
.when(
125+
(name) => name.includes('claude'),
126+
() => 'claude' as const
127+
)
128+
.when(
129+
(name) => name.includes('codex'),
130+
() => 'codex' as const
131+
)
132+
.when(
133+
(name) => name.includes('gemini'),
134+
() => 'gemini' as const
135+
)
136+
.otherwise(() => 'claude' as const);
137+
}
112138

113139
// Generate default signature based on the agent being used
114140
const defaultSignature = match<AgentName, string>(agentName)

‎src/types/schemas.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@ const loggerSchema = z.object({
9090
warn: z.function(),
9191
});
9292

93+
/**
94+
* Schema for Agent object
95+
*
96+
* Agent must implement the Agent interface with generate method and name property.
97+
*/
98+
const agentSchema = z.object({
99+
generate: z.function(),
100+
logger: loggerSchema.optional(),
101+
name: z.string(),
102+
});
103+
93104
/**
94105
* Schema for commit message generator configuration
95106
*
@@ -120,10 +131,12 @@ const loggerSchema = z.object({
120131
export const commitMessageGeneratorConfigSchema = z.object({
121132
/**
122133
* AI agent to use for commit message generation
123-
* Valid options: 'claude' | 'codex' | 'gemini'
134+
* Can be either:
135+
* - A string: 'claude' | 'codex' | 'gemini' (uses factory to create agent)
136+
* - An Agent object: for dependency injection in tests (must have generate method and name property)
124137
* Defaults to 'claude' if not specified
125138
*/
126-
agent: agentNameSchema.optional(),
139+
agent: z.union([agentNameSchema, agentSchema]).optional(),
127140

128141
/**
129142
* Custom git provider for dependency injection

0 commit comments

Comments
 (0)