Crussell/.sisyphus/plans/test-db-optimization.md

Test DB Reset Optimization — Deep Analysis & Updated Plan

Current State: The Problem

Every test (288 total) does a full schema DROP + CREATE + TRUNCATE:

setupTestDB(t)
├── testdb.Pool(t)          → New pgxpool connection (expensive)
├── testdb.Migrate(t, pool) → Full schema DROP + CREATE
│   ├── Check typeCount     → ALWAYS > 0 (shared crussell_test DB)
│   ├── DROP 14 types CASCADE
│   ├── DROP 24 tables CASCADE
│   ├── DROP 4 sequences
│   └── Run init-script.sql → CREATE all tables, types, indexes, stored procs
├── testdb.TruncateTables() → TRUNCATE 21 tables CASCADE
├── db.DB = pool            → Replace global
├── jwt.Init()              → Re-init JWT
└── defer: pool.Close()     → Close connection

227 tests call setupTestDB directly. The remaining 61 tests use custom setup functions (setupTest, setupReserveTestDB, setupTimeBlockersTestDB) that do the exact same thing — Pool + Migrate + Truncate.

All 288 tests do full schema destruction and rebuild.

---

Deep Analysis: Cross-Test Safety Verification

✅ VERIFIED SAFE: No Schema Creation by Tests

Grep for CREATE TABLE|DROP TABLE|ALTER TABLE|CREATE TYPE|DROP TYPE|CREATE INDEX|CREATE FUNCTION in all _test.go files: zero matches. No test creates, alters, or drops any schema objects. All tests only INSERT/UPDATE/DELETE row data.

✅ VERIFIED SAFE: No Cross-Test Dependencies

Every test is self-contained. Each creates its own data via fixtures or direct SQL, and either:

• Uses defer fixtures.Delete... for cleanup, OR
• Relies on TruncateTables to clean up before the next test

No test reads data created by a previous test. No test depends on sequential ordering.

✅ VERIFIED SAFE: Empty-State Tests

5 tests explicitly check for empty results:

• TestPortfolio_ListImages_Empty — expects 0 images
• TestPortfolio_ListTags_Empty — expects 0 tags
• TestPortfolio_ListFilters_Empty — expects 0 filters
• TestNotifications_ListEmpty — expects 0 notifications
• TestCustomerRelationship_NoBookings — expects user with no bookings

All are safe with TRUNCATE — truncation produces the empty state these tests expect.

✅ VERIFIED SAFE: seedDefaultWorkingHours

Used by 38 tests across 4 files. Uses ON CONFLICT (weekday) DO UPDATE — fully idempotent. Safe to call multiple times.

⚠️ FINDING: TruncateTables is missing 3 tables

The truncate list has 21 tables, but the schema has 24. Missing:

• booking_edit_requests — used by 11 tests in bookings/admin
• exceptional_group_applications — used by 3 tests in bookings/scheduling
• business_settings — 1 row, never modified by tests

Why this hasn't broken things: CASCADE foreign keys handle cleanup:

• booking_edit_requests.booking_id → bookings.id (CASCADE) → cleaned when bookings truncated
• exceptional_group_applications.group_id → exceptional_working_hours_groups.id (CASCADE) → cleaned when groups truncated
• business_settings — static seed data, never modified

Action needed: Add these 3 tables to TruncateTables for correctness. Currently relying on implicit CASCADE behavior.

⚠️ FINDING: handlers/handlers_test.go has no setup wrapper

TestIntegration_UserFlow uses testdb.Pool(t) directly (no Migrate, no Truncate) with defer fixtures.DeleteUser for cleanup. The other 3 tests in this file don't use the DB at all. This package needs a TestMain that runs Migrate once.

⚠️ FINDING: discount_test.go has dead code

truncateDiscountTables() helper is defined but never called. The file uses setupTestDB(t) which already calls the full TruncateTables. Safe to delete.

✅ VERIFIED SAFE: Global state

• auth.TokenAuth — set by jwt.Init(), called per-test currently, will be once-per-package in TestMain. No test modifies it.
• loginInProgress / loginAttempts — package-level maps in local.go. Handler cleans up via defer/delete. No test modifies them directly.
• dav.Service — set to &dav.BaseService{} in auth_test.go's setup. Persists across tests but is a read-only mock. Safe.
• fixtures.testEmailCounter — increments for unique emails. Monotonically increasing, never resets. Safe (designed for this).

---

Updated Plan

Phase 1: Add TestMain to Each Package + Fix TruncateTables

Step 1A: Fix TruncateTables to include all 24 tables

Add to testutils/testdb/testdb.go:

tables := []string{
    // ... existing 21 tables ...
    "booking_edit_requests",          // NEW
    "exceptional_group_applications", // NEW
    "business_settings",              // NEW
}

Step 1B: Add TestMain to each package

Each package gets ONE TestMain (not per file — per package). In Go, if multiple files in the same package define TestMain, it's a compile error. So we need ONE file per package with TestMain.

Package	TestMain goes in	Tests covered
handlers/bookings	New file bookings_testmain_test.go	98 (4 files)
handlers/admin	New file admin_testmain_test.go	65 (4 files)
handlers/auth	auth_test.go (add to existing)	25
handlers/scheduling	New file scheduling_testmain_test.go	34 (2 files)
handlers/portfolio	images_test.go (add to existing)	16
handlers/notifications	notifications_test.go (add to existing)	12
handlers/services	services_test.go (add to existing)	5
handlers/user	New file user_testmain_test.go	21 (3 files)
handlers	handlers_test.go (add to existing)	4
main	main_test.go (add to existing)	2

TestMain template (varies slightly per package):

func TestMain(m *testing.M) {
    pool := testdb.NewPool("")
    testdb.Migrate(&testing.T{}, pool)
    db.DB = pool
    jwt.Init()
    // auth_test.go also needs: dav.Service = &dav.BaseService{}
    code := m.Run()
    pool.Close()
    os.Exit(code)
}

Step 1C: Convert setupTestDB(t) to resetTestData(t)

Each package's setup function becomes:

func resetTestData(t *testing.T) {
    t.Helper()
    testdb.TruncateTables(t, db.DB)
    // For packages that need working hours:
    // seedDefaultWorkingHours(t)
}

Step 1D: Remove redundant per-test operations

• Remove db.DB = pool swap (pool is now global)
• Remove jwt.Init() from per-test setup (done once in TestMain)
• Remove pool.Close() from defer (pool is shared, closed in TestMain)
• Remove testdb.Pool(t) calls (use global db.DB)
• Remove testdb.Migrate(t, pool) calls (done once in TestMain)

Phase 2: Clean Up

• Delete dead truncateDiscountTables() in discount_test.go
• Consolidate seedDefaultWorkingHours into one shared function (currently duplicated in 4 files with slight variations)
• Add dav.Service mock to TestMain for auth package only

Phase 3: Smart Truncate (Optional, Later)

Identify which tables each test actually touches and truncate only those. Low priority — Phase 1 gives 95% of the benefit.

---

Risk Assessment (Updated)

Risk	Likelihood	Severity	Mitigation
Test pollution (data leaking)	Low	High	TRUNCATE CASCADE is reliable; verify with -count=2
Missing tables in TruncateTables	Confirmed	Medium	Fix in Phase 1A — add 3 missing tables
TestMain compile conflict	Low	High	ONE TestMain per package, not per file
db.DB global race	Low	High	Tests run sequentially (-p 1)
jwt.Init() called once vs per-test	Low	Medium	JWT state is idempotent; no test modifies TokenAuth
dav.Service mock persistence	Low	Low	Only auth tests use it; mock is stateless

Verification Strategy:

1. Run go test -tags test -v -p 1 -count=1 ./... — record baseline count/timing
2. Apply Phase 1A (fix TruncateTables)
3. Run tests — should still pass (no logic change)
4. Apply Phase 1B-1D (TestMain + refactor)
5. Run go test -tags test -v -p 1 -count=2 ./... — double-run catches state leakage
6. Compare: 288 tests, same pass/fail, faster execution

---

Expected Impact

Metric	Before	After Phase 1	After Phase 3
Schema DROP+CREATE	288	10 (one per package)	10
TRUNCATE per test	21 tables	21 tables	~3-5 tables
Connection pools created	288	10	10
Estimated total time	~144s	~58s	~30s

Estimates based on: DROP+CREATE ~400ms, TRUNCATE 21 tables ~100ms, pool connect ~50ms. Actual timing depends on PostgreSQL container performance.

---

What NOT to Change

• Don't add t.Parallel() — requires per-test transactions or separate databases
• Don't use transaction rollback — some tests verify side effects needing committed data
• Don't change init-script.sql — schema is correct, we're just running it too many times
• Don't touch fixtures.testEmailCounter — it's designed to be monotonic