docs: update README, dev scripts, and documentation

Update project documentation and development scripts.

- Update README test counts (953/957 passing, 8 skipped)
- Simplify dev script: remove test DB seeding, add name history creation,
  clean up stale test databases on startup, remove -p 1 test flag
- Update obsidian documentation for new features:
  - Name history system docs
  - Referral discount system docs
  - Database migration docs (CHAR(12) short IDs)

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
2026-06-20 16:59:56 +01:00
co-authored by Sisyphus
parent 39ac944994
commit f2e8e3eb77
6 changed files with 345 additions and 246 deletions
+1 -1
View File
@@ -74,7 +74,7 @@ Default logins (password: `password`):
```bash ```bash
cd backend && go build -o bin/backend ./main.go cd backend && go build -o bin/backend ./main.go
cd frontend && npm ci && npm run build cd frontend && npm ci && npm run build
cd backend && go test -tags "test,dev" -p 1 ./... # 803/806 passing, 3 skipped cd backend && go test -tags "test,dev" ./... # 953/957 passing, 8 skipped
``` ```
## Full Documentation ## Full Documentation
+31 -27
View File
@@ -70,33 +70,18 @@ log_step "Allowing PostgreSQL to fully initialize..."
sleep 5 sleep 5
# --- 3b. Create Test Database --- # --- 3b. Create Test Database ---
log_step "Setting up test database (crussell_test)..." log_step "Cleaning up stale test databases..."
docker exec postgres psql -U myuser -d mydb -c "CREATE DATABASE crussell_test;" 2>&1 || log_info "Test database may already exist" docker exec postgres psql -U myuser -d mydb -t -c "
sleep 2 SELECT datname FROM pg_database WHERE datname LIKE 'crussell_test%';
" 2>&1 | grep crussell_test | while read -r dbname; do
# --- 3c. Seed Test Database Schema --- docker exec postgres psql -U myuser -d mydb -c "
log_step "Seeding test database schema..." SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '$dbname' AND pid != pg_backend_pid();
docker exec -i postgres psql -U myuser -d crussell_test < init-scripts/init-script.sql 2>&1 | head -5 || true DROP DATABASE IF EXISTS \"$dbname\";
log_success "Test database schema seeded" " 2>&1
# --- 3d. Verify test database is ready ---
log_step "Verifying test database readiness..."
MAX_ATTEMPTS=10
ATTEMPT=0
while [ $ATTEMPT -lt $MAX_ATTEMPTS ]; do
if docker exec postgres psql -U myuser -d crussell_test -c "SELECT 1;" > /dev/null 2>&1; then
log_success "Test database ready"
break
fi
ATTEMPT=$((ATTEMPT+1))
sleep 1
done done
if [ $ATTEMPT -eq $MAX_ATTEMPTS ]; then log_success "Test databases cleaned"
log_error "Test database failed to initialize after $MAX_ATTEMPTS attempts"
exit 1
fi
# --- 3e. Rustfs (wipe data for fresh start) --- # --- 3c. Rustfs (wipe data for fresh start) ---
log_step "Wiping Rustfs (S3 storage)..." log_step "Wiping Rustfs (S3 storage)..."
docker compose stop rustfs > /dev/null 2>&1 || true docker compose stop rustfs > /dev/null 2>&1 || true
docker compose rm -f rustfs > /dev/null 2>&1 || true docker compose rm -f rustfs > /dev/null 2>&1 || true
@@ -1426,7 +1411,26 @@ fi
echo "${C_GREEN}✅ Created $edit_req_count Edit Requests${C_RESET}" echo "${C_GREEN}✅ Created $edit_req_count Edit Requests${C_RESET}"
# ===========================================================================
# 7c. NAME HISTORY (for testing "formerly" display)
# ===========================================================================
echo -e "\n${C_BLUE}📝 Creating name history entry for test user...${C_RESET}"
USER_TOKEN=$(login "user@example.com" "password")
if [[ -n "$USER_TOKEN" ]]; then
name_resp=$(curl -s -w "\n%{http_code}" -X PUT \
-H 'Content-Type: application/json' \
-H "Authorization: Bearer $USER_TOKEN" \
-d '{"firstName":"Regular","lastName":"Joe","phone":"+447000000001"}' \
"$BASE_URL/user/profile")
name_code=$(echo "$name_resp" | tail -n1)
if [[ "$name_code" =~ ^2 ]]; then
echo "${C_GREEN}✅ Name history entry created (Regular User → Joe shows as 'formerly')${C_RESET}"
else
echo "${C_YELLOW}⚠️ Failed to update user name (HTTP $name_code)${C_RESET}"
fi
else
echo "${C_YELLOW}⚠️ Could not log in as test user for name change${C_RESET}"
fi
# =========================================================================== # ===========================================================================
# SUMMARY # SUMMARY
@@ -1471,7 +1475,7 @@ cd /home/popertots/Crussell/backend
export POSTGRES_USER POSTGRES_PASSWORD POSTGRES_HOST POSTGRES_DB GO_TESTING=1 export POSTGRES_USER POSTGRES_PASSWORD POSTGRES_HOST POSTGRES_DB GO_TESTING=1
TEST_OUTPUT_FILE=$(mktemp) TEST_OUTPUT_FILE=$(mktemp)
START_TIME=$(date +%s) START_TIME=$(date +%s)
go test -tags "test,dev" -v -p 1 -count=1 ./... 2>&1 | tee "$TEST_OUTPUT_FILE" || true go test -tags "test,dev" -v -count=1 ./... 2>&1 | tee "$TEST_OUTPUT_FILE" || true
END_TIME=$(date +%s) END_TIME=$(date +%s)
DURATION=$((END_TIME - START_TIME)) DURATION=$((END_TIME - START_TIME))
MINUTES=$((DURATION / 60)) MINUTES=$((DURATION / 60))
@@ -14,7 +14,7 @@ These are blockers: missing functionality that prevents daily operations, legal
| # | Gap | Effort | Area | Notes | | # | Gap | Effort | Area | Notes |
|---|---|---|---|---| |---|---|---|---|---|
| 1 | **CurrentAppointment action stubs** | M (1d) | Frontend | `Extend` and `Cancel` buttons on Today page are dead. Staff cannot cancel or extend an in-progress appointment from the Today page. Edit, Take Payment, and Reschedule are already wired. | | 1 | **CurrentAppointment action stubs** | M (1d) | Frontend | `Extend` and `Cancel` buttons on Today page are dead. Staff cannot cancel or extend an in-progress appointment from the Today page. Edit, Take Payment, and Reschedule are already wired. |
| 2 | **Reservation/anonymization background cron** | S (2-3h) | Backend | `CleanupOldReservations()`, `AnonymizeStaleGuestAccounts()`, `CleanupExpiredGiftCards()`, `CleanupIdleAccounts()`, `CleanupExpiredFinancialRecords()` all run on `GET /api/availability`. If no one fetches availability for days, expired reservations persist and stale guest data isn't anonymized. Should be a background ticker in `main.go` (or a lightweight cron job). | | 2 | **Reservation/anonymization background cron** | S (2-3h) | Backend | `CleanupOldReservations()`, `AnonymizeStaleGuestAccounts()`, `CleanupExpiredGiftCards()`, `CleanupIdleAccounts()`, `CleanupExpiredFinancialRecords()`, `CleanupOldNameHistory()` all run on `GET /api/availability`. If no one fetches availability for days, expired reservations persist and stale guest data isn't anonymized. Should be a background ticker in `main.go` (or a lightweight cron job). |
| 3 | **VAT/Tax export endpoints** | M (1-2d) | Backend | `get_vat_return_data()` and `export_sales_transactions()` SQL functions exist. No admin API to trigger them. Needed for HMRC Making Tax Digital compliance. | | 3 | **VAT/Tax export endpoints** | M (1-2d) | Backend | `get_vat_return_data()` and `export_sales_transactions()` SQL functions exist. No admin API to trigger them. Needed for HMRC Making Tax Digital compliance. |
| 4 | **Password reset flow** | S (2-3h) | Frontend | Backend has `/api/verify/generate` and `/api/verify/check`. Login page has no "forgot password" link or form. Customers who forget their password must call the salon. | | 4 | **Password reset flow** | S (2-3h) | Frontend | Backend has `/api/verify/generate` and `/api/verify/check`. Login page has no "forgot password" link or form. Customers who forget their password must call the salon. |
| 5 | **Email verification flow** | S (2-3h) | Frontend | Users register with `unverified_email` role. No UI to enter verification code or resend. `+layout.svelte` has an alert-based prototype that needs to be wired properly. | | 5 | **Email verification flow** | S (2-3h) | Frontend | Users register with `unverified_email` role. No UI to enter verification code or resend. `+layout.svelte` has an alert-based prototype that needs to be wired properly. |
+4 -4
View File
@@ -204,15 +204,15 @@ npm run dev # Dev server with HMR
```bash ```bash
cd backend cd backend
go test -tags "test,dev" -p 1 -count=1 ./... # 782/785 passing, 3 skipped go test -tags "test,dev" ./... # 953/957 passing, 8 skipped
go test -tags "test,dev" -v -run TestName ./... # Single test go test -tags "test,dev" -v -run TestName ./... # Single test
``` ```
Test infrastructure notes: Test infrastructure notes:
- Shared test database (`crussell_test`), sequential execution (`-p 1`) - **Per-package databases:** Each package gets its own `crussell_test_*` database, created in `TestMain` via `CreateTestDatabase()`. Enables parallel execution (`-p` defaults to `GOMAXPROCS`).
- **⚠️ Build tag:** Always use `-tags "test,dev"`. The `dev` tag is required by Square mock (`internal/square/square_dev.go`) and rate limiter (`mw/ratelimit_dev.go`). Without it, handlers/payments and handlers/bookings tests are silently skipped.
- `TestMain` per package — schema migration runs once per package - `TestMain` per package — schema migration runs once per package
- `TruncateTables()` between tests — `TRUNCATE TABLE ... CASCADE` (~60% faster than DROP+CREATE) - `TruncateTables()` between tests — single `TRUNCATE TABLE ... CASCADE` statement (down from 44 separate truncates), no advisory locks
- Advisory locks: `pg_advisory_lock(1337)` protects migration DDL, `pg_advisory_lock(1338)` prevents CASCADE deadlocks
- Statement-by-statement SQL parser (`splitSQLStatements()`) respects dollar-quoted PL/pgSQL blocks - Statement-by-statement SQL parser (`splitSQLStatements()`) respects dollar-quoted PL/pgSQL blocks
- Build tag: all test files use `//go:build test` - Build tag: all test files use `//go:build test`
- Test JWT secret: `test-secret-key-for-testing-only` - Test JWT secret: `test-secret-key-for-testing-only`
+5 -4
View File
@@ -692,7 +692,7 @@ pending/confirmed ──(deadline passed)──→ pending_release ──(slot c
2. Subtracting existing bookings (with gap logic) 2. Subtracting existing bookings (with gap logic)
3. Subtracting time blockers (including reservations) 3. Subtracting time blockers (including reservations)
4. **Late night lock**: After 22:00, blocks next morning 00:00-11:00 for non-admin users 4. **Late night lock**: After 22:00, blocks next morning 00:00-11:00 for non-admin users
5. Triggers `CleanupOldReservations()`, `AnonymizeStaleGuestAccounts()`, `CleanupExpiredLoyaltyRedemptions()`, `CleanupExpiredFinancialRecords()`, `CleanupExpiredGiftCards()`, `CleanupIdleAccounts()` 5. Triggers `CleanupOldReservations()`, `AnonymizeStaleGuestAccounts()`, `CleanupExpiredLoyaltyRedemptions()`, `CleanupExpiredFinancialRecords()`, `CleanupExpiredGiftCards()`, `CleanupIdleAccounts()`, `CleanupOldNameHistory()`
**Time Blockers:** Can be one-off (no cron) or recurring (cron expression). Cron expansion via `robfig/cron/v3` parser. **Time Blockers:** Can be one-off (no cron) or recurring (cron expression). Cron expansion via `robfig/cron/v3` parser.
@@ -1124,15 +1124,16 @@ Files with this pattern: `bookings.go` (4 handlers), `custom_services.go`, `user
### Test Configuration ### Test Configuration
- **Shared test database:** `crussell_test` - **Per-package databases:** Each package gets its own `crussell_test_*` database, created in `TestMain` via `CreateTestDatabase()`
- **Sequential execution:** `go test -p 1` - **Parallel execution:** `-p` defaults to `GOMAXPROCS` — packages run concurrently against their own databases
- **⚠️ Build tag:** Always `-tags "test,dev"`. Without `dev`, `square_dev.go` (mock client) and `ratelimit_dev.go` are excluded, causing `handlers/payments` and `handlers/bookings` tests to be silently skipped.
- **Build tag:** `//go:build test` - **Build tag:** `//go:build test`
- **Test secret:** `test-secret-key-for-testing-only` - **Test secret:** `test-secret-key-for-testing-only`
- **Fixtures:** Auto-generate unique emails using `fmt.Sprintf("test-%d-%d@example.com", time.Now().UnixNano(), rand.Int())` - **Fixtures:** Auto-generate unique emails using `fmt.Sprintf("test-%d-%d@example.com", time.Now().UnixNano(), rand.Int())`
### Test Coverage ### Test Coverage
**633/636 tests passing** (3 skipped) across 14+ packages. **953/957 tests passing** (8 skipped) across 18 packages.
| Package | Coverage Area | | Package | Coverage Area |
|---------|--------------| |---------|--------------|
@@ -1,123 +1,201 @@
# Testing Architecture & DB Management # Testing Architecture & DB Management
**Last Updated:** June 2026 **Last Updated:** June 2026 (major revision — per-package databases, parallel execution)
--- ---
## 1. Running 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 ### Environment Variables
| Var | Purpose | Example | | Var | Purpose | Required? |
|-----|---------|---------| |-----|---------|-----------|
| `TEST_DB_DSN` | Connection string for the test database | `postgres://myuser:mypassword@localhost:5432/crussell_test?sslmode=disable` | | `GO_TESTING` | Suppresses artificial delays in Square mock; disables zxcvbn password checks in RegisterHandler | **Yes** for dev server & tests |
| `JWT_SECRET_KEY` | Signs test JWT tokens | `my_test_secret_key` | | `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) |
| `GO_TESTING` | Triggers mock clients (Square, etc.); suppresses artificial delays in Square mock | `1` |
`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 ### Commands
```bash ```bash
# Full suite (sequential — required to avoid deadlocks) # Full suite (parallel — packages run concurrently against their own databases)
go test -tags "test,dev" -p 1 -count=1 ./... go test -tags "test,dev" -count=1 ./...
# Single package # Single package
go test -tags "test,dev" -v -count=1 ./handlers/payments/ go test -tags "test,dev" -v -count=1 ./handlers/payments/
# Single test # Single test
go test -tags "test,dev" -v -run TestAcquirePaymentLock_Confirmed ./handlers/payments/ 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
``` ```
`JWT_SECRET_KEY=test-secret-key` is required for the root package tests (main.go init() reads JWT_SECRET_KEY). 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**.
The `-p 1` flag serialises packages so their `TestMain` migrations don't clash. **Never omit it.** ### 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. Tests run, each starting with `resetTestData(t)` which does a **single `TRUNCATE ... CASCADE`** across all tables
5. `TestMain` calls `DestroyTestDatabase` which closes the pool and drops the database
**Duration:** Full suite ~40s (parallel), ~90s (serial `-p 1`)
--- ---
## 2. Database Setup ## 3. Package Setup — `TestMain` Pattern
### Schema Lifecycle Every package with integration tests follows this exact pattern:
Every package that runs integration tests has its own `TestMain` that:
1. Creates a connection pool via `testdb.NewPool("")`
2. Calls `testdb.Migrate(t, pool)` — drops everything (tables + enums) in CASCADE order, then re-runs `init-scripts/init-script.sql` statement-by-statement
3. Assigns `db.DB = pool` for the handlers
Each individual test calls `resetTestData(t)` which **truncates** all tables (CASCADE), leaving the schema intact.
### What You Need To Do When Adding a New Table or Enum
| Change | File to Edit | What to Add |
|--------|-------------|-------------|
| New table | `backend/testutils/testdb/testdb.go` — append table name to **beginning** of `dropOrder` array AND to `tables` list in `TruncateTables` | The table's Go identifier |
| New enum type | `backend/testutils/testdb/testdb.go` — add to `typeDrops` array | `"DROP TYPE IF EXISTS new_type CASCADE"` |
| New `booking_status` value | `init-scripts/init-script.sql` — update `CREATE TYPE booking_status AS ENUM (...)` | The new value in the list |
If you forget these, `testdb.Migrate` will fail with `relation "X" already exists` or `type "X" already exists` because the old objects survive between runs.
---
## 3. Writing Tests — Patterns That Work
### Basic Structure
```go ```go
//go:build test //go:build test
func TestMyHandler(t *testing.T) { package mypackage
resetTestData(t) // only when crossing packages or testing schema-level changes
// seed fixtures, build request, call handler, assert import (
"os"
"testing"
"crussell/db"
"crussell/testutils/jwt"
"crussell/testutils/testdb"
)
func TestMain(m *testing.M) {
pool := testdb.CreateTestDatabase("crussell_test_mypackage")
db.DB = pool
jwt.Init()
// Any additional global setup (Square client, DAV service, etc.)
code := m.Run()
testdb.DestroyTestDatabase(pool, "crussell_test_mypackage")
os.Exit(code)
}
func resetTestData(t *testing.T) {
t.Helper()
testdb.TruncateTables(t, db.DB)
} }
``` ```
**Don't reset the DB if you don't need to.** Full truncation (`resetTestData`) is expensive — it acquires advisory locks, cascades through every table, and re-seeds working hours. Prefer creating fresh entities per test: ### Database naming convention
```go `crussell_test_<package_path_with_underscores>`
// Fast: create a new user + booking for each test, rely on unique IDs
userID, err := fixtures.CreateTestUser(db.DB) | Package path | Database name |
bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, someTime) |---|---|
| `.` (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). Use the naming convention for the database name.
3. **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
``` ```
This is safe as long as tests in the same package don't assert on global counts (total users, total bookings) and don't mutate shared data in conflicting ways. Each test's entities get their own IDs, so they never collide. ### What It Creates Automatically
**`resetTestData(t)` is required when:** - Each package's first test run: `CreateTestDatabase` drops old DB → creates new DB → runs `init-script.sql` migration (~1s)
- Testing schema changes or enum value drops - Subsequent runs: same flow but creates fresh database each time
- Testing global aggregate queries (total bookings, revenue sums)
- A previous test may have left state that would cause a false positive/negative (ambiguous — use your judgement)
- Switching packages (each `TestMain` calls `testdb.Migrate` which drops everything)
**`resetTestData(t)` is wasteful when:** ---
- Every test creates its own user + booking via fixtures
- Tests only read or update their own rows ## 5. Adding a New Table or Type to the Schema
- The only shared table is `working_hours` (seed once in the first test, or use `ON CONFLICT DO UPDATE`)
When you modify `init-scripts/init-script.sql` to add tables or types, you must update the teardown lists in `backend/testutils/testdb/testdb.go`:
| Change | What to Do |
|--------|-----------|
| **New table** | Add to `dropOrder` array AND to the `TRUNCATE TABLE ... CASCADE` statement in `TruncateTables` |
| **New enum type** | Add to `typeDrops` array in `Migrate` |
| **New sequence** | Add to `seqDrops` |
| **Removed table/type** | Remove from the corresponding lists |
**If you forget:** The test will either:
- Hang on the second run when `Migrate` tries to `DROP TYPE ... CASCADE` and fails because a table that depends on the type isn't in the drop chain
- Fail with `relation "X" already exists` when `init-script.sql` tries to CREATE something that survived the DROP phase
Both are caught immediately by running the test suite twice.
---
## 6. Writing Tests
### Basic Structure
```go
func TestMyHandler(t *testing.T) {
resetTestData(t) // clean slate
// create fixtures, make request, assert
}
```
Every test should call `resetTestData(t)` at the start. It's cheap now — a single `TRUNCATE TABLE ... CASCADE` statement, no advisory lock.
### Fixtures
Use helpers from `crussell/testutils/fixtures`:
```go
userID, err := fixtures.CreateTestUser(db.DB)
serviceID, err := fixtures.CreateTestService(db.DB)
bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
```
### Test Helpers per Package ### Test Helpers per Package
Each test package defines its own helpers. Look at the existing test file before writing new ones: | Package | Key Helpers |
|---------|------------|
| `bookings` | `makeRequest`, `makeAuthRequest`, `makeAdminRequest`, `serveChiHandler`, `serveAdminHandler` |
| `payments` | `makePaymentRequest`, `setupPaymentStatusTest(status)`, `setupTestDataPast(t)`, `setupDepositBookingPast(t)` |
| `admin` | `makeAdminRequest`, `makeUserRequest` |
| `scheduling` | Package-specific helpers in test files |
| Package | Helper File | Key Functions | ### Working Hours
|---------|------------|---------------|
| `bookings` | `bookings_test.go` | `makeRequest`, `makeAuthRequest`, `makeAdminRequest`, `serveChiHandler`, `serveAdminHandler` |
| `payments` | `payments_test.go` | `makePaymentRequest`, `setupPaymentStatusTest(status)`, `setupTestDataPast(t)`, `setupDepositBookingPast(t)` |
| `admin` | `test_helpers.go` | `makeAdminRequest`, `makeUserRequest`, `resetTestData` |
| `scheduling` | (in test files) | Package-specific helpers |
Chi URL params need manual wiring. Use `serveChiHandler` or `serveAdminHandler` for routes with path params like `{id}`.
### Avoiding Payment-Split Headaches
`CreateBookingPayment` splits future-dated payments into deposit + non-deposit records. This causes payment-count assertions to fail if you aren't expecting it.
```go
// Booking in the past → no split (correct for idempotency / count tests)
userID, bookingID, _ := setupTestDataPast(t)
// Future booking → split fires (correct for verifying 2-record output)
userID, bookingID := setupDepositBooking(t)
```
Helpers live in `backend/handlers/payments/payments_test.go`.
### Seeding Working Hours
Weekday mapping: `0=Monday, 6=Sunday` (converted from Go's `time.Weekday`). Weekday mapping: `0=Monday, 6=Sunday` (converted from Go's `time.Weekday`).
@@ -130,178 +208,194 @@ if todayWeekday == 0 {
} }
``` ```
### Seeding Exceptional Hours (Holidays) ### Cursor Pagination in Tests
Cursor values contain timestamps with `+` timezone offsets. Always URL-encode them:
```go ```go
var groupID int nextCursor := resp.NextCursor
db.DB.QueryRow(ctx, `INSERT INTO exceptional_working_hours_groups (name) VALUES ('Test') RETURNING id`).Scan(&groupID) req := httptest.NewRequest("GET", "/api/admin/bookings?cursor="+url.QueryEscape(nextCursor), nil)
db.DB.Exec(ctx, `INSERT INTO exceptional_working_hours (group_id, weekday, start_time, end_time, is_open) VALUES ($1, $2, '00:00', '00:00', false)`, groupID, todayWeekday)
db.DB.Exec(ctx, `INSERT INTO exceptional_group_applications (group_id, week_start) VALUES ($1, $2::date)`, groupID, mondayStr)
``` ```
The column is `week_start`, **not** `monday_week_start`. ### Parallelism Within a Package
### Auth & JWT Tests Tests within a package run **sequentially** (no `t.Parallel()`). Each test calls `resetTestData` which does a global `TRUNCATE`, so parallel tests would corrupt each other's data. Making intra-package parallelism safe would require wrapping each test in its own transaction — a significant refactor.
The `crussell/auth` package now connects to the test database for JTI revocation tests. Its `TestMain` calls `testdb.NewPool("")` and `testdb.Migrate()`: ---
## 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 ```go
func TestMain(m *testing.M) { for attempt := 0; attempt < 5; attempt++ { ... }
InitJWT("test-secret-key-for-jwt-test")
pool, err := testdb.NewPool("")
if err != nil {
fmt.Fprintf(os.Stderr, "WARN: No test DB: %v\n", err)
} else {
testdb.Migrate(&testing.T{}, pool)
db.DB = pool
}
code := m.Run()
if pool != nil { pool.Close() }
os.Exit(code)
}
``` ```
Tests that need a database call `requiresDB(t)` which skips if `db.DB` is nil. ### Tests fail after schema changes
**Important:** `GO_TESTING=1` env var skips the server-side zxcvbn password strength check in `RegisterHandler`. Without it, seeded passwords like `"password"` are rejected (zxcvbn score 0). The dev server and CI should always set this. **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:
### Testing Pagination Caps
```go
// per_page=500 accepted
w := makeAdminRequest(handler, "GET", "/api/admin/bookings?per_page=500", nil)
// per_page=600 → capped to default 10
```
---
## 4. Current Status
- **All 16 packages pass**, 0 failed
- Skipped: `TestAdminNotifications_List` / `TestAdminNotifications_Acknowledge` — WIP, no handler yet (also `mw/ratelimit_test.go` skipped by `!dev` build tag)
- All packages pass clean: 16 tested packages (plus 10 with no test files)
- `GO_TESTING=1` suppresses the 1s/3s artificial delays in Square mock (`internal/square/square_dev.go`)
- New test files:
- `handlers/payments/loyalty_test.go` — 10 tests (apply-redemption + campaign auto-apply)
- `handlers/payments/refund_exclude_test.go` — 2 tests (discount/OTH exclusion)
- `handlers/bookings/dedup_test.go` — 8 tests (dedup guards + no-show)
- `handlers/payments/discount_preview_test.go` — 13 tests (discount preview endpoint)
- `handlers/payments/payment_status_test.go` — 32 tests (status guards, payment lock, release lock)
- `handlers/payments/refunds_test.go` — 20 tests (cancellation refund calc + processing)
- `handlers/bookings/deposit_test.go` — 30 tests (deposit fields, eviction, admin cancel)
- `handlers/webhooks/webhooks_test.go` — 13 tests (Square webhook signature + dispatch)
- `handlers/admin/discount_campaigns_test.go` — 11 tests (campaign CRUD + stats)
- `handlers/user/patch_tests_test.go` — 7 tests (user patch test list/delete)
- Extended tests: `handlers/scheduling/time_blockers_test.go` (CleanupExpiredLoyaltyRedemptions), `handlers/notifications/notifications_test.go` (AcknowledgePendingBookingNotification), `handlers/payments/payment_status_test.go` (ReleasePaymentLock)
- Schema changes: `patch_tests.id` now uses `generate_patch_test_id()` (was `generate_service_id()`); `user_patch_tests.id` changed from `BIGSERIAL` to `CHAR(12)` with `generate_user_patch_test_id()`
---
## 5. Debugging — Issues & Resolutions
### Test suite hangs on startup (0% CPU, no output)
**Why:** `testdb.TruncateTables` or `testdb.Migrate` blocks on a lock held by another session.
**Previously resolved by:**
- Killing the Go dev server (`go run ./main.go`) — it holds open connections
- Fixing leaked rows/transactions — `.Query()` without `rows.Close()` or `.Begin()` without commit/rollback leaves the connection in a bad state
- Enforcing `pool.Acquire()` instead of `pool.Exec()` for advisory locks — lock and work **must** use the same connection (see `testdb.go` TruncateTables for the pattern)
**What to do:**
```bash ```bash
docker exec postgres psql -U myuser -d crussell_test -c "SELECT pid, query, state FROM pg_stat_activity WHERE state != 'idle';" # Nuclear option — drop ALL test databases
``` docker exec postgres psql -U myuser -d mydb -t -c "
Kill stale connections: SELECT datname FROM pg_database WHERE datname LIKE 'crussell_test%';
```sql " | grep crussell_test | while read db; do
SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = 'crussell_test' AND pid != pg_backend_pid(); 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`
### Tests fail with `relation "X" does not exist` during migration You added a new table/type to `init-script.sql` but forgot to add it to the drop lists in `testdb.go`. See §5.
**Why:** Table A references Table B but A is created before B in `init-script.sql`. ### Deadlock detected
**Previously resolved by:** Moving inline FK constraints to `ALTER TABLE ADD CONSTRAINT` statements at the bottom of `init-script.sql`. Should not happen with per-package databases. If it does, the cause is likely two concurrent `TruncateTables` calls from the same test binary (within a package). This would require `t.Parallel()` which we don't use. If you're adding parallel tests, make sure they don't share database access.
**What to do:** Declare the column without a REFERENCES clause, then add the constraint at the end of the file. ### 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.
--- ---
### Tests fail with `relation "X" already exists` or `type "X" already exists` ## 8. Performance
**Why:** A new table or enum was added to the schema but not to the test teardown lists. ### Current Baseline
**Previously resolved by:** Adding the table to `dropOrder` + `tables` in `TruncateTables`, or the type to `typeDrops`, in `backend/testutils/testdb/testdb.go`. | Metric | Value |
|--------|-------|
| Serial (`-p 1`) | ~90s |
| **Parallel** | **~40s** |
| Packages | 18 tested, 0 failures |
| Tests | 949 passing, 8 skipped, 0 failing |
**What to do:** Follow the table in §2. If you see this for an existing object, check whether the test database has stale objects from a previous schema version — truncate + recreate manually. ### What Drives Test Time
| Component | Time |
|-----------|------|
| `CREATE DATABASE` + migration per package | ~1s × 15 packages = ~4s (parallelized) |
| Test execution (heaviest: `bookings` 245 tests) | ~25s |
| Test execution (admin 156 tests) | ~16s |
| Test execution (payments: 120+ tests) | ~13s |
### Bottleneck
The `CREATE DATABASE` operation serializes at the PostgreSQL catalog level. With 15+ packages creating databases concurrently, they queue on catalog locks. This adds ~4-8s of overhead in parallel mode.
### How to Make It Faster
| Approach | Gain | Effort |
|----------|------|--------|
| **Shard slow packages** (split `bookings` test files → parallel sub-packages) | 25s → ~13s (halve bookings) | Moderate — 2-3 hours refactoring |
| **Template databases** (create once, clone via `CREATE DATABASE ... TEMPLATE`) | ~4-8s saved on database creation | Low — was attempted, needs careful parallel-safety work |
| **`t.Parallel()` within packages** with transaction-per-test | ~25s → ~15s (bookings halved again) | High — major test refactor |
| **Reduce polling in Square mock** | Already done (0.012s, was 8s) | ✅ Complete |
--- ---
### Deadlock detected during `TRUNCATE` ## 9. FAQ
**Why:** Two concurrent `TruncateTables` calls each hold one end of a circular FK lock chain. ### Q: Can I run `go test ./pkg1/ & go test ./pkg2/ &` in separate terminals?
**Previously resolved by:** **Yes.** Each package gets its own database. No shared state, no lock contention. This is the entire point of the per-package database architecture.
- Adding `pg_advisory_lock(1338)` to serialise all truncation (lock ID 1338; migrate uses 1337 — must not collide)
- Enforcing `pool.Acquire()` so lock, work, and unlock share one connection
- Always using `-p 1` to serialise packages
**What to do:** ### Q: Why don't we use `t.Parallel()`?
1. Kill duplicate `go test` processes
2. Kill stale connections (see first FAQ)
3. Verify `TruncateTables` uses `pool.Acquire()` with lock 1338
4. Run with `-p 1`
--- Because `resetTestData(t)` does a global `TRUNCATE ... CASCADE`. Two parallel tests would truncate each other's data mid-flight. Fixing this requires either:
- Per-test transactions with rollback (wrap every DB operation in a test-scoped `Tx`, roll back at the end)
- Separate PostgreSQL schemas per test
### Tests get 0 results from queries using enum values Both are significant refactors. For now, intra-package parallelism isn't worth the complexity.
**Why:** You removed a value from a PG enum (e.g. `no_deposit` from `booking_status`) but a `WHERE status NOT IN (...)` query still references it. PG returns an error for enum literal checks against removed values — if the error is silently ignored (e.g. `_ = db.DB.QueryRow(...).Scan(&x)`), the target variable stays at Go zero value. ### Q: What happens if `go test` is killed mid-run?
**Previously resolved by:** Replacing all `'no_deposit'` references with the replacement status (`'deposit_lapsed'`) across: 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.
- `backend/handlers/today/today.go` (2 occurrences)
- `backend/handlers/bookings/bookings.go` (multiple) 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.
- `backend/handlers/bookings/manage.go`
- `backend/handlers/scheduling/default-hours.go` ### Q: Why is `testdb.Migrate` called only once per package run?
- Frontend `*.svelte` files referencing the old status name
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 call `TruncateTables` via `resetTestData`, which is now a single `TRUNCATE TABLE ... CASCADE`.
### Q: Do I need to worry about advisory locks?
**No.** Advisory locks (`pg_advisory_lock`) have been removed from both `Migrate` and `TruncateTables`. With per-package databases, there's no cross-package contention. The old lock IDs (1337, 1338) are no longer used.
### 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:
**What to do:** Grep the full codebase for the removed enum value before dropping it from the schema:
```bash ```bash
grep -rn "'old_value'" backend/ frontend/ | grep -v "_test.go" | grep -v ".md" 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"`.
### Panic: `interface conversion: interface {} is nil, not string` during auth ### Q: Tests pass in isolation but fail in the full suite
**Why:** Test request is missing auth context — token not set, or context key not wired. This was a common issue in the old architecture (shared `crussell_test` database). With per-package databases, **this should no longer happen.** Each package is fully isolated. If you see it, check:
1. Are two tests in the **same package** interfering? (Shared state, global variables)
2. Is the test environment different? (Different env vars, Docker state)
**Previously resolved by:** Ensuring `jwt.Init()` runs in `TestMain` and the test uses `makeAuthRequest` (or manually injects `mw.UserIDKey`/`mw.UserRoleKey` into the request context). ### Q: How do I debug a hanging test?
**What to do:** Look at how `makePaymentAuthRequest` in `payments_test.go` or `makeRequestWithContext` in `admin/test_helpers.go` sets up auth context. Copy that pattern. The hang is almost certainly in `CreateTestDatabase` or the first `resetTestData` call. Run with verbose output and a timeout:
---
### Cursor-paginated tests return 0 items on the second page
**Why:** The cursor contains a `+` timezone offset (e.g. `2026-06-17T12:49:32+01:00`) which, when passed unescaped in a URL query string, is decoded as a space. The timestamp parser then fails because it expects `Z07:00` format.
**Previously resolved by:** Using `url.QueryEscape(cursor)` when building the request URL, or — for tests that only need to verify basic listing — skipping the second-page assertion entirely.
**What to do:** If you need to test second-page results, URL-encode the cursor value. If the test only needs to verify the listing contract (total, per_page, presence of next_cursor), the first-page assertions are sufficient.
---
### Pre-existing test files don't compile
**Why:** Tests with `//go:build test` constraint are only compiled when the `test` build tag is active. `go build` without test tags skips them entirely, so compilation errors in test files are silent until you run `go test`.
**Previously resolved by:** Running `go test -tags "test,dev" -count=1 -run ^$ ./path/` — a zero-test run that still compiles all test files — as part of every PR verification.
**What to do:** After changing test files, always run a compile check:
```bash ```bash
go test -tags "test,dev" -count=1 -run ^$ ./package/... timeout 30 go test -tags "test,dev" -v -count=1 ./mypackage/ 2>&1 | head -20
``` ```
This catches test-only compilation errors without executing any tests.
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?
Current: ~957 tests (18 packages, 0 failures, 8 skipped).
Tracked by counting `^=== RUN` lines in the output (excluding sub-tests).