cloudposse / cloudposse/atmos

Enable errcheck linter to catch unchecked error returns in tests

Open
#1,620 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.4k
Forks
175
Avg merge
2d 3h
Merged PRs (30d)
134

Description

## 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
```go
cmd.Flags().Set("identity", "test-id") // ❌ Error ignored
cmd.Flags().Set("format", "json") // ❌ Error ignored
```

**File:** `cmd/about_test.go`
```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`:
```yaml
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:**
```bash
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

```go
// 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:**
```go
cmd.Flags().Set("stack", "dev")
r, w, _ := os.Pipe()
```

**After:**
```go
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)

Contributor guide

Open the contributing guide

Research direction

Start with .golangci.yml and review TEST_QUALITY_ACTION_PLAN.md, then run golangci-lint with --new-from-rev=origin/main to inspect the reported test calls. Check the listed cmd/*_test.go files and the acceptance criteria; done requires the linter configuration, priority test fixes, helper functions, CI and pre-commit validation, and documentation updates.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
ci-cd, cli, testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.