Go Testing Code Review

Go Testing Code Review

Review Workflow

Follow this sequence in order. Do not emit findings until every Pass below is satisfied.

1. 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).

2. 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.

3. 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.

4. 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)

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

• [ ] Tests are table-driven with clear case names
• [ ] Subtests use t.Run for parallel execution
• [ ] Test names describe behavior, not implementation
• [ ] Errors include got/want with descriptive message
• [ ] Cleanup registered with t.Cleanup
• [ ] Parallel tests don't share mutable state
• [ ] Mocks use interfaces defined in test file
• [ ] Coverage includes edge cases and error paths
• [ ] Performance-critical functions have Benchmark* tests
• [ ] Input parsers/validators have Fuzz* tests (Go 1.18+)
• [ ] HTTP handlers tested with httptest.NewRequest/httptest.NewRecorder
• [ ] Golden file tests use testdata/*.golden pattern with -update flag

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

1. Are tests table-driven with named cases? 2. Do error messages include input, got, and want? 3. Are parallel tests isolated (no shared state)? 4. Is cleanup done via t.Cleanup? 5. Do tests verify behavior, not implementation?

Mocking

Interface-Based Mocking

1. Define Interface in Consumer

// service.go
type UserStore interface {
    Get(id int) (*User, error)
}

type UserService struct {
    store UserStore
}

func (s *UserService) GetUser(id int) (*User, error) {
    return s.store.Get(id)
}

2. Create Mock in Test File

// service_test.go
type mockUserStore struct {
    users map[int]*User
    err   error
}

func (m *mockUserStore) Get(id int) (*User, error) {
    if m.err != nil {
        return nil, m.err
    }
    user, ok := m.users[id]
    if !ok {
        return nil, ErrNotFound
    }
    return user, nil
}

func TestGetUser(t *testing.T) {
    mock := &mockUserStore{
        users: map[int]*User{
            1: {ID: 1, Name: "Alice"},
        },
    }

    svc := &UserService{store: mock}
    user, err := svc.GetUser(1)

    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if user.Name != "Alice" {
        t.Errorf("name = %s, want Alice", user.Name)
    }
}

3. Functional Mock Pattern

// More flexible for varying behavior per test
type mockUserStore struct {
    getFn func(id int) (*User, error)
}

func (m *mockUserStore) Get(id int) (*User, error) {
    return m.getFn(id)
}

func TestGetUser_Error(t *testing.T) {
    mock := &mockUserStore{
        getFn: func(id int) (*User, error) {
            return nil, errors.New("db error")
        },
    }

    svc := &UserService{store: mock}
    _, err := svc.GetUser(1)

    if err == nil {
        t.Error("expected error, got nil")
    }
}

Testing HTTP Clients

1. httptest Server

func TestFetchUser(t *testing.T) {
    // Create test server
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if r.URL.Path != "/users/1" {
            t.Errorf("unexpected path: %s", r.URL.Path)
        }
        w.Header().Set("Content-Type", "application/json")
        w.Write([]byte(`{"id": 1, "name": "Alice"}`))
    }))
    defer ts.Close()

    // Use test server URL
    client := NewClient(ts.URL)
    user, err := client.FetchUser(1)

    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
    if user.Name != "Alice" {
        t.Errorf("name = %s, want Alice", user.Name)
    }
}

2. RoundTripper Mock

type mockTransport struct {
    response *http.Response
    err      error
}

func (m *mockTransport) RoundTrip(*http.Request) (*http.Response, error) {
    return m.response, m.err
}

func TestClient_Error(t *testing.T) {
    client := &http.Client{
        Transport: &mockTransport{
            err: errors.New("network error"),
        },
    }

    _, err := FetchData(client, "http://example.com")
    if err == nil {
        t.Error("expected error")
    }
}

Testing Time

1. Inject Time Function

// Code
type Service struct {
    now func() time.Time
}

func (s *Service) IsExpired(expiry time.Time) bool {
    return s.now().After(expiry)
}

// Test
func TestIsExpired(t *testing.T) {
    fixedTime := time.Date(2024, 1, 15, 0, 0, 0, 0, time.UTC)
    svc := &Service{
        now: func() time.Time { return fixedTime },
    }

    tests := []struct {
        name   string
        expiry time.Time
        want   bool
    }{
        {"past", fixedTime.Add(-time.Hour), true},
        {"future", fixedTime.Add(time.Hour), false},
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            got := svc.IsExpired(tt.expiry)
            if got != tt.want {
                t.Errorf("IsExpired() = %v, want %v", got, tt.want)
            }
        })
    }
}

Testing Filesystem

1. fstest.MapFS

import "testing/fstest"

func TestReadConfig(t *testing.T) {
    fs := fstest.MapFS{
        "config.json": &fstest.MapFile{
            Data: []byte(`{"key": "value"}`),
        },
    }

    cfg, err := ReadConfig(fs, "config.json")
    if err != nil {
        t.Fatal(err)
    }
    if cfg.Key != "value" {
        t.Errorf("key = %s, want value", cfg.Key)
    }
}

2. T.TempDir

func TestWriteFile(t *testing.T) {
    dir := t.TempDir()  // automatically cleaned up
    path := filepath.Join(dir, "test.txt")

    err := WriteFile(path, "content")
    if err != nil {
        t.Fatal(err)
    }

    data, _ := os.ReadFile(path)
    if string(data) != "content" {
        t.Errorf("got %q, want content", data)
    }
}

Verifying Calls

1. Call Recording

type mockStore struct {
    getCalls []int
}

func (m *mockStore) Get(id int) (*User, error) {
    m.getCalls = append(m.getCalls, id)
    return &User{ID: id}, nil
}

func TestBatchGet(t *testing.T) {
    mock := &mockStore{}
    svc := &Service{store: mock}

    svc.BatchGet([]int{1, 2, 3})

    if len(mock.getCalls) != 3 {
        t.Errorf("Get called %d times, want 3", len(mock.getCalls))
    }
    if !slices.Equal(mock.getCalls, []int{1, 2, 3}) {
        t.Errorf("Get called with %v, want [1,2,3]", mock.getCalls)
    }
}

Anti-Patterns

1. Over-Mocking

// BAD - mocking everything
func TestAdd(t *testing.T) {
    mockCalc := &mockCalculator{}
    // just test the actual function!
}

// GOOD - only mock external dependencies
func TestService(t *testing.T) {
    mockDB := &mockDB{}  // external dependency
    svc := NewService(mockDB)
    // test service logic
}

2. Mocking Concrete Types

// BAD - can't inject mock
type Service struct {
    store *PostgresStore
}

// GOOD - interface allows mocking
type Service struct {
    store Store  // interface
}

Review Questions

1. Are interfaces defined by consumers, not producers? 2. Are mocks minimal (only implement what's tested)? 3. Are test servers used for HTTP testing? 4. Is time injected for time-dependent tests? 5. Are call recordings used to verify interactions?

Test Structure

File Organization

1. Test File Location

package/
├── user.go
├── user_test.go       # same package tests
├── user_internal_test.go  # internal tests if needed
└── testdata/          # test fixtures
    └── users.json

2. Test Naming Convention

// Function test
func TestFunctionName(t *testing.T) {}

// Method test
func TestTypeName_MethodName(t *testing.T) {}

// Scenario test
func TestGetUser_WhenNotFound_ReturnsError(t *testing.T) {}

Test Patterns

1. Setup and Teardown

func TestMain(m *testing.M) {
    // Global setup
    setup()

    code := m.Run()

    // Global teardown
    teardown()
    os.Exit(code)
}

// Per-test setup
func TestFoo(t *testing.T) {
    db := setupTestDB(t)
    t.Cleanup(func() {
        db.Close()
    })
}

2. Helper Functions

// Mark as helper for better stack traces
func assertNoError(t *testing.T, err error) {
    t.Helper()  // marks this as helper
    if err != nil {
        t.Fatalf("unexpected error: %v", err)
    }
}

func createTestUser(t *testing.T, name string) *User {
    t.Helper()
    u, err := NewUser(name)
    if err != nil {
        t.Fatalf("creating test user: %v", err)
    }
    return u
}

3. Testdata Directory

func TestParseConfig(t *testing.T) {
    // Load from testdata directory
    data, err := os.ReadFile("testdata/config.json")
    if err != nil {
        t.Fatal(err)
    }

    cfg, err := ParseConfig(data)
    // ...
}

Table-Driven Tests

1. Basic Structure

func TestParse(t *testing.T) {
    tests := []struct {
        name    string
        input   string
        want    int
        wantErr bool
    }{
        {
            name:  "valid number",
            input: "42",
            want:  42,
        },
        {
            name:    "invalid input",
            input:   "abc",
            wantErr: true,
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            got, err := Parse(tt.input)

            if tt.wantErr {
                if err == nil {
                    t.Error("expected error, got nil")
                }
                return
            }

            if err != nil {
                t.Fatalf("unexpected error: %v", err)
            }

            if got != tt.want {
                t.Errorf("Parse(%q) = %d, want %d", tt.input, got, tt.want)
            }
        })
    }
}

2. With Setup Function

func TestHandler(t *testing.T) {
    tests := []struct {
        name       string
        setup      func() *Handler
        input      Request
        wantStatus int
    }{
        {
            name: "authorized user",
            setup: func() *Handler {
                return NewHandler(WithAuth(true))
            },
            input:      Request{UserID: 1},
            wantStatus: 200,
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            h := tt.setup()
            resp := h.Handle(tt.input)
            if resp.Status != tt.wantStatus {
                t.Errorf("status = %d, want %d", resp.Status, tt.wantStatus)
            }
        })
    }
}

3. With Assertions

func TestProcess(t *testing.T) {
    tests := []struct {
        name   string
        input  []int
        check  func(t *testing.T, result []int)
    }{
        {
            name:  "preserves order",
            input: []int{3, 1, 2},
            check: func(t *testing.T, result []int) {
                if !slices.Equal(result, []int{1, 2, 3}) {
                    t.Errorf("got %v, want sorted", result)
                }
            },
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            result := Process(tt.input)
            tt.check(t, result)
        })
    }
}

Parallel Testing

1. Top-Level Parallel

func TestFoo(t *testing.T) {
    t.Parallel()  // this test runs in parallel with others

    // test code
}

2. Subtests Parallel

func TestAll(t *testing.T) {
    tests := []struct{...}

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            t.Parallel()  // subtests run in parallel
            // test code using tt
        })
    }
}

3. Avoiding Race Conditions

// Before Go 1.22, capture loop variable
for _, tt := range tests {
    tt := tt  // capture!
    t.Run(tt.name, func(t *testing.T) {
        t.Parallel()
        // use tt safely
    })
}

// Go 1.22+: not needed, loop variable is per-iteration

Error Assertions

1. Using errors.Is

func TestGetUser_NotFound(t *testing.T) {
    _, err := GetUser(999)

    if !errors.Is(err, ErrNotFound) {
        t.Errorf("got %v, want ErrNotFound", err)
    }
}

2. Using errors.As

func TestValidate(t *testing.T) {
    err := Validate(invalidInput)

    var validErr *ValidationError
    if !errors.As(err, &validErr) {
        t.Fatalf("expected ValidationError, got %T", err)
    }

    if validErr.Field != "email" {
        t.Errorf("field = %s, want email", validErr.Field)
    }
}

Benchmarks and Fuzzing

Benchmark File Organization

Benchmarks can live in the same *_test.go file as unit tests, or in a dedicated *_bench_test.go file for large suites:

package/
├── parser.go
├── parser_test.go         # unit tests
├── parser_bench_test.go   # benchmarks (optional, for large suites)
└── testdata/
    └── corpus/            # fuzz seed corpus

Benchmark Naming

// Function benchmark
func BenchmarkFunctionName(b *testing.B) {}

// Method benchmark
func BenchmarkTypeName_Method(b *testing.B) {}

Sub-Benchmarks for Input Sizes

func BenchmarkProcess(b *testing.B) {
    sizes := []int{10, 100, 1000, 10000}

    for _, size := range sizes {
        b.Run(fmt.Sprintf("size=%d", size), func(b *testing.B) {
            data := generateTestData(size)
            b.ResetTimer()

            for i := 0; i < b.N; i++ {
                Process(data)
            }
        })
    }
}

Fuzz Test Seed Corpus

Place seed corpus files in testdata/fuzz/<FuzzTestName>/:

package/
└── testdata/
    └── fuzz/
        └── FuzzParseInput/
            ├── seed1       # each file contains one corpus entry
            └── seed2

Go will also auto-generate corpus entries in $GOCACHE/fuzz/ during fuzzing runs.

Running Benchmarks in CI

# Run all benchmarks with memory stats
go test -bench=. -benchmem ./...

# Compare benchmarks across commits (using benchstat)
go test -bench=. -benchmem -count=5 ./... > old.txt
# make changes
go test -bench=. -benchmem -count=5 ./... > new.txt
benchstat old.txt new.txt

Golden Files

testdata Directory for Golden Files

Store expected outputs as golden files in the testdata/ directory:

package/
├── render.go
├── render_test.go
└── testdata/
    ├── TestRender/simple.golden
    ├── TestRender/complex.golden
    └── TestRender/empty.golden

The -update Flag Pattern

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.MkdirAll(filepath.Dir(golden), 0755); err != nil {
            t.Fatalf("creating golden dir: %v", err)
        }
        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)
    }
}

Run go test -update ./... to regenerate golden files after intentional changes.

When to Use Golden Files

• Complex output: Rendered templates, formatted text, serialized data
• Serialization formats: JSON, YAML, protobuf text format
• Code generation: Generated source files, SQL migrations
• Snapshot testing: CLI output, error messages, log formatting

Golden files are preferable to inline expected values when output is large, multi-line, or changes infrequently.

Review Questions

1. Are test files colocated with source files? 2. Do test names describe the scenario? 3. Are helper functions marked with t.Helper()? 4. Are parallel tests properly isolated? 5. Are fixtures in testdata directory?