Crussell/.sisyphus/coverage-report.md

# Test Coverage Improvement Report

**Date**: 2026-07-10
**Overall Coverage**: 50.4% of statements
**Test Command**: `go test -tags "test,dev" -coverprofile=coverage.out -covermode=atomic ./...`

---

## Coverage by Package

| Package | Coverage | Status |
|---------|----------|--------|
| `handlers/webhooks` | 100.0% | ✅ |
| `internal/images` | 100.0% | ✅ |
| `internal/jobs` | 93.5% | ✅ |
| `mw` | 89.3% | ✅ (small gaps) |
| `internal/validators` | 82.9% | ✅ |
| `clock` | 80.0% | ✅ |
| `internal/square` | 74.4% | ✅ |
| `handlers/scheduling` | 73.1% | ✅ |
| `handlers/notifications` | 71.5% | ✅ |
| `handlers/admin` | 68.6% | ✅ |
| `handlers/auth` | 66.0% | ✅ |
| `db` | 66.7% | ✅ |
| `handlers/today` | 64.5% | ✅ |
| `handlers/payments` | 55.4% | 🟡 Moderate |
| `handlers/bookings` | 47.7% | 🟡 Low |
| `handlers/portfolio` | 48.9% | 🟡 Low |
| `auth` | 45.0% | 🟡 Low |
| `handlers/user` | 39.1% | 🔴 Very Low |
| `handlers/services` | 30.3% | 🔴 Very Low |
| `crussell (root)` | 5.4% | 🔴 Critical |
| `internal/dav` | 0.0% | ⚫ Zero |
| `internal/logutil` | 0.0% | ⚫ Zero |
| `internal/s3` | 0.0% | ⚫ Zero |
| `internal/zxcvbnjs` | 0.0% | ⚫ Zero |

---

## Critical Finding: Cross-Package Coverage Blind Spot

**Several functions show 0% in their own package but HAVE tests in the `admin` package.** Go's per-package coverage (`go test ./handlers/services/`) only counts tests within that package. Cross-package test calls (from `handlers/admin/`) don't count.

Affected handlers (all tested in `handlers/admin/` but show 0% in their own package):

| Function | Actual Coverage | Where Tested |
|----------|----------------|-------------|
| `handlers/services`: ToggleService, CreateServiceHandler, DeleteServiceHandler, AllServicesHandler | ✅ Tested | `admin/services_test.go` |
| `handlers/user`: GetAdminUserHandler, ListAdminUsersHandler, GetEligiblePatchTestServicesHandler, AddPatchTestHandler | ✅ Tested | `admin/users_test.go` |
| `handlers/bookings`: GetAllAdminBookingsHandler, GetAdminBookingHandler, GetAllBookingsByUserHandler, SearchAdminBookingsHandler, etc. | ❌ **Untested** | No admin tests written |
| `handlers/bookings`: AdminListPendingBookingsHandler, AdminGetInProgressBookingHandler | ❌ **Dead code** | No routes registered in main.go |

---

## Priority 1: Immediate Wins (<30 min each, no new dependencies)

### 1.1 `internal/logutil/logutil_test.go` — 70 lines, pure stdlib
**Functions**: `ColoredDuration(d time.Duration) string`, `ColoredRows(n int) string`
**What to test**:
- `ColoredDuration`: 3 branches (<500ms → green, <5s → yellow, >=5s → red)
- `ColoredRows`: singular (n=1) vs plural (n≠1)
- `NO_COLOR` env var behavior (ANSI codes vs empty strings)
**Lines added**: ~40

### 1.2 `internal/zxcvbnjs/zxcvbn_test.go` — 68 Go lines, 1 public function
**Function**: `Score(password string) (int, error)`
**What to test**:
- Known weak passwords (`"password"`, `"123456"`) → score 0-1
- Strong passwords → score 3-4
- Empty string → handle gracefully
- `sync.Once` lazy init works across repeated calls
**Lines added**: ~30

### 1.3 `auth/jwt_test.go` — Refresh token functions (already have handler-level tests)
**Functions**: `generateRefreshTokenString()`, `GenerateRefreshToken(ctx, userID, role)`, `VerifyRefreshToken(ctx, tokenString)`
**What to test**:
- `generateRefreshTokenString`: format (64-char hex) + uniqueness (100 calls)
- `GenerateRefreshToken`: stores in DB, returns non-empty token
- `VerifyRefreshToken`: verify + token rotation (2nd call fails), expired, revoked
**Pattern**: Already works in `handlers/auth/auth_test.go:TestRefreshToken_Generation` — adapt for package-level
**Test infra**: `auth/testmain_test.go` already sets up `InitJWT` + test DB pool — no setup needed

### 1.4 `handlers/bookings/repo_test.go` — 4 trivial DB helpers (~5 lines each)
**Functions**: `GetBookingStatus`, `GetBookingStartTime`, `BookingExists`, `CountUserBookingsInStatus`
**What to test**:
- Happy path: insert booking → call function → assert result
- Not-found: non-existent ID → assert error
**Pattern**: `ctx, tx := testutils.SetupTestTx(t)` → `fixtures.CreateTestUser(tx)` → `fixtures.CreateTestService(tx)` → `fixtures.CreateTestBooking(tx, userID, svcID)` → call function directly

### 1.5 `mw/response_test.go` — Trivial helpers
**Functions**: `RespondJSON(w, status, data)`, `RespondError(w, status, message)`
**What to test**:
- `RespondJSON`: basic write, error encoding, nil data
- `RespondError`: confirm JSON shape `{"error": "..."}`
- **Action**: `RespondError` is dead code (never called anywhere) — either remove it or test + start using it
**Ease**: 5-line pure functions, `httptest.ResponseRecorder` + standard assertions

### 1.6 `internal/dav/types_test.go` — Pure string builders
**Functions**: `GenerateICalEvent(EventInput) string`, `GenerateVCard(ContactInput) string`
**What to test**:
- All-day vs timed events, attendees list, special characters (iCal escaping)
- vCard formatting with various field combinations
- Edge cases: empty fields, long strings
**No DB needed**: Pure functions, no dependencies

---

## Priority 2: Medium Effort (follow existing test patterns, 1-2h each)

### 2.1 `handlers/services/services_test.go` — Jump 30% → ~90%
**What**: Move/add in-package tests for `ToggleService`, `CreateServiceHandler`, `DeleteServiceHandler`, `AllServicesHandler`
**Why**: These are already tested in `handlers/admin/services_test.go` — just need to replicate in the `services` package
**Pattern**: Add helper `makeAdminContextRequest` (5 lines setting `mw.UserIDKey` + `mw.UserRoleKey` on chi context like `admin/test_helpers.go`)
**Bonus**: Remove dead code on lines 200-208 of `services.go` (unreachable after unconditional `return`)

### 2.2 `handlers/user/*_test.go` — Jump 39% → ~55%
**What**: Add in-package tests for `GetAdminUserHandler`, `ListAdminUsersHandler`, `GetEligiblePatchTestServicesHandler`, `AddPatchTestHandler`
**Why**: Same cross-package issue — tested in `admin/users_test.go`
**Also add**:
- `CreateGuestUserHandler` success path (currently only validation failure tests)
- `processProfileImage` unit test with a real JPEG file in `testdata/`
- `GetAdminUserHandler` error paths (not-found, social login query error)

### 2.3 `handlers/bookings/admin_*_test.go` — Admin GET handler tests
**What**: Add HTTP handler tests for uncovered admin GET handlers in `bookings.go`:
- `GetAllAdminBookingsHandler`, `GetAdminBookingHandler`, `GetAllBookingsByUserHandler`
- `SearchAdminBookingsHandler`, `GetOverlappingBookings*`, `GetBookingsBy*Range*`
**Pattern**: `serveChiHandler` with manual admin context injection (already used in `overlap_test.go` and `admin_reserve_test.go`)
**Verify**: `AdminListPendingBookingsHandler` and `AdminGetInProgressBookingHandler` — confirmed unreachable (no routes). Either remove or add routes + tests.

### 2.4 `mw/ratelimit_test.go` — Middleware wrapper tests
**Functions** (dev stubs): `ProgressiveRateLimit`, `RateLimit`
**What to test**: Wrap handler with middleware in dev mode, verify pass-through behavior
**Pattern**: `httptest.NewRecorder` + `httptest.NewRequest` + standard HTTP test

---

## Priority 3: Heavier Effort (needs mocks or refactoring, 2-4h each)

### 3.1 `handlers/portfolio/images_test.go` — UploadImage (currently 7.5%)
**Blockers**: `s3.Client` is nil in tests → handler returns "Storage not configured" early
**Fix**: Mock `s3.Uploader` interface (already exists), set `s3.Client = &mockUploader{}` in test setup
**What to test**: Multi-part form with JPEG data, verify 200, verify DB record inserted with correct URLs
**Also**: `ListFilters` 31.7% — add tests for `?filter[category]=value` (dual-query path), `?tag=forest`, combined filters

### 3.2 `handlers/user/profile_test.go` — UploadProfilePictureHandler (8.8%)
**Blocker**: Same S3 nil issue as portfolio
**Fix**: Same mock approach — `s3.Client = &mockUploader{}`
**Also**: `updateCardDAV` (11.1%) — currently requires `DAV_BASE_URL` env var and HTTP server. Mock the function or use `httptest.NewServer`.

### 3.3 `internal/dav/shared_test.go` — DB-backed CardDAV operations
**Functions**: 14 `BaseService` methods (CRUD for calendars, contacts, events)
**Requires**: PostgreSQL test DB pool (existing pattern in `testutils/testdb/`)
**Pattern**: Create test fixtures (calendar, contact records), call methods, assert DB state
**Note**: Importing `dav` package triggers `service_dev.go`'s `init()` which tries to connect to Postgres. Either set `GO_TESTING` env var or construct `BaseService` directly.

### 3.4 `internal/s3/s3_test.go` — S3 storage
**Approach**: 
- Unit tests via `Uploader` interface mock
- `Connect()` integration test requires local S3 (RustFS/MinIO) → tag `//go:build integration`
- `GetURL` URL-builder logic can be tested independently (currently 0% on both build tags)

---

## Dead Code Found During Investigation

| Location | Function | Status |
|----------|----------|--------|
| `mw/response.go:19` | `RespondError` | **Defined but NEVER called** — remove or wire up |
| `bookings/manage.go:309` | `AdminListPendingBookingsHandler` | **No route registered** in main.go |
| `bookings/manage.go:320` | `AdminGetInProgressBookingHandler` | **No route registered** in main.go |
| `services/services.go:200-208` | Duplicate-key error block | **Unreachable** — after unconditional `return` on line 198 |

---

## Summary: Coverage Impact by Action

| Action | Estimated Coverage Gain |
|--------|----------------------|
| Fix cross-package gap (services) | +15-20% in `handlers/services` |
| Fix cross-package gap (user admin handlers) | +10-15% in `handlers/user` |
| Add repo.go tests | +2-3% in `handlers/bookings` |
| Add admin booking GET handler tests | +15-20% in `handlers/bookings` |
| Auth refresh token tests | +15-20% in `auth` |
| logutil + zxcvbnjs tests | 0% → 80-100% in those packages |
| RespondJSON test | 0% → 100% in `mw/response.go` |
| Portfolio S3 mock + tests | 7.5% → 60-70% in `handlers/portfolio` |
| dav/types pure func tests | 0% → ~25% in `internal/dav` |
| Remove dead code | Removes false-negative 0% entries |
| **Total estimated improvement** | **~50% → ~65-70% overall** |