284 lines
7.5 KiB
Markdown
284 lines
7.5 KiB
Markdown
# 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:
|
||
|
||
```go
|
||
// 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`
|
||
|
||
```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
|
||
|
||
```go
|
||
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)
|
||
|
||
```go
|
||
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
|
||
|
||
```go
|
||
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:
|
||
|
||
```go
|
||
// 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:**
|
||
```go
|
||
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:**
|
||
```go
|
||
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:
|
||
|
||
```bash
|
||
# 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
|