✅ Improve test coverage and reduce mock dependency

[ryota-murakami]

Problem

Current test suite has good coverage but relies heavily on mocks, which can hide integration issues. Several critical scenarios are untested.

Current issues:

• Heavy mocking of major functions (getGitSummary, gptCommit)
• Tests validate mock calls rather than actual behavior
• No integration tests with real OpenAI API
• Missing error scenario coverage
• No tests for edge cases

Test Coverage Gaps

1. Missing Error Scenarios

// Currently untested:
❌ OpenAI API errors (rate limits, invalid keys, timeouts)
❌ Config file corruption or invalid JSON
❌ Concurrent access to config file
❌ Network timeout scenarios
❌ Edge cases in git diff parsing (empty files, binary files, huge diffs)
❌ API response validation (malformed responses)

2. Heavy Mocking Issues

Current test pattern (tests/index.test.js):

const result = gitGptCommit.getGitSummary()
expect(result).toBeTruthy()
expect(gitGptCommit.getGitSummary).toHaveBeenCalled() // ⚠️ Only tests mock call

Problem: Tests pass even if actual implementation is broken.

3. No Integration Tests

No tests verify:

• Actual OpenAI API integration
• Real git repository operations
• End-to-end commit workflow
• Config file persistence across runs

Proposed Improvements

1. Add Error Scenario Tests

// tests/error-handling.test.js
describe('Error Handling', () => {
  it('should handle OpenAI rate limit errors', async () => {
    const mockError = new Error('Rate limit exceeded')
    mockError.status = 429
    
    vi.mocked(openai.chat.completions.create).mockRejectedValueOnce(mockError)
    
    await expect(gptCommit()).rejects.toThrow('Rate limit')
  })

  it('should handle invalid API key errors', async () => {
    const mockError = new Error('Invalid API key')
    mockError.status = 401
    
    vi.mocked(openai.chat.completions.create).mockRejectedValueOnce(mockError)
    
    await expect(gptCommit()).rejects.toThrow('Invalid API key')
  })

  it('should handle network timeout errors', async () => {
    const mockError = new Error('Network timeout')
    mockError.code = 'ETIMEDOUT'
    
    vi.mocked(exec).mockRejectedValueOnce(mockError)
    
    await expect(getGitSummary()).rejects.toThrow('Network')
  })

  it('should handle corrupted config file', () => {
    fs.writeFileSync(CONFIG_FILE, 'invalid json {{{')
    
    expect(() => loadConfig()).not.toThrow()
    expect(model).toBe('gpt-5-mini') // Should use default
  })
})

2. Add Integration Tests (Optional)

// tests/integration/openai.integration.test.js
describe('OpenAI Integration', () => {
  // Only runs if OPENAI_API_KEY is set
  const skipIfNoApiKey = process.env.OPENAI_API_KEY ? test : test.skip

  skipIfNoApiKey('should generate valid commit messages', async () => {
    const testDiff = fs.readFileSync('fixtures/sample-diff.txt', 'utf8')
    
    // Use real OpenAI API
    const message = await generateCommitMessage(testDiff)
    
    expect(message).toBeDefined()
    expect(message.length).toBeGreaterThan(10)
    expect(message.length).toBeLessThan(200)
  }, 10000) // 10s timeout for API call

  skipIfNoApiKey('should respect language setting', async () => {
    const testDiff = 'diff --git a/file.js...'
    
    const englishMessage = await generateCommitMessage(testDiff, 'English')
    const japaneseMessage = await generateCommitMessage(testDiff, '日本語')
    
    // Basic validation that language is respected
    expect(japaneseMessage).toMatch(/[\u3000-\u303f\u3040-\u309f\u30a0-\u30ff]/)
  }, 10000)
})

3. Add Edge Case Tests

// tests/edge-cases.test.js
describe('Edge Cases', () => {
  it('should handle empty git diff', async () => {
    vi.mocked(exec).mockResolvedValueOnce({ stdout: '' })
    
    const result = await getGitSummary()
    expect(result).toBeNull()
  })

  it('should handle huge git diff', async () => {
    const hugeDiff = 'diff --git...\n'.repeat(10000) // Very large diff
    vi.mocked(exec).mockResolvedValueOnce({ stdout: hugeDiff })
    
    // Should validate size and possibly warn or truncate
    await expect(getGitSummary()).rejects.toThrow('too large')
  })

  it('should handle binary file diffs', async () => {
    const binaryDiff = 'Binary files differ\n'
    vi.mocked(exec).mockResolvedValueOnce({ stdout: binaryDiff })
    
    const result = await getGitSummary()
    expect(result).toBeDefined()
  })

  it('should handle diff with sensitive data patterns', async () => {
    const sensitiveDiff = 'diff --git\n+const API_KEY = "secret123"'
    vi.mocked(exec).mockResolvedValueOnce({ stdout: sensitiveDiff })
    
    // Should warn user about sensitive data
    const consoleSpy = vi.spyOn(console, 'warn')
    await getGitSummary()
    expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('sensitive'))
  })
})

4. Reduce Mocking in Core Tests

// tests/sanitization.test.js - Good example (no mocks)
describe('sanitizeCommitMessage', () => {
  it('should remove dangerous characters', () => {
    const input = 'feat: add feature!@#$%^&*()'
    const output = sanitizeCommitMessage(input)
    expect(output).toBe('feat: add feature')
  })
})

// tests/services/GitService.test.js - Test real logic
describe('GitService', () => {
  it('should construct correct git diff command', () => {
    const service = new GitService()
    const command = service.buildDiffCommand()
    expect(command).toContain('git diff --cached')
    expect(command).toContain(':(exclude)*lock.json')
  })
})

Test Organization

tests/
  ├─ unit/                       # Pure unit tests (no mocks)
  │   ├─ utils/
  │   │   └─ sanitize.test.js
  │   ├─ services/
  │   │   ├─ OpenAIService.test.js
  │   │   ├─ GitService.test.js
  │   │   └─ ConfigService.test.js
  │   └─ validators.test.js
  ├─ integration/                # Integration tests (optional)
  │   ├─ openai.integration.test.js
  │   └─ e2e.integration.test.js
  ├─ error-scenarios/            # Error handling tests
  │   ├─ api-errors.test.js
  │   ├─ config-errors.test.js
  │   └─ network-errors.test.js
  └─ edge-cases/                 # Edge case tests
      ├─ large-diffs.test.js
      ├─ binary-files.test.js
      └─ sensitive-data.test.js

Benefits

• ✅ Better confidence in code correctness
• ✅ Catch integration issues before production
• ✅ Validate error handling actually works
• ✅ Reduce false confidence from mocks
• ✅ Better regression prevention

Acceptance Criteria

• Add error scenario test suite (10+ tests)
• Add edge case test suite (10+ tests)
• Optional: Add integration test suite (gated by env var)
• Reduce mocking in existing tests where possible
• Achieve >80% code coverage
• All tests pass reliably
• Document how to run integration tests

Priority

Medium-High - Prevents bugs from reaching users

Related

Quality analysis report: claudedocs/quality-analysis-report.md section 5

Notes

• Integration tests should be optional (gated by OPENAI_API_KEY env var)
• Consider using fixtures for consistent test data
• Add test timeout configurations for API calls
• Document test organization in tests/README.md