# Testing Architecture & DB Management

**Last Updated:** July 2026 (v5 — coverage 50.4%→65.0%, ~1,642 tests)

---

## 1. Architecture Overview

Every Go package with integration tests gets **its own isolated PostgreSQL database**. This eliminates all cross-package test contamination and enables parallel test execution.

```
┌─────────────────────┐     ┌──────────────────────────┐
│ handlers/bookings   │ ─── │ crussell_test_bookings    │
├─────────────────────┤     ├──────────────────────────┤
│ handlers/auth       │ ─── │ crussell_test_handlers_auth     │
├─────────────────────┤     ├──────────────────────────┤
│ handlers/payments   │ ─── │ crussell_test_handlers_payments  │
├─────────────────────┤     ├──────────────────────────┤
│ ...                 │     │ ...                      │
└─────────────────────┘     └──────────────────────────┘
```

Each database is created on demand by `testdb.CreateTestDatabase(dbName)` in the package's `TestMain`, migrated with the full schema (`init-scripts/init-script.sql`), and dropped by `testdb.DestroyTestDatabase(pool, dbName)` when tests finish.

---

## 2. Running Tests

### Environment Variables

| Var | Purpose | Required? |
|-----|---------|-----------|
| `GO_TESTING` | Suppresses artificial delays in Square mock; disables zxcvbn password checks in RegisterHandler | **Yes** for dev server & tests |
| `JWT_SECRET_KEY` | Used by production `main.go` init() | Only for `go run ./main.go` (not needed for tests since the `-test.` flag guard skips it) |

`POSTGRES_USER`, `POSTGRES_PASSWORD`, `POSTGRES_HOST`, `POSTGRES_DB` are set by `local-dev-2.sh` but NOT consumed by the test infrastructure directly — the test DSN is hardcoded in `testutils/testdb/testdb.go`.

### Commands

```bash
# Full suite (parallel — packages run concurrently against their own databases)
go test -tags "test,dev" -count=1 -parallel 8 ./...

# Single package
go test -tags "test,dev" -v -count=1 ./handlers/payments/

# Single test
go test -tags "test,dev" -v -count=1 -run TestMyTest ./handlers/payments/

# Compile-check only (zero-test run)
go test -tags "test,dev" -count=1 -run ^$ ./path/to/package
```

Packages **can safely run in parallel** (`-p` defaults to `GOMAXPROCS`). Each package has its own database, so there's no advisory lock contention. The `-p 1` flag from the old architecture is **no longer required or recommended**.

### What Happens When You Run Tests

1. `go test` compiles each package's test binary
2. Each binary starts → runs `TestMain` → calls `CreateTestDatabase("crussell_test_<package>")`
3. `CreateTestDatabase`:
   - Connects to `mydb` (admin DB)
   - Drops the package's old test database if it exists
   - Creates a fresh database
   - Runs `init-scripts/init-script.sql` (full schema migration)
   - Returns a pool connected to the new database
4. Each test starts with `ctx, tx := testutils.SetupTestTx(t)` which begins a PostgreSQL transaction. All DB operations within the test use `tx` (the `db.Querier` interface) and `ctx` (the context with the embedded transaction). `t.Cleanup` rolls back the transaction when the test completes — **no truncation needed**.
5. Request helpers (`makeRequest`, `makeAuthRequest`, etc.) accept `ctx context.Context` — always pass the `ctx` from `SetupTestTx` to route DB calls through the test's transaction.
6. Tests can safely use `t.Parallel()` — each test goroutine has its own transaction context, so there's no cross-test data contamination. The `SetupTestTx` semaphore (cap 8) prevents connection pool exhaustion.
7. `TestMain` calls `DestroyTestDatabase` which closes the pool and drops the database

### Verification Workflow

Tests must pass reliably at both verification levels:

```
# Development quick-check (fast):
go test -tags "test,dev" -count=1 -parallel 8 ./...    # ~14s

# Thorough completion verification (catches flakiness):
go test -tags "test,dev" -count=10 -parallel 8 ./...   # longer — run before merging
```

Features and their tests should pass `-count=1` for iterative development, but always confirm with `-count=10` before considering a feature complete. This catches race conditions in shared globals, goroutine-unsafe library types (e.g., `golang.org/x/text/cases.Caser`), and timing-dependent failures.

**Known flakiness sources caught by -count=10:**
- Goroutine-unsafe package-level variables used concurrently (fixed: `titleCaser` in `local.go`, `testEmailCounter` in fixtures)
- Shared global state cleared by one test affecting another (`loginInProgress` map in auth handler)
- Polling timeouts in mock clients (`square_dev_test.go` mockSleep 3s vs test polling 100ms)

---

## 3. Package Setup — `TestMain` Pattern

Every package with integration tests follows this exact pattern:

```go
//go:build test

package mypackage

import (
    "os"
    "testing"

    "crussell/db"
    "crussell/testutils"
    "crussell/testutils/jwt"
    "crussell/testutils/testdb"
)

func TestMain(m *testing.M) {
    pool := testdb.CreateTestDatabase("crussell_test_mypackage")
    db.Conn = db.NewPoolProxy(pool)
    testdb.SeedBaseline(pool)  // working hours, business settings
    jwt.Init()
    code := m.Run()
    db.Conn.Pool().Close()
    testdb.DestroyTestDatabase(pool, "crussell_test_mypackage")
    os.Exit(code)
}
```

The key difference from the old architecture: `db.Conn` is a `*db.PoolProxy` (not a raw `*pgxpool.Pool`). The `PoolProxy` checks `context.Context` for an active transaction via `TxFromContext()`. If found, all `Exec`/`Query`/`QueryRow` calls route through the transaction; if not, they delegate to the underlying pool.

**`db.Conn` is now the single point of DB access for both production and test code.** No more raw pool references in handlers.

### Test Writing Pattern

```go
func TestMyHandler(t *testing.T) {
    t.Parallel()
    ctx, tx := testutils.SetupTestTx(t)

    // Use 'tx' (db.Querier) for all DB operations
    userID, err := fixtures.CreateTestUser(tx)

    // Pass 'ctx' to request helpers so handlers route through the tx
    w := makeRequest(handler, "GET", "/path", nil, token, ctx)

    // Verify results directly in the tx
    var status string
    err = tx.QueryRow(ctx, "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&status)
}
```

### PoolProxy Architecture

```
PoolProxy (db/proxy.go)
├── Exec/Query/QueryRow  → checks context for tx, routes to tx or pool
├── Begin                → checks context for tx → savepoint, or pool.Begin
├── Acquire              → ALWAYS goes to p.pool.Acquire (no context tx check)
├── Ping                 → ALWAYS goes to p.pool.Ping
└── Pool()               → returns raw *pgxpool.Pool (escape hatch for tests)

**Exception — DAV service:** `internal/dav/` has its own separate `*pgxpool.Pool` (not wrapped in PoolProxy). DAV operations (`CreateContact`, `DeleteContact`, `CreateEvent`) bypass PoolProxy entirely and always hit the real database. In tests, `dav.Service` is replaced with a nil-db stub (`&dav.BaseService{}`) so DAV operations are no-ops.
```

### Database naming convention

`crussell_test_<package_path_with_underscores>`

| Package path | Database name |
|---|---|
| `.` (root) | `crussell_test` |
| `auth` | `crussell_test_auth` |
| `handlers/admin` | `crussell_test_handlers_admin` |
| `handlers/bookings` | `crussell_test_handlers_bookings` |
| `handlers/user` | `crussell_test_handlers_user` |
| `handlers/today` | `crussell_test_handlers_today` |

### Exceptions

- **`auth/jwt_test.go`**: Uses the standard `CreateTestDatabase` + `DestroyTestDatabase` pattern. JTI revocation tests require the database; non-DB tests (token format, uniqueness) work without it.
- **`db/db_test.go`**: Manages its own connection pool via `db.Connect()`. Uses `CreateTestDatabase` to ensure the database exists, but the pool is managed separately.

---

## 4. Adding a New Test Package

1. **Create the test file(s)** in the package directory with `//go:build test` constraint
2. **Add TestMain** (see §3 pattern). Must call `testdb.SeedBaseline(pool)` after creating the database and before `m.Run()` — this seeds working hours and business settings at the pool level so all per-test transactions can see them.
3. **Use `db.NewPoolProxy(pool)`** to wrap the pool — don't assign the raw pool directly to `db.Conn`.
4. **Write tests** using `ctx, tx := testutils.SetupTestTx(t)` pattern with `t.Parallel()`.
5. **Add the database name** to the cleanup list in `local-dev-2.sh` if it doesn't match `crussell_test*`:

```bash
# In local-dev-2.sh — the cleanup regex catches crussell_test* by default
docker exec postgres psql -U myuser -d mydb -t -c "
  SELECT datname FROM pg_database WHERE datname LIKE 'crussell_test%';
" | grep crussell_test | while read -r dbname; do ... done
```

### What It Creates Automatically

- Each package's first test run: `CreateTestDatabase` drops old DB → creates new DB → runs `init-script.sql` migration (~1s)
- Subsequent runs: same flow but creates fresh database each time
- `SeedBaseline` inserts default working hours (Mon-Sun 08:00-20:00, all open) and business settings
- Each test's `SetupTestTx` creates a transaction that rolls back automatically — no data persists between tests

---

## 5. Adding a New Table or Type to the Schema

When you modify `init-scripts/init-script.sql` to add tables or types, you must update the drop lists in `backend/testutils/testdb/testdb.go`:

| Change | What to Do |
|--------|-----------|
| **New table** | Add to `dropOrder` array |
| **New enum type** | Add to `typeDrops` array in `Migrate` |
| **New sequence** | Add to `seqDrops` |
| **Removed table/type** | Remove from the corresponding lists |

**Note:** The old `TruncateTables` function is deprecated and no longer used by tests (all tests use `SetupTestTx` with per-test transaction rollback). Only `dropOrder` matters for the fresh-database-per-run model.

**If you forget:** The test will fail with `relation "X" already exists` when `init-script.sql` tries to CREATE something that survived the DROP phase in `Migrate`. Caught immediately by running the test suite.

---

## 6. Writing Tests

### Basic Structure

```go
func TestMyHandler(t *testing.T) {
    t.Parallel()
    ctx, tx := testutils.SetupTestTx(t)

    // Use 'tx' for all DB operations
    userID, err := fixtures.CreateTestUser(tx)

    // Pass 'ctx' to request helpers to route through the tx
    w := makeRequest(handler, "GET", "/path", nil, token, ctx)

    // Assert against response and optionally query via tx directly
    var status string
    err = tx.QueryRow(ctx, "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&status)
}
```

### SetupTestTx Helper

`testutils.SetupTestTx` returns `(context.Context, db.Querier)`:
- Begins a PostgreSQL transaction on the raw pool (via `db.Conn.Pool()` — the PoolProxy escape hatch)
- Stores the transaction in the returned context via `db.ContextWithTx`
- Registers `t.Cleanup` to roll back the transaction automatically when the test ends
- The returned `tx` is a `db.Querier` interface (implemented by both `*pgxpool.Pool` and `pgx.Tx`)

### Fixtures

Use helpers from `crussell/testutils/fixtures`. They accept `db.Querier` so they work with both pool-level and transaction-level queries:

```go
userID, err := fixtures.CreateTestUser(tx)      // inside a test tx
serviceID, err := fixtures.CreateTestService(tx)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
```

### Context Threading

**Critical Rule:** The `ctx` from `SetupTestTx` must be passed through to request handlers. When a handler calls `db.Conn.Query(r.Context(), ...)`, the `PoolProxy` checks `r.Context()` for a transaction. If the context doesn't have one (e.g., `context.Background()` was passed to the request helper), the handler's DB calls go to the pool and **cannot see** data created in the test's transaction.

```go
// CORRECT — handler sees the test's data:
w := makeRequest(handler, "POST", "/api/bookings", body, token, ctx)

// WRONG — handler queries the pool, can't see tx data:
w := makeRequest(handler, "POST", "/api/bookings", body, token, context.Background())
```

For request helpers that don't accept context, inject it directly:
```go
req, _ := http.NewRequest("POST", "/path", body)
req = req.WithContext(ctx)  // inject tx context
```

### Request Helper Pattern

Most packages follow this variadic context pattern:
```go
func makeRequest(handler http.Handler, method, path string, body interface{}, token string, requestCtx ...context.Context) *httptest.ResponseRecorder {
    req := httptest.NewRequest(method, path, marshalBody(body))
    req.Header.Set("Authorization", "Bearer "+token)
    if len(requestCtx) > 0 {
        req = req.WithContext(requestCtx[0])  // routes through test tx
    }
    // ... serve handler ...
}
```

### Test Helpers per Package

| Package | Key Helpers |
|---------|------------|
| `bookings` | `makeRequest`, `makeAuthRequest`, `makeAdminRequest`, `serveChiHandler`, `serveAdminHandler` |
| `payments` | `makePaymentRequest`, `setupPaymentStatusTest(status)`, `setupTestDataPast(t)`, `setupDepositBookingPast(t)` |
| `admin` | `makeAdminRequest(handler, method, path, body, ctx)`, `makeUserRequest` |
| `scheduling` | Package-specific helpers in test files |

### Working Hours

Weekday mapping: `0=Monday, 6=Sunday` (converted from Go's `time.Weekday`). The DB convention differs from Go's (Go: 0=Sunday).

```go
dbWeekday := int((localStart.Weekday() + 6) % 7)  // Go Sunday=0 → DB 6
```

`SeedBaseline` (called in `TestMain`) seeds working hours as 08:00-20:00, all days open. Individual tests no longer need to call `seedDefaultWorkingHours`.

### Timezone in Tests

Tests no longer use `time.LoadLocation("Europe/London")`. The `nextWeekday()` helper was simplified — it no longer accepts a `*time.Location` parameter:

```go
// Before:
func nextWeekday(weekday time.Weekday, loc *time.Location) time.Time {
    now := time.Now().In(loc)
    ...
}

// After:
func nextWeekday(weekday time.Weekday) time.Time {
    now := time.Now()
    ...
}
```

All test times are generated as UTC (via `time.Now()` or explicit `time.Date(..., time.UTC)`). The `londonLocation` variable is no longer imported in test files. `Time.Date()` calls in tests now use `time.UTC` instead of `londonLocation` or `ukLocation`. This aligns with the backend's UTC-normalised timezone architecture where `clock.Now()` returns `time.Now().UTC()`.

### Cursor Pagination in Tests

Cursor values contain timestamps with `+` timezone offsets. Always URL-encode them:

```go
nextCursor := resp.NextCursor
req := httptest.NewRequest("GET", "/api/admin/bookings?cursor="+url.QueryEscape(nextCursor), nil)
```

### Parallelism Within a Package

Tests within a package **can and should use `t.Parallel()`**. Each test gets its own transaction via `SetupTestTx`, which rolls back when the test completes. No cross-test data contamination.

**The key enablers:**
1. `PoolProxy` routes DB calls through the per-test transaction via `TxFromContext(ctx)`
2. `SetupTestTx` registers `t.Cleanup` (not `defer`) for rollback — works even on test failure
3. Semaphore in `SetupTestTx` (cap 8) + per-pool `MaxConns=16` + `-parallel 8` prevents connection pool exhaustion

**Tests can and should use `t.Parallel()`** wherever they use `SetupTestTx`. Per-test transaction isolation via `SetupTestTx` means no cross-test data contamination. The exceptions are cases where shared mutable state outside the database makes parallelism unsafe:

**Caveats — do NOT use `t.Parallel()` when:**
- Tests share package-level globals (fixed: `testAdminID` in `custom_services_test.go` was refactored to a parameter; `loginInProgress` map clearing was removed from auth tests)
- Tests hit rate limiters (auth login tests with rapid attempts — these have `t.Parallel()` but the shared `loginInProgress` map should NOT be bulk-cleared; fixed by removing the blanket clear)
- The handler uses a goroutine-unsafe library type as a global (fixed: `titleCaser` in `local.go` — `golang.org/x/text/cases.Caser` is not goroutine-safe, switched to per-call creation)
- Health check tests (`main_test.go`) mutate `db.Conn` directly

**Historical context — conn-busy bugs found via t.Parallel:**
When `t.Parallel()` was first added during migration, it revealed three pre-existing "conn busy" bugs in production handlers where `defer rows.Close()` + subsequent `db.Conn.QueryRow()` ran on the same transaction-routed context. These were all fixed:
- `handlers/admin/custom_services.go:GetCustomServices` — count query before rows loop
- `handlers/bookings/bookings.go:CreateBookingHandler` — `defer rows.Close()` before `db.Conn.Begin`  
- `handlers/bookings/bookings.go:AdminSearchBookings` — count query before rows loop

**Current t.Parallel() coverage:**
Nearly all test files using `SetupTestTx` also use `t.Parallel()`. Exceptions (no `t.Parallel()`): some health check tests, dev-only mocks, and pure unit tests without DB interaction.

### SeedBaseline

`testdb.SeedBaseline(pool)` is called in `TestMain` and seeds:
- **Working hours**: Mon-Sun, 08:00-20:00, all open
- **Business settings**: Business name, address, gift card config

Individual tests should NOT seed working hours manually — they're visible to all per-test transactions because they were committed to the pool before any test started.

### Connection Pool & Semaphore

Tests use a per-package semaphore (cap 8) in `SetupTestTx` and `MaxConns=16` per pool to prevent connection exhaustion:

```go
// testutils/tx.go
var txSemaphore = make(chan struct{}, 8)  // max concurrent test transactions
```

```go
// testutils/testdb/testdb.go
poolCfg.MaxConns = 16
```

The test runner also uses `-parallel 8` to limit concurrent test execution within each package. These three numbers (semaphore cap, MaxConns, -parallel flag) should stay in sync.

### SimpleProtocol

Test pools use `QueryExecModeSimpleProtocol` to disable prepared statements. Always set this in the test database config:

```go
config.ConnConfig.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol
```

---

## 7. What Can Go Wrong & How to Fix It

### Test suite hangs on startup (no output)

**Likely cause:** Stale `crussell_test_*` database from a killed test run has active connections, preventing `CreateTestDatabase` from dropping it.

**Fix:**
```bash
docker exec postgres psql -U myuser -d mydb -t -c "
  SELECT pg_terminate_backend(pid)
  FROM pg_stat_activity
  WHERE datname LIKE 'crussell_test%' AND pid != pg_backend_pid();
"
```

Or drop and recreate the admin DB connection:
```bash
docker exec postgres psql -U myuser -d mydb -c "
  SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname LIKE 'crussell_test%';
  DROP DATABASE IF EXISTS crussell_test_handlers_bookings;
"
```

### `database "crussell_test_*" is being accessed by other users`

`DestroyTestDatabase` failed to drop the database because connections lingered after `pool.Close()`. The retry loop in `DestroyTestDatabase` handles this, but if it exhausts 3 attempts, it logs a warning and moves on. The stale database is cleaned up by the next test run's `CreateTestDatabase`.

If you see this repeatedly, increase the retry count in `DestroyTestDatabase`:
```go
for attempt := 0; attempt < 5; attempt++ { ... }
```

### Tests fail after schema changes

**Old schema objects survive in the test database.** `CreateTestDatabase` creates a fresh database each time, so this shouldn't happen. But if you changed `init-script.sql` while a test binary was running, or if Docker volumes persist stale data:

```bash
# Nuclear option — drop ALL test databases
docker exec postgres psql -U myuser -d mydb -t -c "
  SELECT datname FROM pg_database WHERE datname LIKE 'crussell_test%';
" | grep crussell_test | while read db; do
  docker exec postgres psql -U myuser -d mydb -c "
    SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '$db';
    DROP DATABASE \"$db\";
  "
done
```

### `relation "X" already exists` or `type "X" already exists`

You added a new table/type to `init-script.sql` but forgot to add it to the drop lists in `testdb.go`. See §5.

### Transaction is aborted (SQLSTATE 25P02)

A PostgreSQL query inside a test's transaction failed, putting the transaction in an aborted state. All subsequent queries on the same `tx` will fail with this error. Common causes:
- FK violation from a subquery returning NULL
- Invalid data format
- The `tx` was used after a `t.Fatalf` (which panics the goroutine) — fix: use `t.Cleanup` instead of `defer tx.Rollback()`

Fix: identify and fix the root cause query. The error message usually includes the original failure. Wrap the failing query with detailed error logging, or run the test with `-v` to see all log output.

### `conn busy` errors in handler output

When tests use `SetupTestTx` (transaction-routed context), the handler's `db.Conn.Query(r.Context(), ...)` + `defer rows.Close()` followed by `db.Conn.QueryRow(r.Context(), ...)` causes pgx to return `conn busy`. This happens because the transaction's connection is held by the open (but consumed) rows object.

**Root cause:** The production handler runs a count query BEFORE consuming the data query's result set. With `SetupTestTx`, both queries route through the same transaction. pgx requires the first query's rows to be closed before executing the next query on the same transaction.

**Fix:** Move the count query after the `for rows.Next()` loop, or replace `defer rows.Close()` with `rows.Close()` immediately after the loop. Both patterns fix the issue. (Three handlers were already fixed — see `custom_services.go`, `bookings.go:CreateBookingHandler`, `bookings.go:AdminSearchBookings`.)

**Prevention:** When using `SetupTestTx` with handlers, verify the handler doesn't have `defer rows.Close()` followed by another `db.Conn.Query/QueryRow/Exec` on the same context without closing rows first.

### Tests pass in isolation but fail in full suite (within same package)

Likely parallel test interference. Even with per-test transactions, tests can interfere through:
1. **Package-level globals** (shared state like `testAdminID`)
2. **Rate limiters** (auth tests making rapid login attempts trigger progressive backoff)
3. **Webhook signature checks** (env vars set by one test affect parallel tests)

Fixes:
1. Remove the shared global or remove `t.Parallel()` from those specific tests
2. Remove `t.Parallel()` from rate-limited auth tests
3. Use `t.Setenv` correctly (runs in `t.Cleanup` so it's safe for parallel use)

### Deadlock detected

Should not happen with per-package databases. If it does, the cause is likely two concurrent parallel test transactions inserting into the same table with conflicting foreign key ordering (e.g., `INSERT INTO working_hours` without deterministic weekday ordering). Fix: make inserts order-deterministic or use `ON CONFLICT DO NOTHING/UPDATE`.

### OptionalAuth: invalid token: ... illegal base64 data

This is **expected** — the test `TestOptionalAuth_InvalidToken` sends a deliberately malformed token and the middleware logs the error. The **test passes.** This is just noisy output, not a failure.

### `Warning: Failed to delete CardDAV contact`

This appears in `TestAccount_DeleteGuest` and `TestLoyalty_Get`. The `dav.Service` is initialized as `&dav.BaseService{}` with a nil database connection. The CardDAV cleanup is a best-effort background goroutine. The **test passes.** This is harmless noise.

---

## 8. Performance

### Current Baseline

| Metric | Value |
|--------|-------|
| Quick check (`-count=1`) | **~13s** |
| Packages | 19 tested, 0 failures |
| Tests | ~1,642 passed, 0 failing |

New test additions in this batch:
| Test | Coverage |
|------|----------|
| `TestProgressBooking_DuplicateCompletion` | Calls ProgressBookingHandler twice with "completed" — verifies second call is idempotent (no extra stamps) |
| `TestProgressBooking_DailyStampCap` | Completes two bookings for the same user on the same day — verifies only 1 stamp awarded |
| `TestProgressBooking_InvalidTransitions` | Tests `no_show→completed` and `client_cancelled→in_progress` → both rejected with 400 |
| `TestBookings_Edit_SequentialEdit` | Calls EditBookingHandler twice with different start times — verifies both edits take effect (no stale-duration bug) |
| `TestBookings_TimezoneIndependence` | Creates a booking with a UTC time, verifies the stored and retrieved times match exactly with no timezone shift |
| `TestDeleteBooking_PastConfirmed_NoNoShow` | Cancels a past confirmed booking — verifies no retroactive no-show penalty via `startTime.After(clock.Now())` guard |
| `TestClosingTime_*` | 3 tests for closing hours validation (edge cases, error handling) |
| `TestContentType_*` | 2 tests for the new `JsonContentType` middleware |
| `TestAdminReserveSlot_*` (expanded) | New overlap coverage using `FOR UPDATE` inside transactions |
| `TestBuyGiftCard_*` (expanded) | Tests for pending-payment-first flow with VAT integration |
| `TestAdminCancelReservation_*` (12 new) | New admin reservation-cancel coverage: walkin + callin success, isolation, no-op, unauth, empty ctx, walkin+callin coexistence, anon untouched, response format parity with user handler, overlapping reservations deleted, user reservations untouched, idempotent double-cancel |
| `TestCancelReservation_DoesNotTouchAnonReservations` | Inverse-isolation test — user cancel ignores `RESERVATION:anon:%` (defensive — the WHERE clause only matches `RESERVATION:user:%`) |
| `TestCancelReservation_DoesNotTouchAdminReservations` | Inverse-isolation test — user cancel ignores `RESERVATION:admin:%`. Pairs with the admin-side test that verifies admin cancel ignores `RESERVATION:user:%`. Proves the two endpoints are properly partitioned. |

**Total tests:** ~1,642 passed across all packages. 0 failures. Growth driven by: coverage improvement pass (new test files for bookings, user, payments, giftcards, till, refunds, DAV, auth, middleware, validators, zxcvbn — 56 new files, coverage 50.4%→65.0%), VAT lifecycle and parallel-deadlock regression tests, savepoint/transaction-context pattern for time-sensitive tests, split-lunch detection tests, and removal of 10 dead test functions flagged by staticcheck U1000.

### What Drives Test Time

| Component | Time |
|-----------|------|
| `CREATE DATABASE` + migration per package | ~1s × 18 packages = ~4s (parallelized) |
| Test execution (heaviest: `bookings` ~200 tests) | ~25s |
| Test execution (admin ~180 tests) | ~17s |
| Test execution (payments ~150 tests) | ~6s |

### Speed Improvements Realized

| Change | Before | After | Saving |
|--------|--------|-------|--------|
| **Per-package databases** | ~90s serial | ~40s parallel | 55% |
| **`t.Parallel()` within packages** | ~40s parallel | ~25s parallel | 37% |
| **New tests (closing_time, middleware, expanded bookings)** | — | — | Added 30+ tests, marginal time impact |
| **Per-test transaction rollback by `SetupTestTx`** | — | Eliminates truncation overhead (~5-10s) | Included above |
| **`PreferSimpleProtocol` on test pool** | — | Eliminates prepared statement "conn busy" | Required for parallelism |

### Bottleneck

The `CREATE DATABASE` operation serializes at the PostgreSQL catalog level. With 18+ packages creating databases concurrently, they queue on catalog locks. This adds ~4-8s of overhead.

### How to Make It Even Faster

| Approach | Gain | Effort |
|----------|------|--------|
| **Template databases** (clone via `CREATE DATABASE ... TEMPLATE`) | ~4-8s saved on database creation | Low — was attempted, needs care |
| **Shard heaviest package** (split bookings into sub-packages) | ~10-12s saved | High — significant refactor |
| **Eliminate deferred `fixtures.Delete*` calls** (they waste tx operations before rollback) | ~1-2s | Low — sed removal |

---

## 9. FAQ

### Q: Can I run `go test ./pkg1/ & go test ./pkg2/ &` in separate terminals?

**Yes.** Each package gets its own database. No shared state, no lock contention. This is the entire point of the per-package database architecture.

### Q: Why don't all tests use `t.Parallel()`?

Most test files using `SetupTestTx` now also use `t.Parallel()`. The exceptions that intentionally lack `t.Parallel()`:

1. **Health check tests** (`main_test.go`) — mutate global `db.Conn` directly, incompatible with parallelism
2. **Dev-only Square mocks** (`square_dev_test.go`) — each test creates its own MockClient, already parallel-safe
3. **Some pure unit tests** (no DB interaction) — don't need `SetupTestTx` or `t.Parallel()`

Previously, shared mutable globals (`testAdminID` in `custom_services_test.go`, `titleCaser` in `local.go`, blanket `loginInProgress` map clearing) prevented parallelism. All of these have been fixed:
- `testAdminID` → refactored to a function parameter
- `titleCaser` → switched from global to per-call creation (`cases.Caser` is not goroutine-safe)
- `loginInProgress` blanket clear → removed from individual tests (each test creates a unique user)

### Q: What happens if `go test` is killed mid-run?

Stale `crussell_test_*` databases are left behind. The next full test run cleans them up in `CreateTestDatabase` (which drops the old database before creating a new one). The cleanup script in `local-dev-2.sh` also drops all `crussell_test*` databases at startup.

If a database can't be dropped because of stale connections, `DestroyTestDatabase` tries `pg_terminate_backend` + retry loop. If that fails after 3 attempts, it logs a warning and moves on. The database is cleaned up on the next run.

### Q: Why is `testdb.Migrate` called only once per package run?

It's called inside `CreateTestDatabase`, which is called in `TestMain`. Since each package gets a fresh database per test binary run, migration only happens once. Individual tests don't call `Migrate` — they use `SetupTestTx` with per-test transaction rollback, so no cleanup between tests is needed.

### Q: Do I need to worry about advisory locks?

**No.** Advisory locks (`pg_advisory_lock`) have been removed from test infrastructure. With per-package databases and per-test transactions, there's no cross-package or cross-test contention. The single production use of `pg_advisory_lock` is in the payment handler (`handlers/payments/handlers.go:706`) for serializing concurrent payment attempts on the same booking — this is unrelated to test infrastructure.

### Q: What's the `GO_TESTING` env var for?

It's set by `local-dev-2.sh` and consumed in several places:
- `internal/square/square_dev.go`: `mockSleep(d)` skips the sleep when `GO_TESTING=1` (this is how the 4s checkout polling delay is bypassed)
- `handlers/auth/local.go`: zxcvbn password strength check is skipped (seeded passwords like "password" would otherwise be rejected)
- `main.go` init(): checks for `-test.` flags in os.Args (more precise than GO_TESTING) to skip JWT initialization during test runs

### Q: Why does `main.go` skip JWT init during tests?

The root `main.go` package's `init()` function reads `JWT_SECRET_KEY` and calls `auth.InitJWT()`. When the root package's tests run, `JWT_SECRET_KEY` may not be set. The `init()` function checks `os.Args` for `-test.` flags — if present, it's a test binary, and JWT init is handled by `testutils/jwt` which each test package imports.

This uses `strings.HasPrefix(arg, "-test.")` rather than the env var `GO_TESTING` because the env var can persist in the shell environment and leak into non-test contexts (like the seeding script that starts the actual server).

### Q: I added a new test package and it shows `[no test files]`

Make sure your test file has the `//go:build test` constraint and that you're passing the `test` build tag:

```bash
go test -tags "test,dev" ./path/to/package/
```

The `dev` tag is required by some packages (Square mock, rate limiter). Always use `-tags "test,dev"`.

### Q: Tests pass in isolation but fail in the full suite

This can still happen within a single package when tests run in parallel. Common causes:
1. **Package-level globals** (e.g., `testAdminID` set by one test, read by another)
2. **Rate limiters** (auth tests with rapid login attempts trigger progressive backoff)
3. **`t.Setenv`** — use `t.Setenv` which automatically restores in `t.Cleanup` (safe for parallel)
4. **Test order dependencies** (rare — one test creates data that another test expects)

Fix: remove the shared global or remove `t.Parallel()` from the affected test(s).

### Q: How do I debug a hanging test?

The hang is almost certainly in `CreateTestDatabase` or the first `resetTestData` call. Run with verbose output and a timeout:

```bash
timeout 30 go test -tags "test,dev" -v -count=1 ./mypackage/ 2>&1 | head -20
```

If you see no output at all for 30s, the hang is in `CreateTestDatabase`. Check for stale database connections (see §7).

### Q: I see `WARN: No test DB available` from `auth/jwt_test.go`

This shouldn't appear anymore — the auth package's TestMain was updated to use `CreateTestDatabase` like all other packages. If you see it, the auth package TestMain wasn't updated. Fix: replace `testdb.NewPool("")` with `testdb.CreateTestDatabase("crussell_test_auth")`.

### Q: What's the total test count?

~1,642 tests run across all packages. 0 failures.

**Notable new tests:** Centralised job scheduler tests (3 — RegisterAll count, schedules, handler signatures), scheduled-cleanup handler tests (21 — NotifyUnpaidOneWeek/Month, TransitionDiscountCampaigns, CleanupExpiredVerificationCodes/RefreshTokens), GDPR export cache cleanup (4), stale login entry cleanup (4), rate limiter cleanup tests (6), rate limiter production behavior tests (6). Duplicate completion guard (idempotent second `"completed"` call), daily stamp cap (two completions same day → 1 stamp), invalid status transitions (no-show→completed rejected with 400), sequential edit (two edits in sequence), timezone independence (UTC in, UTC out — no shift), past-booking no-show guard (past confirmed booking cancelled → `client_cancelled`, not `no_show`). New closing_time tests (3), content-type middleware tests (2), clock package tests, expanded admin reserve overlap tests, expanded gift card buy flow tests with VAT, and full admin reservation cancel coverage (12 tests covering walkin + callin + isolation + no-op + idempotency + response format parity).

### Q: Why use `-count=10` for thorough verification?

A single `-count=1` pass confirms no compilation errors and basic correctness. `-count=10` (10 iterations per test) catches flakiness from:
- **Goroutine-unsafe globals** used concurrently under `t.Parallel()` — the `titleCaser` panic (`slice bounds out of range` in `golang.org/x/text/cases`) only appeared ~1 in 10 runs
- **Timing-dependent mocks** — the Square mock's 3s async completion vs test polling timeout
- **Shared state races** — `loginInProgress` map clearing under parallel auth tests

**Workflow:**
```
-count=1  → iterative dev (fast, ~9s)
-count=10 → feature completion (thorough, ~39s)
```

Both must pass before considering a change complete.
