Production code imports `testing` package (`internal/config/config.go`, `internal/cmdparser/test.go`)
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 55/100
- Issue type
- Refactor
- Clarity
- Mostly clear
- Activity status
- Quiet
- Tech stack
- go
- Domain
- testing-qa
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.
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).
- Dominant language
- Go
- Stars
- 595
- Forks
- 91
- Avg merge
- 9h 35m
- Merged PRs (30d)
- 1
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.
More from microsoft/go-sqlcmd
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
-
Difficulty 3/5 1-2 days Newbie friendliness 55/100
-
Difficulty 3/5 1-2 days Newbie friendliness 65/100
-
Difficulty 3/5 1-2 days Newbie friendliness 76/100
All issues in microsoft/go-sqlcmd
Similar issues
-
textual definition
Difficulty 1/5 Under an hour Newbie friendliness 90/100
geneontology/go-ontology#32653 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 75/100
-
needs design
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
Priority/High ready-for-agent Severity/Major Type/Bug
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100