Verify test assertions already use Error()/NoError() best practices

[Copilot]

Summary

Issue requested replacing assert.NotNil(t, err) / assert.Nil(t, err) with idiomatic assert.Error() / assert.NoError() patterns for error checking.

Investigation shows zero instances of the anti-patterns exist in the codebase.

Current State

Anti-patterns found: 0

• assert.NotNil(t, err) - none
• assert.Nil(t, err) - none
• require.NotNil(t, err) - none
• require.Nil(t, err) - none

Best practices usage: 1,154 instances across codebase

• assert.Error() - 16 uses
• assert.NoError() - 47 uses
• require.Error() - 113 uses
• require.NoError() - 978 uses

The 3 instances of Nil/NotNil found check non-error variables (config, result, finding) and are valid uses.

Conclusion

Codebase already complies with the requested best practices. No code changes required.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Code Quality] Improve test assertions to use assert.Error() instead of assert.NotNil()</issue_title>
<issue_description>## Description

Many test files use assert.NotNil(t, err) for error checking instead of the more idiomatic assert.Error(t, err). This should be updated for better test readability and consistency with Go testing best practices.

Current Pattern (Anti-pattern)

// ❌ BAD: Using NotNil for error checking
if err != nil {
    assert.NotNil(t, err, "Expected error")
}

// or
assert.NotNil(t, err, "Expected error")

Recommended Pattern

// ✅ GOOD: Using Error/NoError for error checking
assert.Error(t, err, "Expected error when...")
assert.NoError(t, err, "Should not error when...")

Why This Matters

1. Semantic clarity: Error() explicitly checks for errors, not just non-nil values
2. Better error messages: Error() includes the actual error message in test output
3. Linter compliance: Aligns with testifylint best practices
4. Consistency: Matches the pattern used in AGENTS.md guidelines (lines 90-95)

Affected Areas

Tests throughout the codebase that check for errors using:

• assert.NotNil(t, err)
• assert.Nil(t, err) (should be assert.NoError(t, err))
• require.NotNil(t, err) (should be require.Error(t, err))
• require.Nil(t, err) (should be require.NoError(t, err))

Suggested Changes

Automated Refactoring

Use a script or manual find-replace to update:

# Find files with the anti-pattern
grep -r "assert\.NotNil(t, err" pkg/ cmd/ --include="*_test.go"
grep -r "assert\.Nil(t, err" pkg/ cmd/ --include="*_test.go"
grep -r "require\.NotNil(t, err" pkg/ cmd/ --include="*_test.go"
grep -r "require\.Nil(t, err" pkg/ cmd/ --include="*_test.go"

Replacement Patterns

// Pattern 1: assert.NotNil(t, err, msg) → assert.Error(t, err, msg)
assert.NotNil(t, err, "Expected error") 
// becomes:
assert.Error(t, err, "Expected error")

// Pattern 2: assert.Nil(t, err, msg) → assert.NoError(t, err, msg)
assert.Nil(t, err, "Should not error")
// becomes:
assert.NoError(t, err, "Should not error")

// Pattern 3: Same for require.*
require.NotNil(t, err, "Setup must fail")
// becomes:
require.Error(t, err, "Setup must fail")

Success Criteria

• ✅ All error assertions use Error() or NoError()
• ✅ No usage of NotNil() or Nil() for error checking
• ✅ All tests pass (make test-unit)
• ✅ Linter passes (make lint)
• ✅ Test error messages are descriptive

Benefits

• Better test output: Error messages show actual error content
• Code clarity: Intent is immediately clear
• Linter compliance: Eliminates testifylint warnings
• Consistency: Aligns with project guidelines in AGENTS.md
• Best practices: Follows Go testing community standards

Implementation Strategy

1. Phase 1: Update pkg/constants/ tests (smallest scope)
2. Phase 2: Update pkg/cli/ tests
3. Phase 3: Update pkg/workflow/ tests
4. Phase 4: Update remaining packages

Run make agent-finish after each phase to ensure no regressions.

Reference

• AGENTS.md lines 90-95: Testifylint assertion best practices
• Testify documentation

Source

Extracted from AGENTS.md testing guidelines and common linting patterns

Priority

Low-Medium - Code quality improvement that enhances test readability

AI generated by Discussion Task Miner - Code Quality Improvement Agent

• expires on Feb 10, 2026, 5:12 AM UTC

Comments on the Issue (you are @copilot in this section)

• Fixes [Code Quality] Improve test assertions to use assert.Error() instead of assert.NotNil() #11997

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.