bookhoard/cmd/server/tests/TEST_CLEANUP_PATTERN.md

Test Resource Cleanup Pattern

Overview

This document describes the TestServerSetup pattern used for automatic resource cleanup in integration tests, which prevents database connection leaks and goroutine leaks.

Problem

Prior to this pattern, integration tests had resource leaks:

// OLD PATTERN (BROKEN)
func TestExample(t *testing.T) {
    ts, db, _ := setupTestServer(t)
    defer ts.Close()  // ❌ Only closes HTTP server
    
    // ... test code ...
    // ❌ dbPool never closed
    // ❌ connManager.StartCleanupTask() goroutine never stopped
    // ❌ queueProcessor.Start() goroutine never stopped
}

Impact:

• Each test leaked ~4 database connections (pgxpool default max_conns)
• Leaked 2+ goroutines per test (cleanup task, queue processor)
• ~160 tests = potential 640+ leaked connections
• PostgreSQL max_connections = 100 → exhaustion after ~25 tests

Solution

TestServerSetup Struct

Location: /cmd/server/tests/test_helpers.go

// TestServerSetup manages the lifecycle of a test server with proper resource cleanup
type TestServerSetup struct {
    Server          *httptest.Server
    DB              *database.Queries
    DBPool          *pgxpool.Pool
    Config          *config.Config
    ConnManager     *wsync.ConnectionManager
    QueueProcessor  *wsync.SyncQueueProcessor
    CleanupCancel   context.CancelFunc  // For connManager cleanup task
    QueueCtx        context.Context      // For queueProcessor
    QueueCancel     context.CancelFunc  // For queueProcessor
    mu              sync.Mutex
    closed          bool
}

// Close cleans up all resources in the correct order
func (s *TestServerSetup) Close() error {
    s.mu.Lock()
    defer s.mu.Unlock()

    if s.closed {
        return nil
    }

    // 1. Stop queue processor goroutine
    if s.QueueCancel != nil {
        s.QueueCancel()
        s.QueueCancel = nil
    }

    // 2. Stop connection manager cleanup task
    if s.CleanupCancel != nil {
        s.CleanupCancel()
        s.CleanupCancel = nil
    }

    // 3. Close HTTP server
    if s.Server != nil {
        s.Server.Close()
        s.Server = nil
    }

    // 4. Close database pool (waits for all connections to release)
    if s.DBPool != nil {
        s.DBPool.Close()
        s.DBPool = nil
    }

    s.closed = true
    return nil
}

setupTestServer Function

func setupTestServer(t *testing.T) *TestServerSetup {
    cfg := config.LoadConfig()
    // ... config setup ...

    // Create database pool
    dbPool, err := pgxpool.New(context.Background(), cfg.DatabaseURL())
    require.NoError(t, err)

    // Create connManager and capture cleanup cancel function
    connManager := wsync.NewConnectionManager()
    cleanupCancel := connManager.StartCleanupTask()  // ← Returns CancelFunc!

    // Create queue processor with cancellable context
    queueProcessor := wsync.NewSyncQueueProcessor(queries)
    queueCtx, queueCancel := context.WithCancel(context.Background())
    go queueProcessor.Start(queueCtx)  // ← Now cancellable!

    // ... create handlers, router, etc ...

    ts := httptest.NewServer(e)

    setup := &TestServerSetup{
        Server:         ts,
        DB:             queries,
        DBPool:         dbPool,
        Config:         cfg,
        ConnManager:    connManager,
        QueueProcessor: queueProcessor,
        CleanupCancel:  cleanupCancel,  // ← Saved for cleanup
        QueueCtx:       queueCtx,
        QueueCancel:     queueCancel,  // ← Saved for cleanup
    }

    // AUTOMATIC CLEANUP via t.Cleanup()
    t.Cleanup(func() {
        if err := setup.Close(); err != nil {
            t.Errorf("Failed to cleanup test server: %v", err)
        }
    })

    return setup
}

Usage

NEW PATTERN (Correct)

func TestExample(t *testing.T) {
    setup := setupTestServer(t)
    // No defer needed! t.Cleanup handles it automatically

    // Access resources through setup
    token := loginTestUser(t, setup.Server, setup.DB)
    mediaID := createTestMediaItemID(t, setup.Server, token)

    req, _ := http.NewRequest("GET", setup.Server.URL+"/api/test", nil)
    // ... test code ...

    // When test completes (pass or fail), setup.Close() is called automatically
}

Nested Tests

func TestWithSubtests(t *testing.T) {
    setup := setupTestServer(t)
    // setup is available in outer scope

    t.Run("subtest 1", func(t *testing.T) {
        // setup is available here too
        req, _ := http.NewRequest("POST", setup.Server.URL+"/api/test", nil)
        // ...
    })

    t.Run("subtest 2", func(t *testing.T) {
        // Each subtest shares the same setup
        // Cleanup happens when outer test completes
    })
}

Helper Functions

IMPORTANT: Helper functions that take ts *httptest.Server as parameter:

// CORRECT: Helper uses ts parameter
func createTestLibrary(t *testing.T, ts *httptest.Server, token string) string {
    req, _ := http.NewRequest("POST", ts.URL+"/api/libraries", ...)
    // ...
}

// CORRECT: Call helper with setup.Server
func TestSomething(t *testing.T) {
    setup := setupTestServer(t)
    libID := createTestLibrary(t, setup.Server, token, "test-lib")
}

Resource Cleanup Order

When setup.Close() is called (automatically via t.Cleanup()):

1. Stop Queue Processor (QueueCancel())

   • Stops goroutine processing sync queue
   • Releases queue resources

2. Stop Connection Manager (CleanupCancel())

   • Stops goroutine cleaning stale WebSocket connections
   • Releases WebSocket resources

3. Close HTTP Server (Server.Close())

   • Stops accepting new connections
   • Shuts down HTTP server gracefully

4. Close Database Pool (DBPool.Close())

   • Waits for all connections to be released
   • Returns connections to pool
   • Closes all database connections

Benefits

✅ No manual cleanup needed - t.Cleanup() handles it automatically ✅ Works even if test panics - Go runtime calls cleanup ✅ Thread-safe - Mutex prevents double-close issues ✅ Idempotent - Can call Close() multiple times safely ✅ Catches test failures - Cleanup happens even on test failure

Migration Guide

To migrate an existing test:

Before:

func TestOld(t *testing.T) {
    ts, db, _ := setupTestServer(t)
    defer ts.Close()
    
    token := loginTestUser(t, ts, db)
    req, _ := http.NewRequest("GET", ts.URL+"/api/test", nil)
}

After:

func TestNew(t *testing.T) {
    setup := setupTestServer(t)
    // No defer needed
    
    token := loginTestUser(t, setup.Server, setup.DB)
    req, _ := http.NewRequest("GET", setup.Server.URL+"/api/test", nil)
}

Verification

Check that cleanup is working:

# Before tests
podman exec bookhoard_db psql -U postgres -d bookhoard -c \
  "SELECT count(*) FROM pg_stat_activity WHERE datname = 'bookhoard';"
# Should be: 3 (app + 2 idle)

# Run tests
go test -v ./cmd/server/tests/

# After tests
podman exec bookhoard_db psql -U postgres -d bookhoard -c \
  "SELECT count(*) FROM pg_stat_activity WHERE datname = 'bookhoard';"
# Should still be: 3 (not 3 + number of tests × 4)

Implementation History

• Created: 2026-02-10
• Commits:
  • f3141f1 - Create TestServerSetup struct
  • 6c61046 - Update all test files
  • 5b32b59 - Fix edge cases
  • f15bf21 - Fix t.Run block issues
  • bb2ba14 - Final compilation fixes

Related Files

• /cmd/server/tests/test_helpers.go - TestServerSetup implementation
• /cmd/server/tests/*.go - All test files using the pattern
• PROJECT_GUIDELINES.md - Project coding standards