-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
cmd.Flags().Set("identity", "test-id") // ❌ Error ignored
cmd.Flags().Set("format", "json") // ❌ Error ignoredFile: cmd/about_test.go
r, w, _ := os.Pipe() // ❌ Error ignoredTop 10 Files by Unchecked Calls
cmd/auth_integration_test.go- 14 unchecked callscmd/auth_whoami_test.go- 13 unchecked callscmd/auth_user_test.go- 11 unchecked callscmd/auth_env_test.go- 9 unchecked callscmd/auth_exec_test.go- 9 unchecked callscmd/auth_validate_test.go- 7 unchecked callscmd/auth_login_test.go- 6 unchecked callscmd/atlantis_generate_repo_config_test.go- 5 unchecked callscmd/auth_user_test.go- 6 unchecked calls- 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.MarshalIndentPhase 2: Fix Incrementally (4-6 hours)
Option A - Start with new code only:
golangci-lint run --new-from-rev=origin/mainOption 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
-
errchecklinter 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
testingtech-debtpriority: highgood first issue(for incremental fixes)