Skip to content

Enable errcheck linter to catch unchecked error returns in tests #1620

@osterman

Description

@osterman

What

Enable errcheck linter in golangci-lint configuration to catch unchecked error returns, particularly in test code.

Why

Comprehensive test audit identified 947 instances of unchecked function calls in test files. Tests calling functions that return errors but not checking those errors will pass even when the underlying code fails, providing false confidence in test coverage.

Impact

CRITICAL - Tests may be showing green while hiding real failures. This undermines the entire test suite's reliability.

Examples of Current Issues

File: cmd/auth_integration_test.go - 14 unchecked calls

cmd.Flags().Set("identity", "test-id")  // ❌ Error ignored
cmd.Flags().Set("format", "json")       // ❌ Error ignored

File: cmd/about_test.go

r, w, _ := os.Pipe()  // ❌ Error ignored

Top 10 Files by Unchecked Calls

  1. cmd/auth_integration_test.go - 14 unchecked calls
  2. cmd/auth_whoami_test.go - 13 unchecked calls
  3. cmd/auth_user_test.go - 11 unchecked calls
  4. cmd/auth_env_test.go - 9 unchecked calls
  5. cmd/auth_exec_test.go - 9 unchecked calls
  6. cmd/auth_validate_test.go - 7 unchecked calls
  7. cmd/auth_login_test.go - 6 unchecked calls
  8. cmd/atlantis_generate_repo_config_test.go - 5 unchecked calls
  9. cmd/auth_user_test.go - 6 unchecked calls
  10. Multiple other files throughout codebase

References

  • Full test quality audit: TEST_QUALITY_ACTION_PLAN.md
  • Related: 27 tests without assertions (being fixed separately)
  • Related: 20 skip-only tests (being addressed separately)

Proposed Implementation

Phase 1: Enable Linter (1 hour)

Add to .golangci.yml:

linters:
  enable:
    - errcheck
    - gosec

linters-settings:
  errcheck:
    check-blank: true
    check-type-assertions: true
    exclude-functions:
      - encoding/json.Marshal
      - encoding/json.MarshalIndent

Phase 2: Fix Incrementally (4-6 hours)

Option A - Start with new code only:

golangci-lint run --new-from-rev=origin/main

Option B - Fix top priority files first:
Focus on cmd/auth_* files with highest counts (14 files, ~80 instances)

Phase 3: Create Helper Functions

// tests/testhelpers/flags.go
func SetFlag(t *testing.T, cmd *cobra.Command, name, value string) {
    t.Helper()
    err := cmd.Flags().Set(name, value)
    require.NoError(t, err, "failed to set flag %s=%s", name, value)
}

Common Fix Patterns

Before:

cmd.Flags().Set("stack", "dev")
r, w, _ := os.Pipe()

After:

err := cmd.Flags().Set("stack", "dev")
require.NoError(t, err)

r, w, err := os.Pipe()
require.NoError(t, err)

Acceptance Criteria

  • errcheck linter enabled in .golangci.yml
  • CI pipeline running errcheck on new code
  • Top 10 files with unchecked calls are fixed
  • Helper functions created to reduce boilerplate
  • Documentation updated with best practices
  • Pre-commit hook includes errcheck validation

Estimated Effort

  • Setup: 1 hour
  • Fix top 10 files: 3-4 hours
  • Ongoing fixes: Incremental during code reviews

Total initial work: 4-6 hours

Labels

  • testing
  • tech-debt
  • priority: high
  • good first issue (for incremental fixes)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions