atmos icon indicating copy to clipboard operation
atmos copied to clipboard

Add lintroller rule to enforce TestKit usage in cmd tests

Open osterman opened this issue 4 months ago • 1 comments

what

  • Add new lintroller rule testkit-required to enforce TestKit usage in cmd package tests
  • Rule detects tests that modify RootCmd state without calling NewTestKit
  • Only flags state modifications (Execute, SetArgs, ParseFlags, flag modifications)
  • Does not flag read-only access to RootCmd (Commands, Find, etc.)

why

  • RootCmd is global state shared across all tests in the cmd package
  • Without TestKit cleanup, flag values and state leak between tests causing mysterious failures
  • Tests pass in isolation but fail when run together due to state pollution
  • Enforcing TestKit usage prevents hard-to-debug test failures

references

  • Implements pattern described in CLAUDE.md Test Isolation section
  • TestKit pattern follows Go 1.15+ testing.TB interface idiom
  • Similar to existing rules: os.Setenv → t.Setenv, os.MkdirTemp → t.TempDir

details

Rule Behavior

Requires TestKit:

  • RootCmd.Execute() or RootCmd.ExecuteC()
  • RootCmd.SetArgs()
  • RootCmd.ParseFlags()
  • RootCmd.PersistentFlags().Set() or RootCmd.Flags().Set()

Does NOT require TestKit:

  • RootCmd.Commands() - read-only
  • RootCmd.Find() - read-only
  • Any other read-only access

Error Message

test function TestFoo modifies RootCmd state but does not call NewTestKit;
use _ = NewTestKit(t) to ensure proper RootCmd state cleanup
(only needed for Execute/SetArgs/ParseFlags/flag modifications, not read-only access)

Example Fix

// Before (fails linter)
func TestMyCommand(t *testing.T) {
    RootCmd.SetArgs([]string{"terraform", "plan"})
    err := RootCmd.Execute()
    // ...
}

// After (passes linter)
func TestMyCommand(t *testing.T) {
    _ = NewTestKit(t)  // Automatic cleanup
    RootCmd.SetArgs([]string{"terraform", "plan"})
    err := RootCmd.Execute()
    // ...
}

Implementation

  • New rule: tools/lintroller/rule_testkit.go
  • Test coverage: tools/lintroller/testdata/src/testkit/bad_test.go
  • Configuration: .golangci.yml enables testkit-required: true
  • Fixed existing tests: cmd/root_test.go (2 tests needed TestKit, 1 was read-only)

Testing

All lintroller tests pass:

go test ./tools/lintroller/...
# PASS

Rule successfully catches violations during pre-commit.

osterman avatar Oct 23 '25 19:10 osterman

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

github-actions[bot] avatar Oct 23 '25 19:10 github-actions[bot]