Skip to content

chore: standardize on *_internal_test.go for white-box tests - #25601

Merged
ethanndickson merged 1 commit into
mainfrom
test-style-5wyk
May 22, 2026
Merged

chore: standardize on *_internal_test.go for white-box tests#25601
ethanndickson merged 1 commit into
mainfrom
test-style-5wyk

Conversation

@ethanndickson

Copy link
Copy Markdown
Member

My agent added //nolint:testpackage to 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). The testpackage linter's default skip-regexp exempts that filename suffix, so the //nolint:testpackage directive is unnecessary in every case where someone reached for it. This PR renames 51 such files to *_internal_test.go via git mv so 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.md now documents the rule explicitly under Test Package Naming, which is imported into the root AGENTS.md via @.claude/docs/TESTING.md. The rule: prefer package foo_test; if you need internal access, rename the file to *_internal_test.go rather than adding a nolint directive.

@ethanndickson
ethanndickson marked this pull request as ready for review May 22, 2026 09:57
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.
Comment thread .claude/docs/TESTING.md
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 |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs lint drift, boutta put up a PR to prevent this from happening again.

@ethanndickson
ethanndickson merged commit c650aab into main May 22, 2026
27 checks passed
@ethanndickson
ethanndickson deleted the test-style-5wyk branch May 22, 2026 10:24
@github-actions github-actions Bot locked and limited conversation to collaborators May 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants