test: adopt table-driven t.Run subtests as house convention#15
Merged
Conversation
Adopt table-driven tests with t.Run subtests as the house convention. Collapse standalone `func TestXxx_Case` functions into table-driven parents: data tables for homogeneous clusters, closure tables for heterogeneous families. Coverage is unchanged — every case is preserved as a subtest row (top-level test funcs 555 -> 146; executed cases 524 -> 552, the +28 from pre-existing bare-loop tables that gained per-case t.Run names). No production code or behavior changed. Asserted values and verbatim error-string contracts are preserved byte-for-byte. Setup stays inside each subtest; no t.Parallel added. Document the convention in TESTING.md and link it from CLAUDE.md.
Reduce the test footprint introduced by the table-driven migration by
factoring out repeated scaffolding. No behavior change; every assertion,
expected value, and error-string contract is preserved (leaf counts
unchanged: claude 93, codex 107, copilot 33, cmd/aistat 89).
- testutil: add MemStore, CountingServer, RejectServer (hoisted from the
byte-identical per-package stub helpers, now deleted).
- claude/codex: replace NewMemoryStore()+Upsert loops with MemStore;
add wantNoErr/wantErrIs/wantAccounts (+wantWarn) assertion helpers;
add a runFetch(fetchOpts{}) fixture that defaults the usual usage/
profile/refresh stub triplet so rows set only what differs.
- cmd/aistat: add wantExit (Fatalf, matches prior) / wantOut / wantErrOut
CLI assertion helpers; adopt MemStore.
- copilot: wantNoErr.
Net -568 lines vs the migration commit; the suite is now +192 lines over
the pre-migration baseline while fully on the t.Run convention.
Second footprint pass on top of the table-driven migration: - testutil: add WantNoErr/WantErrIs/WantErrContains (shared error assertions) and adopt across cred (-96), httpx, claude, codex, copilot, usagecache, orchestrate, multiaccount, cmd/aistat; delete the duplicated per-package wantNoErr/wantErrIs. accounts is left alone (its white-box `package accounts` tests can't import testutil — cycle). - render/orchestrate: externalize the large exact-equality golden output blocks to testdata/*.golden, read via testutil.LoadFixture. Byte-exact (verified) and every comparison preserved. No behavior change: leaf counts unchanged across all packages; asserted values/error-string contracts preserved byte-for-byte. Failure-mode note: ~16 errors.Is sentinel checks in claude/codex move t.Errorf -> t.Fatalf (via the shared WantErrIs), a strictness escalation that aborts before a wrong-error-type cascade; no pass/fail outcome changes.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adopts table-driven tests with
t.Runsubtests as the house test convention (perissues/table-driven-test-convention.md) and documents it in a newTESTING.md(linked fromCLAUDE.md).What changed
Churn-only — no production code or behavior changed. Every assertion and verbatim error-string contract is preserved byte-for-byte; executed-case (true-leaf) counts are unchanged across all 12 packages.
Three commits, each independently green:
func TestXxx_Caseinto table-driven parents (data tables for homogeneous clusters, closure tables for heterogeneous families). Top-level test funcs 555 → 146; executed cases 524 → 552 (the +28 are pre-existing bare-loop tables that gained per-caset.Runnames).testutilhelpers (MemStore,CountingServer,RejectServer) + per-package assertion helpers + arunFetch(fetchOpts{})fixture defaulting the usage/profile/refresh stub triplet; cmd/aistatwantExit/wantOut/wantErrOut.testutil.WantNoErr/WantErrIs/WantErrContains; move large exact-equality golden output totestdata/*.goldenviaLoadFixture.Footprint
main)testdata)Net flat on lines while gaining full
t.Runcoverage, 552 named subtest cases, and one source of test helpers.Convention doc
New
TESTING.mddocuments the idiom (slicetests, looptt,namefield,t.Run), the data-vs-closure-table rules, "cluster don't over-merge", and the verification approach.Verification
go test ./...,-race,go vet,staticcheck— all green on darwin and linux. Cross-GOOSgo test -ccompiles for the build-tagged packages, FreeBSD gate for the!darwin && !linuxfiles,-tags=fake(test + smoke) and-tags=livecompile. Faithfulness verified via per-package leaf-count parity, string-literal contract audits, and multiple reviewer passes.Notes for reviewers
internal/accountskeeps its inline helpers: its white-boxpackage accountstests can't importtestutil(which importsaccounts) without an import cycle.Containschecks useErrorfviawantOut/wantErrOut(a few wereFatalf); ~16errors.Issentinel checks in claude/codex useFatalfviaWantErrIs(a strictness escalation that aborts before a wrong-error-type cascade).