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).
Summary
internal/config/config.goandinternal/cmdparser/test.goare non-_test.gofiles that import the standard librarytestingpackage, 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.goimportstestingsolely forSetFileNameForTest(t *testing.T), which only ever callst.Name().internal/cmdparser/test.goimportstestingforTestSetup(t *testing.T)and related helpers.Both files exist to provide test helpers callable from
_test.gofiles in other packages, which is why they couldn't simply be renamed_test.go. However, the dependency ontestingfrom production-compiled files is still undesirable.Introduced in commit
10ee238("Modern CLI - Part 2"), long predating PR #747.Suggested fix
Minimal-impact option:
config.SetFileNameForTestto accept astring(the test name) instead of*testing.T. Callers passt.Name(). Drops thetestingimport fromconfig.go.cmdparser/test.go, or move its helpers into a dedicatedcmdparsertestsubpackage.Larger option:
*testsubpackages (e.g.internal/config/configtest,internal/cmdparser/cmdparsertest).