# 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`:
```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):
```go
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:
```go
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