Enable errcheck linter to catch unchecked error returns in tests
- 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
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