microsoft / microsoft/go-sqlcmd
Production code imports `testing` package (`internal/config/config.go`, `internal/cmdparser/test.go`)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 595
- Forks
- 91
- Avg merge
- 9h 35m
- Merged PRs (30d)
- 1
Description
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 https://github.com/microsoft/go-sqlcmd/pull/747#discussion_r3443879059.
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.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.SetFileNameForTestto accept astring(the test name) instead of*testing.T. Callers passt.Name(). Drops thetestingimport fromconfig.go. - Apply a similar refactor to
cmdparser/test.go, or move its helpers into a dedicatedcmdparsertestsubpackage.
Larger option:
- Move all cross-package test helpers into dedicated
*testsubpackages (e.g.internal/config/configtest,internal/cmdparser/cmdparsertest).
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading internal/config/config.go and internal/cmdparser/test.go, then inspect their cross-package test-helper callers. Refactor the helpers so production-compiled files no longer import Go's testing package, choosing between string-based inputs and dedicated test subpackages. Done means those imports are gone and the existing tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing-qa
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100