Skip to content

refactor: inject client factories into command handlers - #59

Merged
rianjs merged 1 commit into
mainfrom
refactor/36-client-factory-injection
Jan 25, 2026
Merged

refactor: inject client factories into command handlers#59
rianjs merged 1 commit into
mainfrom
refactor/36-client-factory-injection

Conversation

@rianjs

@rianjs rianjs commented Jan 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds ClientFactory variables to mail, calendar, and contacts command packages to enable dependency injection for testing. The factory functions return the corresponding interface types (from PR #58), allowing tests to inject mock implementations.

Changes

  • mail package: ClientFactory returns gmail.GmailClientInterface
  • calendar package: ClientFactory returns calendar.CalendarClientInterface
  • contacts package: ClientFactory returns contacts.ContactsClientInterface
  • Updated downloadAttachment helper to accept interface type

Testing Example

func TestSearchCommand(t *testing.T) {
    // Save and restore
    originalFactory := mail.ClientFactory
    defer func() { mail.ClientFactory = originalFactory }()

    // Inject mock
    mockClient := &MockGmailClient{...}
    mail.ClientFactory = func() (gmail.GmailClientInterface, error) {
        return mockClient, nil
    }

    // Test command behavior
    cmd := newSearchCommand()
    // ...
}

Test plan

  • All tests pass (make test)
  • Lint passes (make lint)
  • Build passes (go build ./...)

Closes #36

Add ClientFactory variables to mail, calendar, and contacts command
packages to enable dependency injection for testing. The factory
functions return the corresponding interface types, allowing tests
to inject mock implementations.

Changes:
- mail: ClientFactory returns gmail.GmailClientInterface
- calendar: ClientFactory returns calendar.CalendarClientInterface
- contacts: ClientFactory returns contacts.ContactsClientInterface
- Update downloadAttachment helper to accept interface type

Tests can now override ClientFactory to inject mocks:

    originalFactory := mail.ClientFactory
    defer func() { mail.ClientFactory = originalFactory }()
    mail.ClientFactory = func() (gmail.GmailClientInterface, error) {
        return mockClient, nil
    }

Closes #36
@rianjs

rianjs commented Jan 25, 2026

Copy link
Copy Markdown
Contributor Author

Test Coverage Assessment

Summary

This PR introduces ClientFactory variables in the mail, calendar, and contacts command packages to enable dependency injection for testing. The changes are well-structured and follow Go best practices for testability.

What the PR Changes

  1. mail/output.go: Adds ClientFactory returning gmail.GmailClientInterface
  2. calendar/output.go: Adds ClientFactory returning calendar.CalendarClientInterface
  3. contacts/output.go: Adds ClientFactory returning contacts.ContactsClientInterface
  4. mail/attachments_download.go: Updates downloadAttachment() helper to accept interface type

Test Coverage Analysis

Current State:
The existing tests focus on command structure (flags, arguments, usage strings) rather than execution behavior. None of the existing tests use the ClientFactory pattern introduced by this PR.

Assessment:

This is acceptable for an infrastructure/refactoring PR. Here's why:

  1. The PR is preparatory work - It adds the testing hooks (dependency injection) that will enable future tests. The PR description explicitly states this is refactor: work and shows how tests will use the factory.

  2. No behavioral changes - The PR only changes type signatures from concrete types (*gmail.Client) to interfaces (gmail.GmailClientInterface). The actual behavior is unchanged since the factories still call the real constructors.

  3. Compile-time verification exists - PR refactor: define interfaces for Google API clients #58 (now merged) includes compile-time interface verification: var _ GmailClientInterface = (*Client)(nil) which ensures the concrete types satisfy the interfaces.

  4. Tests still pass - All 91 existing tests continue to pass, confirming no regression.

What Would Be Nice (But Not Required)

A minimal test demonstrating the pattern would prove the mechanism works:

func TestClientFactoryCanBeOverridden(t *testing.T) {
    original := ClientFactory
    defer func() { ClientFactory = original }()
    
    called := false
    ClientFactory = func() (gmail.GmailClientInterface, error) {
        called = true
        return nil, fmt.Errorf("mock error")
    }
    
    _, err := newGmailClient()
    assert.True(t, called)
    assert.Error(t, err)
}

However, this is a "nice to have" rather than a blocker. The pattern is straightforward and follows established Go conventions.

Verdict

Approve - The test coverage is appropriate for an infrastructure refactoring PR. The real test coverage benefits will come when future PRs add mock-based tests using these factories.

@rianjs
rianjs merged commit 079ca17 into main Jan 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: inject client factories into commands for testability

1 participant