Go Testing Code Review
Review Workflow
Follow this sequence in order. Do not emit findings until every Pass below is satisfied.
-
Baseline go.mod — Open go.mod for the module under review and read the go directive.
Pass: You can state the exact go X.YY value (in the review preamble or working notes). Apply version-gated advice only when it matches this baseline (e.g. fuzz tests Go 1.18+, loop-variable capture pre-Go 1.22).
-
Read surrounding tests — For each *_test.go (or benchmark/fuzz file) in scope, read full test functions and any table struct{...} / helpers they use, not only the diff hunk.
Pass: At least one full func Test... / func Benchmark... / func Fuzz... (or helper it calls) containing the change was read per in-scope file.
-
Scope the checklist — Decide which Review Checklist rows apply (table-driven structure, parallelism, HTTP, golden files, mocks). Open references/structure.md and/or references/mocking.md for those topics; skip rows N/A to the diff with a one-line reason (e.g. “no t.Parallel in change”).
Pass: The review (or working notes) lists which checklist themes you applied, or marks themes N/A with a diff-tied reason.
-
Pre-report verification — Load and follow review-verification-protocol.
Pass: The protocol’s Pre-Report Verification Checklist is satisfied for each finding you will report (actual test code read, surrounding context checked, “wrong” vs “different style” distinguished, etc.).
Hard gates (same sequence, shorter)
| Step | Objective pass condition |
|---|
| 1 | go X.YY from go.mod is recorded before version-specific test advice. |
| 2 | Full enclosing test (or helper it uses) read per in-scope test file, not diff-only. |
| 3 | In-scope checklist themes listed or N/A with diff-tied reason; references opened as needed. |
| 4 | review-verification-protocol completed for every reported issue. |
Output Format
Report findings as:
[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.
Quick Reference
Review Checklist
Critical Patterns
Table-Driven Tests
// BAD - repetitive
func TestAdd(t *testing.T) {
if Add(1, 2) != 3 {
t.Error("wrong")
}
if Add(0, 0) != 0 {
t.Error("wrong")
}
}
// GOOD
func TestAdd(t *testing.T) {
tests := []struct {
name string
a, b int
want int
}{
{"positive numbers", 1, 2, 3},
{"zeros", 0, 0, 0},
{"negative", -1, 1, 0},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := Add(tt.a, tt.b)
if got != tt.want {
t.Errorf("Add(%d, %d) = %d, want %d", tt.a, tt.b, got, tt.want)
}
})
}
}
Error Messages
// BAD
if got != want {
t.Error("wrong result")
}
// GOOD
if got != want {
t.Errorf("GetUser(%d) = %v, want %v", id, got, want)
}
// For complex types
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("GetUser() mismatch (-want +got):\n%s", diff)
}
Parallel Tests
func TestFoo(t *testing.T) {
tests := []struct{...}
for _, tt := range tests {
tt := tt // capture (not needed Go 1.22+)
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
// test code
})
}
}
Cleanup
// BAD - manual cleanup, skipped on failure
func TestWithTempFile(t *testing.T) {
f, _ := os.CreateTemp("", "test")
defer os.Remove(f.Name()) // skipped if test panics
}
// GOOD
func TestWithTempFile(t *testing.T) {
f, _ := os.CreateTemp("", "test")
t.Cleanup(func() {
os.Remove(f.Name())
})
}
Additional Patterns
Benchmarks
func BenchmarkProcess(b *testing.B) {
data := generateTestData(1000)
b.ResetTimer()
for i := 0; i < b.N; i++ {
Process(data)
}
}
// Run: go test -bench=BenchmarkProcess -benchmem
Fuzz Tests (Go 1.18+)
func FuzzParseInput(f *testing.F) {
// Seed corpus
f.Add(`{"name": "test"}`)
f.Add(``)
f.Add(`{invalid}`)
f.Fuzz(func(t *testing.T, input string) {
result, err := ParseInput(input)
if err != nil {
return // invalid input is expected
}
// If parsing succeeded, re-encoding should work
if _, err := json.Marshal(result); err != nil {
t.Errorf("Marshal after Parse: %v", err)
}
})
}
// Run: go test -fuzz=FuzzParseInput -fuzztime=30s
HTTP Handler Tests
func TestHandler(t *testing.T) {
srv := NewServer(mockDeps)
req := httptest.NewRequest("GET", "/api/users/123", nil)
w := httptest.NewRecorder()
srv.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Errorf("status = %d, want %d", w.Code, http.StatusOK)
}
}
Golden Files
var update = flag.Bool("update", false, "update golden files")
func TestRender(t *testing.T) {
got := Render(input)
golden := filepath.Join("testdata", t.Name()+".golden")
if *update {
if err := os.WriteFile(golden, got, 0644); err != nil {
t.Fatalf("writing golden file: %v", err)
}
}
want, err := os.ReadFile(golden)
if err != nil {
t.Fatalf("reading golden file: %v (run with -update to create)", err)
}
if !bytes.Equal(got, want) {
t.Errorf("output mismatch:\ngot:\n%s\nwant:\n%s", got, want)
}
}
Anti-Patterns
1. Testing Internal Implementation
// BAD - tests private state
func TestUser(t *testing.T) {
u := NewUser("alice")
if u.id != 1 { // testing internal field
t.Error("wrong id")
}
}
// GOOD - tests behavior
func TestUser(t *testing.T) {
u := NewUser("alice")
if u.ID() != 1 {
t.Error("wrong ID")
}
}
2. Shared Mutable State
// BAD - tests interfere with each other
var testDB = setupDB()
func TestA(t *testing.T) {
t.Parallel()
testDB.Insert(...) // race!
}
// GOOD - isolated per test
func TestA(t *testing.T) {
db := setupTestDB(t)
t.Cleanup(func() { db.Close() })
db.Insert(...)
}
3. Assertions Without Context
// BAD
assert.Equal(t, want, got) // "expected X got Y" - which test?
// GOOD
assert.Equal(t, want, got, "user name after update")
When to Load References
- Reviewing test file structure → structure.md
- Reviewing mock implementations → mocking.md
Review Questions
- Are tests table-driven with named cases?
- Do error messages include input, got, and want?
- Are parallel tests isolated (no shared state)?
- Is cleanup done via t.Cleanup?
- Do tests verify behavior, not implementation?