chore: standardize on *_internal_test.go for white-box tests - #25601
Merged
Conversation
ethanndickson
force-pushed
the
test-style-5wyk
branch
from
May 22, 2026 09:57
6fbaaf7 to
347321c
Compare
ethanndickson
marked this pull request as ready for review
May 22, 2026 09:57
ethanndickson
force-pushed
the
test-style-5wyk
branch
from
May 22, 2026 09:59
347321c to
22fdb4d
Compare
The 'testpackage' linter's default skip-regexp already exempts files named *_internal_test.go, so a //nolint:testpackage directive on a regular *_test.go file is unnecessary. Rename 51 such files (via git mv so history follows) and strip the dead directive from 2 files that were already correctly named. Document the convention in .claude/docs/TESTING.md so agents stop re-introducing the directive.
ethanndickson
force-pushed
the
test-style-5wyk
branch
from
May 22, 2026 10:03
22fdb4d to
1b0b024
Compare
ethanndickson
commented
May 22, 2026
Comment on lines
+81
to
+94
| | Command | Purpose | | ||
| |------------------------------------------------------|---------------------------------| | ||
| | `make test` | Run all Go tests | | ||
| | `make test RUN=TestFunctionName` | Run specific test | | ||
| | `go test -v ./path/to/package -run TestFunctionName` | Run test with verbose output | | ||
| | `make test-race` | Run tests with Go race detector | | ||
| | `make test-e2e` | Run end-to-end tests | | ||
|
|
||
| ### Frontend Testing | ||
|
|
||
| | Command | Purpose | | ||
| |---------|---------| | ||
| | `pnpm test` | Run frontend tests | | ||
| | `pnpm check` | Run code checks | | ||
| | Command | Purpose | | ||
| |--------------|--------------------| | ||
| | `pnpm test` | Run frontend tests | | ||
| | `pnpm check` | Run code checks | |
Member
Author
There was a problem hiding this comment.
Docs lint drift, boutta put up a PR to prevent this from happening again.
johnstcn
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
My agent added
//nolint:testpackageto a test file on one of my PRs. Again. This PR cleans it up across the entire repo and updates the in-repo conventions so future agents stop doing it.The repo already has a precedent for white-box tests that need to touch unexported symbols:
*_internal_test.go(145+ existing files). Thetestpackagelinter's defaultskip-regexpexempts that filename suffix, so the//nolint:testpackagedirective is unnecessary in every case where someone reached for it. This PR renames 51 such files to*_internal_test.goviagit mvso blame and history follow, and strips the dead directive from 2 files that were already correctly named (coderd/oauth2provider/authorize_internal_test.go,coderd/x/chatd/advisor_internal_test.go)..claude/docs/TESTING.mdnow documents the rule explicitly under Test Package Naming, which is imported into the rootAGENTS.mdvia@.claude/docs/TESTING.md. The rule: preferpackage foo_test; if you need internal access, rename the file to*_internal_test.gorather than adding a nolint directive.