Improve nil handling in test mock GetDB() method

[sgaunet]

Problem

The mock's GetDB() method in internal/app/app_test.go:102 doesn't differentiate between "value not found" and "nil value", making test failures harder to debug.

Current implementation:

func (m *MockPostgreSQLClient) GetDB() *sql.DB {
    args := m.Called()
    if db, ok := args.Get(0).(*sql.DB); ok {
        return db
    }
    return nil
}

Issues

1. Ambiguous nil: Returns nil for both "not mocked" and "mocked as nil"
2. Silent failures: If Get(0) returns wrong type, silently returns nil
3. Harder debugging: Test failures don't show why GetDB returned nil

Scenarios

Scenario 1: Not mocked

mock := new(MockPostgreSQLClient)
// Forgot to mock GetDB
db := mock.GetDB() // Returns nil - but why?

Scenario 2: Mocked as nil

mock := new(MockPostgreSQLClient)
mock.On("GetDB").Return((*sql.DB)(nil))
db := mock.GetDB() // Returns nil - expected

Scenario 3: Wrong type

mock := new(MockPostgreSQLClient)
mock.On("GetDB").Return("wrong type")
db := mock.GetDB() // Returns nil - silent type error!

All three scenarios return nil, making debugging difficult.

Proposed Solution

Option 1: Use Assertion (Recommended for Tests)

func (m *MockPostgreSQLClient) GetDB() *sql.DB {
    args := m.Called()
    if args.Get(0) == nil {
        return nil
    }
    return args.Get(0).(*sql.DB) // Will panic on wrong type
}

Benefits:

• Type mismatches cause immediate panic with stack trace
• Clear distinction between nil value and missing mock
• Fails fast during test execution

Option 2: Return Error

func (m *MockPostgreSQLClient) GetDB() (*sql.DB, error) {
    args := m.Called()
    db, ok := args.Get(0).(*sql.DB)
    if !ok && args.Get(0) != nil {
        return nil, fmt.Errorf("GetDB mock returned wrong type: %T", args.Get(0))
    }
    return db, nil
}

Issues:

• Changes the interface signature
• Not worth the complexity for test code

Option 3: Better Nil Check

func (m *MockPostgreSQLClient) GetDB() *sql.DB {
    args := m.Called()
    
    // Explicit nil check
    if args.Get(0) == nil {
        return nil
    }
    
    // Type assertion with panic on failure (good for tests)
    db, ok := args.Get(0).(*sql.DB)
    if !ok {
        panic(fmt.Sprintf("GetDB mock expected *sql.DB, got %T", args.Get(0)))
    }
    return db
}

Benefits:

• Explicit handling of nil
• Clear error on type mismatch
• Self-documenting code

Recommended Approach

Use Option 3 for clarity, or Option 1 for simplicity.

Similar Issues to Fix

Check other mock methods for the same pattern:

grep -A 5 "func (m \*Mock" internal/app/app_test.go

Likely candidates:

• Any mock method returning pointers
• Any mock method using type assertions

Impact

• Severity: TRIVIAL
• Type: Testing, Code Quality
• Location: internal/app/app_test.go:102
• Benefits: Better test debugging, clearer error messages

Checklist

• Update GetDB() mock method with better nil handling
• Review other mock methods for similar issues
• Add test case for type mismatch scenario
• Document mock best practices in CONTRIBUTING.md
• Consider using testify/mock Assert methods instead of type assertions