Skip to content

Production code imports testing package (internal/config/config.go, internal/cmdparser/test.go) #780

@dlevy-msft-sql

Description

@dlevy-msft-sql

Summary

internal/config/config.go and internal/cmdparser/test.go are non-_test.go files that import the standard library testing package, which is a Go anti-pattern — it pulls test-only code (and its transitive deps) into production builds.

Noted by @shueybubbles in #747 (comment).

Details

  • internal/config/config.go imports testing solely for SetFileNameForTest(t *testing.T), which only ever calls t.Name().
  • internal/cmdparser/test.go imports testing for TestSetup(t *testing.T) and related helpers.

Both files exist to provide test helpers callable from _test.go files in other packages, which is why they couldn't simply be renamed _test.go. However, the dependency on testing from production-compiled files is still undesirable.

Introduced in commit 10ee238 ("Modern CLI - Part 2"), long predating PR #747.

Suggested fix

Minimal-impact option:

  • Change config.SetFileNameForTest to accept a string (the test name) instead of *testing.T. Callers pass t.Name(). Drops the testing import from config.go.
  • Apply a similar refactor to cmdparser/test.go, or move its helpers into a dedicated cmdparsertest subpackage.

Larger option:

  • Move all cross-package test helpers into dedicated *test subpackages (e.g. internal/config/configtest, internal/cmdparser/cmdparsertest).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions