bookhoard/TEST_RELIABILITY_PLAN.md

Test Reliability Improvement Plan

Bookhoard Pre-Production Test Overhaul

Status: Planning Phase - DO NOT IMPLEMENT YET
Date: 2025-02-13
Scope: Comprehensive test refactoring for production-grade reliability
Goal: Transform tests from brittle map-based assertions to robust struct-based validation with full database verification and edge case coverage

---

🎯 Executive Summary

Current State

• 507+ instances of map[string]interface{} instead of structs
• Zero concurrency tests for race conditions
• Minimal database verification after mutations
• Happy-path only for WebSocket, sync, OPDS, and book matching
• No null/empty/missing field edge case coverage

Target State

• ✅ 100% struct-based assertions for API responses (compile-time safety)
• ✅ 100% database verification for all mutation operations
• ✅ Comprehensive edge cases: null/empty/missing, boundaries, Unicode
• ✅ Concurrency coverage for all critical sync paths
• ✅ Error path coverage for WebSocket, sync, OPDS, matching

Impact

• Silent API contract changes → Compile-time detection
• Data corruption bugs → Pre-deployment prevention
• Race conditions → Caught in testing
• Edge case crashes → Eliminated before production

---

📋 Table of Contents

1. Phase 1: Stop Silent Failures - HIGHEST ROI
2. Phase 2: Concurrency Protection
3. Phase 3: Hardened Error Handling
4. Phase 4: Load and Security
5. Implementation Order
6. Verification Checklist

---

Phase 1: Stop Silent Failures

Priority: 🔴 CRITICAL - Do this first
Time Estimate: 20-25 files
Risk Level: Medium (compilation errors will guide fixes)
ROI: Highest - catches API changes and data corruption before production

Objectives

1. Replace map[string]interface{} with actual structs in all assertions
2. Add database state verification after all mutation operations
3. Add null/empty/missing field edge cases for critical paths

---

1.1 Replace Map-Based Assertions with Structs

Problem Areas (507+ instances)

File	Lines	Issue	Impact
cmd/server/tests/registration_test.go	18-305	Registration requests/responses use maps	Silent field renames
cmd/server/tests/user_test.go	14-488	Profile updates use maps	API contract drift
cmd/server/tests/device_test.go	18-529	Device CRUD uses maps	Missing field validation
cmd/server/tests/collections_bulk_test.go	1-200	Bulk operations use maps	Data corruption undetected
cmd/server/tests/media_bulk_test.go	14-100	Bulk updates use maps	Schema changes missed
cmd/server/tests/conflicts_bulk_test.go	1-150	Conflict resolution uses maps	Resolution failures silent
cmd/server/tests/analytics_test.go	1-250	Analytics queries use maps	Wrong field types undetected
cmd/server/tests/search_test.go	21-244	Search responses use maps	API changes break users
cmd/server/tests/filtering_test.go	20-262	Filter results use maps	Null handling untested
cmd/server/tests/sorting_test.go	1-150	Sort responses use maps	Type errors silent
cmd/server/tests/library_test.go	15-378	Library CRUD uses maps	Missing fields in tests
cmd/server/tests/auth_test.go	19-300	Auth responses use maps	Security issues undetected

Conversion Pattern

BEFORE (brittle):

// device_test.go:138-141
var response map[string]interface{}
json.Unmarshal(rec.Body.Bytes(), &response)
devices := response["devices"].([]interface{})
firstDevice := devices[0].(map[string]interface{})
deviceName, ok := firstDevice["device_name"].(string)
assert.True(t, ok, "Should have device_name")
assert.Equal(t, "Test Device", deviceName)
// ❌ If "device_name" → "device_name_display", test passes with empty string

AFTER (safe):

// Import handler types
import "bookhoard/internal/handlers"

// device_test.go:138-141 (NEW)
var response handlers.DeviceListResponse
err := json.Unmarshal(rec.Body.Bytes(), &response)
require.NoError(t, err, "Response should match DeviceListResponse schema")
require.Greater(t, len(response.Devices), 0, "Should have at least one device")
assert.Equal(t, "Test Device", response.Devices[0].DeviceName)
// ✅ If DeviceName field renamed, COMPILATION FAILS

File-by-File Plan

File: cmd/server/tests/device_test.go

Lines to modify:

• 18-529: Full file conversion

Specific changes:

Line Range	Current	New
36-38	var regResponse map[string]interface{}	var response handlers.DeviceRegistrationResponse
60-61	var statusResponse map[string]interface{}	var response handlers.DeviceAuthStatusResponse
82-85	var loginResponse map[string]interface{}	var response handlers.LoginResponse
131-135	var response map[string]interface{}	var response handlers.DeviceListResponse
167-173	var response map[string]interface{}	var response handlers.DeviceUpdateResponse
342-345	var response map[string]interface{}	var response handlers.DeviceTokenResponse

Required imports (add to top):

import (
    "bookhoard/internal/handlers"  // NEW
    // ... existing imports
)

Action: Convert all response parsing to use handler structs. Update assertions to access struct fields directly.

---

File: cmd/server/tests/user_test.go

Lines to modify:

• 14-488: Full file conversion

Specific changes:

Line Range	Current	New
43-50	profile := map[string]interface{}	Use handlers.UserInfo struct
258-260	var result map[string]interface{}	var response handlers.UpdateResponse
472-481	users := []map[string]interface{}	var response handlers.UsersListResponse

Action: Import handlers package. Convert all user profile and list responses to structs.

---

File: cmd/server/tests/registration_test.go

Lines to modify:

• 18-305: Full file conversion

Specific changes:

Test Function	Lines	Change
TestRegistration_Flow	18-85	Use handlers.DeviceRegistrationResponse
TestRegistration_StatusCheck	87-120	Use handlers.DeviceAuthStatusResponse
TestRegistration_Approval	122-160	Use handlers.ApprovalResponse
TestRegistration_Rejection	162-200	Use handlers.RejectionResponse
TestRegistration_QRCode	202-240	Use handlers.QRCodeResponse
TestRegistration_Expiry	242-280	Use handlers.ExpiryResponse
TestRegistration_Concurrent	282-305	Use all above structs

Action: Import handlers package. Replace all map-based response parsing with struct-based unmarshaling.

---

File: cmd/server/tests/collections_bulk_test.go

Lines to modify:

• 1-200: Full file conversion

Specific changes:

Test	Lines	Current	New
TestCollectionsBulk_Add	14-50	map[string]interface{}	handlers.BulkAddResponse
TestCollectionsBulk_Remove	52-100	map[string]interface{}	handlers.BulkRemoveResponse
TestCollectionsBulk_Move	102-150	map[string]interface{}	handlers.BulkMoveResponse
TestCollectionsBulk_Validation	152-200	map[string]interface{}	handlers.ValidationErrorResponse

Action:

1. Import handlers package
2. Create response structs in handlers/collections.go if they don't exist
3. Update all bulk operation tests to use structs

---

File: cmd/server/tests/media_bulk_test.go

Lines to modify:

• 14-100: Full file conversion

Specific changes:

Test	Lines	Current	New
TestMediaBulk_Delete	14-50	map[string]interface{}	handlers.BulkDeleteResponse
TestMediaBulk_Update	52-100	map[string]interface{}	handlers.BulkUpdateResponse

Action:

1. Import handlers package
2. Ensure handlers.MediaHandler has response structs for bulk operations
3. Update all bulk media tests to use structs

---

File: cmd/server/tests/conflicts_bulk_test.go

Lines to modify:

• 1-150: Full file conversion

Specific changes:

Test	Lines	Current	New
TestConflictsBulk_Resolve	14-50	map[string]interface{}	handlers.BulkResolveResponse
TestConflictsBulk_Dismiss	52-100	map[string]interface{}	handlers.BulkDismissResponse
TestConflictsBulk_Escalate	102-150	map[string]interface{}	handlers.BulkEscalateResponse

Action:

1. Import handlers package
2. Ensure conflict response structs exist
3. Update all bulk conflict tests to use structs

---

File: cmd/server/tests/analytics_test.go

Lines to modify:

• 1-250: Full file conversion

Specific changes:

Test	Lines	Current	New
TestAnalytics_ReadingStats	14-50	map[string]interface{}	handlers.ReadingStatsResponse
TestAnalytics_PopularBooks	52-100	map[string]interface{}	handlers.PopularBooksResponse
TestAnalytics_DeviceUsage	102-150	map[string]interface{}	handlers.DeviceUsageResponse
TestAnalytics_UserActivity	152-200	map[string]interface{}	handlers.UserActivityResponse

Action:

1. Import handlers package
2. Ensure analytics response structs exist
3. Update all analytics tests to use structs

---

File: cmd/server/tests/search_test.go

Lines to modify:

• 21-244: Full file conversion

Specific changes:

Test	Lines	Current	New
TestSearch_Books	21-80	map[string]interface{}	handlers.SearchBooksResponse
TestSearch_Authors	82-140	map[string]interface{}	handlers.SearchAuthorsResponse
TestSearch_Series	142-200	map[string]interface{}	handlers.SearchSeriesResponse
TestSearch_Advanced	202-244	map[string]interface{}	handlers.AdvancedSearchResponse

Action:

1. Import handlers package
2. Ensure search response structs exist
3. Update all search tests to use structs

---

File: cmd/server/tests/filtering_test.go

Lines to modify:

• 20-262: Full file conversion

Specific changes:

Test	Lines	Current	New
TestFiltering_ByStatus	20-60	map[string]interface{}	handlers.FilteredBooksResponse
TestFiltering_ByCollection	62-120	map[string]interface{}	handlers.CollectionBooksResponse
TestFiltering_ByDateRange	122-180	map[string]interface{}	handlers.DateRangeResponse
TestFiltering_Advanced	182-262	map[string]interface{}	handlers.AdvancedFilterResponse

Action:

1. Import handlers package
2. Ensure filtering response structs exist
3. Update all filtering tests to use structs

---

File: cmd/server/tests/sorting_test.go

Lines to modify:

• 1-150: Full file conversion

Specific changes:

Test	Lines	Current	New
TestSorting_ByTitle	14-40	map[string]interface{}	handlers.SortedBooksResponse
TestSorting_ByAuthor	42-80	map[string]interface{}	handlers.SortedBooksResponse
TestSorting_ByDateAdded	82-120	map[string]interface{}	handlers.SortedBooksResponse
TestSorting_ByRating	122-150	map[string]interface{}	handlers.SortedBooksResponse

Action:

1. Import handlers package
2. Ensure sorting response struct exists
3. Update all sorting tests to use structs

---

File: cmd/server/tests/library_test.go

Lines to modify:

• 15-378: Full file conversion

Specific changes:

Test	Lines	Current	New
TestLibraryCreation	91-161	map[string]interface{}	handlers.LibraryResponse
TestLibraryTypes	164-205	map[string]interface{}	handlers.LibraryTypesResponse
TestUserVisibleLibraries	208-238	map[string]interface{}	handlers.VisibleLibrariesResponse
TestMediaItemsList	241-269	map[string]interface{}	handlers.MediaItemsResponse
TestJSONValidation	272-333	map[string]interface{}	handlers.ValidationResponse
TestErrorHandling	336-377	map[string]interface{}	handlers.ErrorResponse

Action:

1. Import handlers package
2. Ensure library response structs exist
3. Update all library tests to use structs

---

File: cmd/server/tests/auth_test.go

Lines to modify:

• 19-300: Full file conversion

Specific changes:

Test	Lines	Current	New
TestAuth_Login	19-80	map[string]interface{}	handlers.LoginResponse
TestAuth_Register	82-140	map[string]interface{}	handlers.RegisterResponse
TestAuth_Refresh	142-200	map[string]interface{}	handlers.RefreshResponse
TestAuth_Logout	202-250	map[string]interface{}	handlers.LogoutResponse
TestAuth_PasswordReset	252-300	map[string]interface{}	handlers.PasswordResetResponse

Action:

1. Import handlers package
2. Ensure auth response structs exist
3. Update all auth tests to use structs

---

1.2 Add Database State Verification

Problem Areas

File	Tests Missing DB Verification	Risk
device_test.go	TestUpdateDevice, TestDeleteDevice	API says success but DB unchanged
user_test.go	All profile update tests	Silent data loss
collections_bulk_test.go	All bulk operations	Orphaned collection_books records
media_bulk_test.go	All bulk deletes	Files deleted but DB records remain
conflicts_bulk_test.go	All conflict resolutions	Conflicts marked resolved but aren't
library_test.go	Create, update, delete libraries	Schema violations undetected
registration_test.go	Device approval flow	Device created but not linked to user
sync_integration_test.go	Sync success but no DB verification	Progress lost

Verification Pattern

BEFORE (insufficient):

// device_test.go:191
assert.Equal(t, http.StatusNoContent, rec.Code, "Should delete device")
// ❌ No DB query - device might still exist

AFTER (complete):

// device_test.go:191 (NEW)
assert.Equal(t, http.StatusNoContent, rec.Code, "Should delete device")

// NEW: Verify device actually deleted from database
pgDeviceID := pgtype.UUID{Bytes: [16]byte(device.ID), Valid: true}
_, err := setup.DB.GetDevice(context.Background(), pgDeviceID)
assert.Error(t, err, "Device should be deleted from database")
// ✅ Now we know delete actually worked

File-by-File Plan

File: cmd/server/tests/device_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestUpdateDevice	144-176	Checks HTTP 200 only	Query DB to verify name, sync_enabled, sync_frequency actually updated
TestDeleteDevice	178-197	Already has DB check	✅ Already good - keep as example
TestRegenerateDeviceToken_Success	326-362	Checks HTTP 200 only	Query DB to verify auth_token actually changed
TestRegenerateDeviceToken_OldTokenInvalidated	364-407	Checks token works only	Query DB to verify only one valid token exists
TestDeviceRegistrationFlow	18-114	Checks response codes	Query DB after step 4 to verify device exists and is linked to user

Action: After each successful mutation, add DB query to verify state change:

// Example for TestUpdateDevice (after line 175)
// NEW: Verify database state
updatedDevice, err := setup.DB.GetDevice(context.Background(), pgtype.UUID{Bytes: [16]byte(device.ID), Valid: true})
require.NoError(t, err, "Should retrieve updated device")
assert.Equal(t, "Updated Device Name", updatedDevice.DeviceName)
assert.Equal(t, false, updatedDevice.SyncEnabled.Bool)
assert.Equal(t, int32(10), updatedDevice.SyncFrequencyMinutes.Int32)

---

File: cmd/server/tests/user_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestUserProfileEndpoints	15-77	Checks HTTP 200 only	Query DB after PUT to verify first_name, last_name updated
TestUserUpdateEndpoints	80-336	All checks HTTP codes only	Query DB after each successful update (email, username, password, theme)
TestAccountDeletion	340-440	Checks HTTP codes only	Query DB after delete to verify user record removed

Action: Add DB verification queries after successful updates:

// Example for email update (after line 112)
// NEW: Verify database state
updatedUser, err := setup.DB.GetUserByEmail(context.Background(), "newemail@example.com")
require.NoError(t, err, "Should find user with new email")
assert.Equal(t, "newemail@example.com", updatedUser.Email)

---

File: cmd/server/tests/collections_bulk_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestCollectionsBulk_Add	All	Checks response only	Query collection_items table to verify all book IDs added
TestCollectionsBulk_Remove	All	Checks response only	Query collection_items table to verify all book IDs removed
TestCollectionsBulk_Move	All	Checks response only	Query collection_items table to verify books moved to target collection
TestCollectionsBulk_DuplicateHandling	All	Checks response only	Query collection_items to verify no duplicate entries created

Action: After bulk operations, query junction table:

// Example for TestCollectionsBulk_Add (after assertion)
// NEW: Verify database state
collectionItems, err := setup.DB.GetCollectionItems(context.Background(), collectionID)
require.NoError(t, err, "Should retrieve collection items")
require.Equal(t, len(bookIDs), len(collectionItems), "All books should be in collection")

// Verify each book ID exists
actualIDs := make([]uuid.UUID, len(collectionItems))
for i, item := range collectionItems {
    actualIDs[i] = item.MediaItemID
}
assert.ElementsMatch(t, bookIDs, actualIDs, "All book IDs should match")

---

File: cmd/server/tests/media_bulk_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestMediaBulk_Delete	80-100	Checks response only	Query media_items table to verify records deleted (not just file_deleted flag)
TestMediaBulk_Update	All (if exists)	Checks response only	Query media_items table to verify all fields actually updated

Action: Add DB verification after bulk deletes:

// Example for TestMediaBulk_Delete (after line 99)
// NEW: Verify database state
for _, mediaID := range mediaIDs {
    pgID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
    _, err := setup.DB.GetMediaItem(context.Background(), pgID)
    assert.Error(t, err, "Media item should be deleted from database")
}

---

File: cmd/server/tests/conflicts_bulk_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestConflictsBulk_Resolve	All	Checks response only	Query sync_conflicts table to verify resolution_status updated
TestConflictsBulk_Dismiss	All	Checks response only	Query sync_conflicts table to verify dismissed status
TestConflictsBulk_Escalate	All	Checks response only	Query sync_conflicts table to verify escalated to admin

Action: Add DB verification after conflict operations:

// Example for TestConflictsBulk_Resolve (after assertion)
// NEW: Verify database state
for _, conflictID := range conflictIDs {
    pgID := pgtype.UUID{Bytes: [16]byte(conflictID), Valid: true}
    conflict, err := setup.DB.GetSyncConflict(context.Background(), pgID)
    require.NoError(t, err, "Should retrieve conflict")
    assert.Equal(t, "resolved", conflict.ResolutionStatus.String)
}

---

File: cmd/server/tests/sync_integration_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestSyncFull_Initial	All	Checks response only	Query reading_progress, media_notes, media_highlights tables to verify sync data saved
TestSyncFull_Incremental	All	Checks response only	Query DB to verify only changed fields updated
TestSyncConflict_Resolution	All	Checks response only	Query sync_conflicts table to verify conflict created and resolved

Action: Add comprehensive DB verification after sync:

// Example for TestSyncFull_Initial (after success assertion)
// NEW: Verify database state
progress, err := setup.DB.GetReadingProgress(context.Background(), database.GetReadingProgressParams{
    MediaItemID: pgtype.UUID{Bytes: [16]byte(mediaItemID), Valid: true},
    UserID: pgtype.UUID{Bytes: [16]byte(userID), Valid: true},
})
require.NoError(t, err, "Should retrieve reading progress")
assert.Equal(t, testProgress.Percentage, progress.Percentage.Float64)

---

File: cmd/server/tests/registration_test.go

Tests needing DB verification:

Test Function	Lines	Current State	Required Addition
TestRegistrationFlow	18-114	Checks responses only	After approval, query devices table to verify device exists and linked to user
TestRegistrationApproval	All	Checks response only	Query devices table to verify auth_token set and device active

Action: Add DB verification after registration approval:

// Example for TestRegistrationFlow (after line 93)
// NEW: Verify database state
pgDeviceID := pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}
device, err := setup.DB.GetDevice(context.Background(), pgDeviceID)
require.NoError(t, err, "Device should exist in database")
assert.NotEmpty(t, device.AuthToken, "Device should have auth token after approval")

---

1.3 Add Null/Empty/Missing Edge Cases

Problem Areas

Field Type	Missing Tests	Production Risk
Text fields (first_name, last_name, description)	null vs "" vs missing	Constraint violations, different behavior
Optional arrays (tags, contributors)	null vs [] vs missing	Search failures
Optional dates (date_published)	null vs invalid vs missing	Display corruption
Optional IDs (asin, isbn)	null vs "" vs missing	External API failures
Optional enums (reading_status)	null vs invalid vs missing	Filter breaks

Test Patterns to Add

Pattern 1: Null vs Empty String vs Missing

// NEW TEST: Add to user_test.go
func TestUserProfile_NullVsEmptyVsMissing(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    testCases := []struct {
        name     string
        payload   map[string]interface{}
        expectOK  bool
    }{
        {
            name: "Null first_name",
            payload: map[string]interface{}{
                "first_name": nil,
            },
            expectOK: true,  // null should be allowed (optional field)
        },
        {
            name: "Empty string first_name",
            payload: map[string]interface{}{
                "first_name": "",
            },
            expectOK: true,  // empty string should be allowed
        },
        {
            name: "Missing first_name",
            payload: map[string]interface{}{
                // first_name not included
            },
            expectOK: true,  // missing should be allowed (optional)
        },
        {
            name: "Valid first_name",
            payload: map[string]interface{}{
                "first_name": "John",
            },
            expectOK: true,
        },
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            jsonData, _ := json.Marshal(tc.payload)
            req := httptest.NewRequest("PUT", "/api/auth/profile", bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            rec := httptest.NewRecorder()
            setup.Server.Config.Handler.ServeHTTP(rec, req)

            if tc.expectOK {
                assert.Equal(t, http.StatusOK, rec.Code)
            } else {
                assert.NotEqual(t, http.StatusOK, rec.Code)
            }
        })
    }
}

Pattern 2: Null vs Empty Array vs Missing

// NEW TEST: Add to media_bulk_test.go
func TestMediaTags_NullVsEmptyVsMissing(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    mediaID := createTestMediaItemID(t, setup.Server, token)

    testCases := []struct {
        name     string
        tags      interface{}
        expectOK  bool
    }{
        {
            name: "Null tags",
            tags: nil,
            expectOK: true,  // null should clear tags
        },
        {
            name: "Empty array tags",
            tags: []string{},
            expectOK: true,  // empty array should clear tags
        },
        {
            name: "Missing tags",
            // tags field not included
            expectOK: true,  // missing should not change tags
        },
        {
            name: "Valid tags",
            tags: []string{"fiction", "science-fiction"},
            expectOK: true,
        },
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            payload := map[string]interface{}{"tags": tc.tags}
            jsonData, _ := json.Marshal(payload)
            
            req := httptest.NewRequest("PUT", fmt.Sprintf("/api/media-items/%s", mediaID), bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            rec := httptest.NewRecorder()
            setup.Server.Config.Handler.ServeHTTP(rec, req)

            if tc.expectOK {
                assert.Equal(t, http.StatusOK, rec.Code)
                
                // NEW: Verify database state
                pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
                media, err := setup.DB.GetMediaItem(context.Background(), pgMediaID)
                require.NoError(t, err)
                
                if tc.tags == nil {
                    assert.Empty(t, media.Tags, "Tags should be null/empty")
                } else if tags, ok := tc.tags.([]string); ok && len(tags) == 0 {
                    assert.Empty(t, media.Tags, "Tags should be empty")
                } else if tags, ok := tc.tags.([]string); ok {
                    assert.ElementsMatch(t, tags, media.Tags, "Tags should match")
                }
            }
        })
    }
}

File-by-File Plan

File: cmd/server/tests/user_test.go

Add new test function after line 336:

// NEW TEST FUNCTION
func TestUserProfile_NullVsEmptyVsMissing(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    testCases := []struct {
        name     string
        field     string
        value     interface{}
        expectOK  bool
    }{
        // First name tests
        {"Null first_name", "first_name", nil, true},
        {"Empty first_name", "first_name", "", true},
        {"Missing first_name", "first_name", nil, true},  // Not included in payload
        {"Valid first_name", "first_name", "Jane", true},
        
        // Last name tests
        {"Null last_name", "last_name", nil, true},
        {"Empty last_name", "last_name", "", true},
        {"Valid last_name", "last_name", "Doe", true},
        
        // Email tests (required field, different expectations)
        {"Null email", "email", nil, false},  // Email required, null should fail
        {"Empty email", "email", "", false},  // Email required, empty should fail
        {"Valid email", "email", "new@example.com", true},
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            payload := map[string]interface{}{tc.field: tc.value}
            jsonData, _ := json.Marshal(payload)

            req := httptest.NewRequest("PUT", "/api/auth/profile", bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            rec := httptest.NewRecorder()
            setup.Server.Config.Handler.ServeHTTP(rec, req)

            if tc.expectOK {
                assert.Equal(t, http.StatusOK, rec.Code, tc.name)
            } else {
                assert.NotEqual(t, http.StatusOK, rec.Code, tc.name)
            }
        })
    }
}

Action: Add new test file or append to existing user_test.go after line 336.

---

File: cmd/server/tests/media_bulk_test.go

Add new test function:

// NEW TEST FUNCTION
func TestMediaItem_NullVsEmptyVsMissing(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    mediaID := createTestMediaItemID(t, setup.Server, token)

    testCases := []struct {
        name     string
        field     string
        value     interface{}
        expectOK  bool
        verifyDB  bool
    }{
        // Tags tests (array field)
        {"Null tags", "tags", nil, true, true},
        {"Empty array tags", "tags", []string{}, true, true},
        {"Valid tags", "tags", []string{"fiction", "sci-fi"}, true, true},
        
        // ISBN tests (optional text)
        {"Null ISBN", "isbn", nil, true, true},
        {"Empty ISBN", "isbn", "", true, true},
        {"Valid ISBN", "isbn", "978-0-123456-78-9", true, true},
        
        // ASIN tests (optional text)
        {"Null ASIN", "asin", nil, true, true},
        {"Empty ASIN", "asin", "", true, true},
        {"Valid ASIN", "asin", "B08XXXXXXX", true, true},
        
        // Date published tests (optional date)
        {"Null date_published", "date_published", nil, true, true},
        {"Invalid date_published", "date_published", "not-a-date", false, false},
        {"Valid date_published", "date_published", "2024-01-15", true, true},
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            payload := map[string]interface{}{tc.field: tc.value}
            jsonData, _ := json.Marshal(payload)
            
            req := httptest.NewRequest("PUT", fmt.Sprintf("/api/media-items/%s", mediaID), bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            rec := httptest.NewRecorder()
            setup.Server.Config.Handler.ServeHTTP(rec, req)

            if tc.expectOK {
                assert.Equal(t, http.StatusOK, rec.Code, tc.name)
                
                // NEW: Verify database state
                if tc.verifyDB {
                    pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
                    media, err := setup.DB.GetMediaItem(context.Background(), pgMediaID)
                    require.NoError(t, err, "Should retrieve media item")
                    
                    // Verify field value matches expected
                    switch tc.field {
                    case "tags":
                        if tc.value == nil {
                            assert.Empty(t, media.Tags, "Tags should be null/empty")
                        } else if tags, ok := tc.value.([]string); ok {
                            assert.ElementsMatch(t, tags, media.Tags, "Tags should match")
                        }
                    case "isbn":
                        if tc.value == nil || tc.value == "" {
                            assert.False(t, media.Isbn.Valid, "ISBN should be null")
                        } else {
                            assert.True(t, media.Isbn.Valid, "ISBN should be set")
                            assert.Equal(t, tc.value.(string), media.Isbn.String)
                        }
                    case "asin":
                        if tc.value == nil || tc.value == "" {
                            assert.False(t, media.Asin.Valid, "ASIN should be null")
                        } else {
                            assert.True(t, media.Asin.Valid, "ASIN should be set")
                            assert.Equal(t, tc.value.(string), media.Asin.String)
                        }
                    }
                }
            } else {
                assert.NotEqual(t, http.StatusOK, rec.Code, tc.name)
            }
        })
    }
}

Action: Add new test function to media_bulk_test.go.

---

File: cmd/server/tests/library_test.go

Add new test function:

// NEW TEST FUNCTION
func TestLibrary_NullVsEmptyVsMissing(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    userID := getTestUserID(t, setup.DB)

    // Create library first
    libReq := map[string]interface{}{
        "name": "Test Library",
        "type": "ebooks",
    }
    libBody, _ := json.Marshal(libReq)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)
    
    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()
    require.Equal(t, http.StatusCreated, resp.StatusCode)

    var libResponse map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&libResponse)
    libraryID := libResponse["id"].(string)

    testCases := []struct {
        name     string
        field     string
        value     interface{}
        expectOK  bool
    }{
        {"Null description", "description", nil, true},
        {"Empty description", "description", "", true},
        {"Valid description", "description", "A test library", true},
        
        {"Null name", "name", nil, false},  // Name required
        {"Empty name", "name", "", false},    // Name required
        {"Valid name", "name", "Updated Library", true},
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            payload := map[string]interface{}{tc.field: tc.value}
            jsonData, _ := json.Marshal(payload)
            
            req := httptest.NewRequest("PUT", fmt.Sprintf("%s/api/libraries/%s", setup.Server.URL, libraryID), bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            
            resp, err := client.Do(req)
            require.NoError(t, err)
            defer resp.Body.Close()

            if tc.expectOK {
                assert.Equal(t, http.StatusOK, resp.StatusCode, tc.name)
            } else {
                assert.NotEqual(t, http.StatusOK, resp.StatusCode, tc.name)
            }
        })
    }
}

Action: Add new test function to library_test.go.

---

Phase 1 Summary

Files Modified: 12+ test files
Lines Changed: ~2000+ lines
New Test Functions: 15+
Compilation Checks: 100% (struct-based assertions)
Database Verification: 100% of mutations

Verification Steps:

1. ✅ All tests compile with struct-based assertions
2. ✅ All mutation operations verify database state
3. ✅ Null/empty/missing edge cases covered for critical fields

---

Phase 2: Concurrency Protection

Priority: 🟠 HIGH - Do after Phase 1
Time Estimate: 8-10 new test functions
Risk Level: Low (new tests only)
ROI: High - prevents lost updates and deadlocks

Objectives

1. Add race condition tests for all sync operations
2. Add concurrent bulk operation tests
3. Add concurrent profile + device update tests

---

2.1 Sync Operation Race Conditions

Problem Areas

Operation	Missing Tests	Risk
Reading progress sync	Multiple devices updating same book simultaneously	Lost updates, last-write-wins
Conflict resolution	Two users resolving same conflict	Duplicate resolution records
Bulk operations	Bulk delete + individual update simultaneously	Orphaned records
Device registration	Multiple devices registering simultaneously	Duplicate device IDs
Token regeneration	Regenerate token while old token in use	Auth failures

Test Pattern

// NEW TEST TEMPLATE: Concurrent sync
func TestSyncConcurrent_ProgressUpdates(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-concurrent")
    mediaID := createTestMediaItemID(t, setup.Server, token)

    // Start with initial progress
    initialProgress := map[string]interface{}{
        "media_item_id": mediaID,
        "percentage": 25.0,
        "current_page": 50,
        "total_pages": 200,
    }

    // Simulate 3 concurrent updates from different "sources"
    var wg sync.WaitGroup
    errors := make(chan error, 3)

    for i := 0; i < 3; i++ {
        wg.Add(1)
        go func(updateNum int) {
            defer wg.Done()
            
            progressData := initialProgress
            progressData["percentage"] = 25.0 + float64(updateNum * 10)
            progressData["current_page"] = 50 + (updateNum * 20)
            
            jsonData, _ := json.Marshal(progressData)
            req := httptest.NewRequest("POST", setup.Server.URL+"/api/sync/koreader/progress", bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+device.AuthToken)
            
            client := &http.Client{Timeout: 5 * time.Second}
            resp, err := client.Do(req)
            if err != nil {
                errors <- err
                return
            }
            defer resp.Body.Close()
            
            if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusConflict {
                errors <- fmt.Errorf("unexpected status: %d", resp.StatusCode)
            }
        }(i)
    }

    wg.Wait()
    close(errors)

    // Check for errors (conflicts are OK, but not failures)
    for err := range errors {
        t.Logf("Concurrent update error: %v", err)
        // We expect either success or conflict, not failures
    }

    // NEW: Verify final state is consistent
    pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
    pgUserID := pgtype.UUID{Bytes: [16]byte(setup.User.ID), Valid: true}
    progress, err := setup.DB.GetReadingProgress(context.Background(), database.GetReadingProgressParams{
        MediaItemID: pgMediaID,
        UserID: pgUserID,
    })
    require.NoError(t, err, "Should retrieve final progress")
    
    // Verify one of the updates won (no corruption)
    assert.GreaterOrEqual(t, progress.Percentage.Float64, 25.0)
    assert.LessOrEqual(t, progress.Percentage.Float64, 55.0)
    assert.True(t, progress.Percentage.Float64 == 25.0 || 
                progress.Percentage.Float64 == 35.0 || 
                progress.Percentage.Float64 == 45.0,
                "Final progress should be one of the concurrent updates")
}

File-by-File Plan

File: cmd/server/tests/sync_integration_test.go

Add new test functions:

1. TestSyncConcurrent_ProgressUpdates (after existing sync tests)

   • Lines: Add ~80 lines
   • Purpose: Multiple devices updating same book's progress
   • Verification: Final progress is one of the updates (no corruption)

2. TestSyncConcurrent_NotesUpdates (after progress test)

   • Lines: Add ~80 lines
   • Purpose: Multiple devices adding notes to same book
   • Verification: All notes saved, no duplicates

3. TestSyncConcurrent_HighlightsUpdates (after notes test)

   • Lines: Add ~80 lines
   • Purpose: Multiple devices adding highlights to same book
   • Verification: All highlights saved, no corruption

4. TestSyncConcurrent_ProgressPlusNote (after highlights test)

   • Lines: Add ~80 lines
   • Purpose: Progress update + note creation simultaneously
   • Verification: Both operations succeed

Action: Add 4 new test functions to sync_integration_test.go.

---

2.2 Bulk Operation Race Conditions

Test Pattern

// NEW TEST: Concurrent bulk + individual operations
func TestBulkConcurrent_DeleteAndUpdate(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    // Create multiple media items
    var mediaIDs []string
    for i := 0; i < 5; i++ {
        id := createTestMediaItemID(t, setup.Server, token)
        mediaIDs = append(mediaIDs, id)
    }

    var wg sync.WaitGroup
    errors := make(chan error, 2)

    // Concurrent operation 1: Bulk delete
    wg.Add(1)
    go func() {
        defer wg.Done()
        
        payload := map[string]interface{}{
            "media_item_ids": mediaIDs[0:3],  // Delete first 3
        }
        jsonData, _ := json.Marshal(payload)
        
        req := httptest.NewRequest("POST", setup.Server.URL+"/api/media-items/bulk-delete", bytes.NewBuffer(jsonData))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        
        client := &http.Client{Timeout: 5 * time.Second}
        resp, err := client.Do(req)
        if err != nil {
            errors <- err
            return
        }
        defer resp.Body.Close()
        
        if resp.StatusCode != http.StatusOK {
            errors <- fmt.Errorf("bulk delete failed: %d", resp.StatusCode)
        }
    }()

    // Concurrent operation 2: Individual update on same items
    wg.Add(1)
    go func() {
        defer wg.Done()
        
        // Try to update item that might be deleted
        payload := map[string]interface{}{
            "title": "Updated Title",
        }
        jsonData, _ := json.Marshal(payload)
        
        req := httptest.NewRequest("PUT", fmt.Sprintf("%s/api/media-items/%s", setup.Server.URL, mediaIDs[1]), bytes.NewBuffer(jsonData))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        
        client := &http.Client{Timeout: 5 * time.Second}
        resp, err := client.Do(req)
        if err != nil {
            errors <- err
            return
        }
        defer resp.Body.Close()
        
        // Either succeeds (delete hadn't run yet) or fails (item deleted)
        if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNotFound {
            errors <- fmt.Errorf("update failed with unexpected status: %d", resp.StatusCode)
        }
    }()

    wg.Wait()
    close(errors)

    // Check for unexpected errors
    for err := range errors {
        t.Logf("Concurrent operation error: %v", err)
    }

    // NEW: Verify final database state
    // Items 0,1,2 should be deleted (bulk delete won or ran first)
    // Item 1 might be updated (update ran first)
    // Items 3,4 should exist untouched
    
    for i, mediaID := range mediaIDs {
        pgID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
        _, err := setup.DB.GetMediaItem(context.Background(), pgID)
        
        if i < 3 {
            // Should be deleted
            assert.Error(t, err, "Media item %d should be deleted", i)
        } else {
            // Should exist
            assert.NoError(t, err, "Media item %d should exist", i)
        }
    }
}

File-by-File Plan

File: cmd/server/tests/media_bulk_test.go

Add new test function:

1. TestBulkConcurrent_DeleteAndUpdate (after existing bulk tests)
   • Lines: Add ~80 lines
   • Purpose: Bulk delete + individual update on same items
   • Verification: Consistent final state, no orphaned records

File: cmd/server/tests/collections_bulk_test.go

Add new test function:

1. TestBulkConcurrent_AddAndRemove (after existing bulk tests)
   • Lines: Add ~80 lines
   • Purpose: Add items to collection while removing from same collection
   • Verification: Collection items consistent

File: cmd/server/tests/conflicts_bulk_test.go

Add new test function:

1. TestBulkConcurrent_ResolveAndEscalate (after existing bulk tests)
   • Lines: Add ~80 lines
   • Purpose: Resolve conflicts while escalating others to admin
   • Verification: No conflicts in ambiguous state

Action: Add 3 new test functions to respective bulk test files.

---

2.3 Profile + Device Concurrent Updates

Test Pattern

// NEW TEST: Concurrent profile + device updates
func TestUserConcurrent_ProfileAndDeviceUpdates(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-profile-device")

    var wg sync.WaitGroup
    errors := make(chan error, 2)

    // Concurrent operation 1: Update user profile
    wg.Add(1)
    go func() {
        defer wg.Done()
        
        payload := map[string]interface{}{
            "first_name": "UpdatedFirstName",
        }
        jsonData, _ := json.Marshal(payload)
        
        req := httptest.NewRequest("PUT", setup.Server.URL+"/api/auth/profile", bytes.NewBuffer(jsonData))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        
        client := &http.Client{Timeout: 5 * time.Second}
        resp, err := client.Do(req)
        if err != nil {
            errors <- err
            return
        }
        defer resp.Body.Close()
        
        if resp.StatusCode != http.StatusOK {
            errors <- fmt.Errorf("profile update failed: %d", resp.StatusCode)
        }
    }()

    // Concurrent operation 2: Update device sync settings (affects user)
    wg.Add(1)
    go func() {
        defer wg.Done()
        
        payload := map[string]interface{}{
            "sync_enabled": false,
        }
        jsonData, _ := json.Marshal(payload)
        
        req := httptest.NewRequest("PUT", fmt.Sprintf("%s/api/devices/%s", setup.Server.URL, device.ID.String()), bytes.NewBuffer(jsonData))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        
        client := &http.Client{Timeout: 5 * time.Second}
        resp, err := client.Do(req)
        if err != nil {
            errors <- err
            return
        }
        defer resp.Body.Close()
        
        if resp.StatusCode != http.StatusOK {
            errors <- fmt.Errorf("device update failed: %d", resp.StatusCode)
        }
    }()

    wg.Wait()
    close(errors)

    // Check for unexpected errors
    for err := range errors {
        t.Logf("Concurrent update error: %v", err)
    }

    // NEW: Verify final database state
    // Both updates should succeed
    pgUserID := pgtype.UUID{Bytes: [16]byte(setup.User.ID), Valid: true}
    user, err := setup.DB.GetUser(context.Background(), pgUserID)
    require.NoError(t, err, "Should retrieve user")
    assert.Equal(t, "UpdatedFirstName", user.FirstName.String)
    
    pgDeviceID := pgtype.UUID{Bytes: [16]byte(device.ID), Valid: true}
    updatedDevice, err := setup.DB.GetDevice(context.Background(), pgDeviceID)
    require.NoError(t, err, "Should retrieve device")
    assert.False(t, updatedDevice.SyncEnabled.Bool)
}

File-by-File Plan

File: cmd/server/tests/user_test.go

Add new test function:

1. TestUserConcurrent_ProfileAndDevice (after existing user tests)
   • Lines: Add ~70 lines
   • Purpose: Profile update + device update simultaneously
   • Verification: Both updates succeed

File: cmd/server/tests/device_test.go

Add new test function:

1. TestDeviceConcurrent_TokenRegenAndSync (after existing device tests)
   • Lines: Add ~80 lines
   • Purpose: Regenerate token while sync is in progress
   • Verification: Old token fails current sync, new token works for next

Action: Add 2 new test functions.

---

Phase 2 Summary

New Test Functions: 10+
Lines Added: ~800 lines
Race Coverage: Sync, bulk, profile + device
Database Verification: 100% of final states

Verification Steps:

1. ✅ All sync operations have concurrent tests
2. ✅ All bulk operations have concurrent tests
3. ✅ Profile + device interactions have concurrent tests

---

Phase 3: Hardened Error Handling

Priority: 🟡 MEDIUM - Do after Phase 2
Time Estimate: 15-20 new test functions
Risk Level: Low (new tests only)
ROI: Medium - prevents edge case crashes

Objectives

1. Add WebSocket error path tests
2. Add sync failure scenario tests
3. Add OPDS edge case tests
4. Add book matching edge case tests

---

3.1 WebSocket Error Paths

Problem Areas

Test File	Missing Tests	Risk
websocket_test.go	Disconnection mid-message	Orphaned connections
	Malformed WebSocket frames	Parser crashes
	Connection timeout	Resource leaks
	Connection hijacking	Security issues

Test Patterns

Pattern 1: Disconnection Mid-Message

// NEW TEST: WebSocket disconnect during message
func TestWebSocket_DisconnectMidMessage(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-ws-disconnect")

    // Connect WebSocket
    wsURL := strings.Replace(setup.Server.URL, "http", "ws", 1) + "/ws/device/" + device.AuthToken
    ws, _, err := websocket.DefaultDialer.Dial(wsURL, nil)
    require.NoError(t, err, "Should connect WebSocket")
    defer ws.Close()

    // Send partial message
    partialData := map[string]interface{}{
        "media_item_id": uuid.New(),
        "percentage": 25.5,
        // Missing required fields for incomplete message
    }
    jsonData, _ := json.Marshal(partialData)
    
    // Send message
    err = ws.WriteMessage(websocket.TextMessage, jsonData)
    require.NoError(t, err, "Should send message")

    // Immediately close connection (simulating disconnect)
    ws.Close()

    // Wait a bit for server to process
    time.Sleep(100 * time.Millisecond)

    // NEW: Verify no orphaned progress records in database
    // If message was processed, it should be complete
    // If not, it should be rolled back
    pgUserID := pgtype.UUID{Bytes: [16]byte(setup.User.ID), Valid: true}
    progress, err := setup.DB.ListReadingProgress(context.Background(), pgUserID)
    
    // Either message processed (with default values) or rolled back
    // No partial/corrupted records should exist
    for _, p := range progress {
        if p.Percentage.Float64 == 25.5 {
            // If partial percentage saved, other fields should have defaults
            assert.NotZero(t, p.TotalPages.Int32, "Should have default total_pages")
        }
    }
}

Pattern 2: Malformed Frames

// NEW TEST: Malformed WebSocket frames
func TestWebSocket_MalformedFrames(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-ws-malformed")

    // Connect WebSocket
    wsURL := strings.Replace(setup.Server.URL, "http", "ws", 1) + "/ws/device/" + device.AuthToken
    ws, _, err := websocket.DefaultDialer.Dial(wsURL, nil)
    require.NoError(t, err, "Should connect WebSocket")
    defer ws.Close()

    // Send invalid JSON
    invalidJSON := []byte("{invalid json}")
    err = ws.WriteMessage(websocket.TextMessage, invalidJSON)
    require.NoError(t, err, "Should send message")

    // Send binary data (unexpected)
    binaryData := []byte{0x00, 0x01, 0x02}
    err = ws.WriteMessage(websocket.BinaryMessage, binaryData)
    require.NoError(t, err, "Should send binary")

    // Send extremely large message
    largeData := make([]byte, 1024*1024) // 1MB
    err = ws.WriteMessage(websocket.TextMessage, largeData)
    if err != nil {
        t.Logf("Large message rejected (expected): %v", err)
    }

    // Wait for server to process
    time.Sleep(100 * time.Millisecond)

    // NEW: Verify WebSocket still open (not crashed)
    err = ws.WriteMessage(websocket.PingMessage, nil)
    // If server crashed, this will fail
    if err != nil {
        t.Logf("WebSocket connection closed after malformed frames: %v", err)
    }
}

File-by-File Plan

File: cmd/server/tests/websocket_test.go

Add new test functions:

1. TestWebSocket_DisconnectMidMessage (after existing tests)

   • Lines: Add ~50 lines
   • Purpose: Connection closes during message processing
   • Verification: No partial/corrupted data in DB

2. TestWebSocket_MalformedFrames (after previous test)

   • Lines: Add ~50 lines
   • Purpose: Invalid JSON, binary data, oversized messages
   • Verification: Server handles gracefully, no crashes

3. TestWebSocket_ConnectionTimeout (after previous test)

   • Lines: Add ~40 lines
   • Purpose: Connection times out during inactivity
   • Verification: Resources cleaned up

4. TestWebSocket_ConcurrentMessages (after previous test)

   • Lines: Add ~60 lines
   • Purpose: Multiple messages sent simultaneously
   • Verification: All processed correctly

Action: Add 4 new test functions to websocket_test.go.

---

3.2 Sync Failure Scenarios

Problem Areas

Scenario	Missing Tests	Risk
Offline device	Device syncs after being offline	Stale data overwrites fresh
Sync queue overflow	Too many pending sync operations	Queue exhaustion
Stale sync data	Old sync data arrives after newer	Incorrect progress
Sync conflicts	Two devices with conflicting data	Data inconsistency

Test Patterns

Pattern 1: Offline Device Sync

// NEW TEST: Device syncs after extended offline period
func TestSync_OfflineDevice(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-offline")
    mediaID := createTestMediaItemID(t, setup.Server, token)

    // Device sets progress to 50% while offline
    offlineProgress := map[string]interface{}{
        "media_item_id": mediaID,
        "percentage": 50.0,
        "current_page": 100,
        "total_pages": 200,
        "timestamp": time.Now().Add(-24 * time.Hour).Unix(), // 24 hours ago
    }

    // Simulate offline sync (old timestamp)
    jsonData, _ := json.Marshal(offlineProgress)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/sync/koreader/progress", bytes.NewBuffer(jsonData))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+device.AuthToken)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()

    // Should succeed but warn about stale data
    assert.Equal(t, http.StatusOK, resp.StatusCode)

    // NEW: Verify database handling of stale data
    pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
    pgUserID := pgtype.UUID{Bytes: [16]byte(setup.User.ID), Valid: true}
    progress, err := setup.DB.GetReadingProgress(context.Background(), database.GetReadingProgressParams{
        MediaItemID: pgMediaID,
        UserID: pgUserID,
    })
    require.NoError(t, err, "Should retrieve progress")

    // Stale data should either:
    // 1. Be rejected (keep existing newer data)
    // 2. Be accepted with warning (user notification)
    // Verify behavior matches expected
    assert.LessOrEqual(t, progress.Percentage.Float64, 50.0, "Should handle stale data appropriately")
}

Pattern 2: Sync Queue Overflow

// NEW TEST: Sync queue exceeds limits
func TestSync_QueueOverflow(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-queue-overflow")

    // Simulate many rapid sync operations (queue overflow)
    const numOps = 100
    var wg sync.WaitGroup
    errors := make(chan error, numOps)

    for i := 0; i < numOps; i++ {
        wg.Add(1)
        go func(opNum int) {
            defer wg.Done()
            
            progressData := map[string]interface{}{
                "media_item_id": uuid.New(), // Different items
                "percentage": float64(opNum),
            }
            jsonData, _ := json.Marshal(progressData)
            
            req := httptest.NewRequest("POST", setup.Server.URL+"/api/sync/koreader/progress", bytes.NewBuffer(jsonData))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+device.AuthToken)
            
            client := &http.Client{Timeout: 5 * time.Second}
            resp, err := client.Do(req)
            if err != nil {
                errors <- err
                return
            }
            defer resp.Body.Close()
            
            // Queue might be full (503) or accepted (202)
            if resp.StatusCode != http.StatusOK && 
               resp.StatusCode != http.StatusAccepted && 
               resp.StatusCode != http.StatusServiceUnavailable {
                errors <- fmt.Errorf("unexpected status: %d", resp.StatusCode)
            }
        }(i)
    }

    wg.Wait()
    close(errors)

    // Some operations might fail due to queue overflow (expected)
    errorCount := 0
    for range errors {
        errorCount++
    }
    t.Logf("Queue overflow: %d/%d operations failed", errorCount, numOps)

    // NEW: Verify queue didn't deadlock or crash
    // Queue should process or reject, not hang
}

File-by-File Plan

File: cmd/server/tests/sync_integration_test.go

Add new test functions:

1. TestSync_OfflineDevice (after concurrent tests)

   • Lines: Add ~60 lines
   • Purpose: Old sync data arrives after newer data
   • Verification: Appropriate handling (reject or warn)

2. TestSync_QueueOverflow (after previous test)

   • Lines: Add ~70 lines
   • Purpose: Many rapid sync operations
   • Verification: Queue handles gracefully, no deadlocks

3. TestSync_ConflictingData (after previous test)

   • Lines: Add ~70 lines
   • Purpose: Two devices send conflicting progress
   • Verification: Conflict detection or last-write-wins with notification

4. TestSync_OrphanedQueueItems (after previous test)

   • Lines: Add ~50 lines
   • Purpose: Queue items for deleted books
   • Verification: Orphaned items cleaned up

Action: Add 4 new test functions to sync_integration_test.go.

---

3.3 OPDS Edge Cases

Problem Areas

Scenario	Missing Tests	Risk
Empty library	OPDS feed with no books	Empty feed parsing errors
Very large feeds	Library with thousands of books	Pagination failures
Corrupted metadata	Books with invalid EPUB metadata	Feed generation crashes
Special characters	Titles with Unicode, emojis	XML/JSON encoding issues

Test Patterns

Pattern 1: Empty Library

// NEW TEST: OPDS feed for empty library
func TestOPDS_EmptyLibrary(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    // Create empty library
    libReq := map[string]interface{}{
        "name": "Empty Library",
        "type": "ebooks",
    }
    libBody, _ := json.Marshal(libReq)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()
    require.Equal(t, http.StatusCreated, resp.StatusCode)

    var libResponse map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&libResponse)
    libraryID := libResponse["id"].(string)

    // Get OPDS feed
    req = httptest.NewRequest("GET", fmt.Sprintf("%s/api/opds/libraries/%s", setup.Server.URL, libraryID), nil)
    req.Header.Set("Authorization", "Bearer "+token)
    
    resp, err = client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()

    assert.Equal(t, http.StatusOK, resp.StatusCode)

    // NEW: Verify feed structure
    var feed map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&feed)
    
    assert.Contains(t, feed, "entries", "Feed should have entries array")
    entries := feed["entries"].([]interface{})
    assert.Empty(t, entries, "Entries should be empty for empty library")
    assert.Contains(t, feed, "total", "Feed should have total count")
    assert.Equal(t, 0, int(feed["total"].(float64)), "Total should be 0")
}

Pattern 2: Large Feed

// NEW TEST: OPDS feed with many books
func TestOPDS_LargeFeed(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    // Create library
    libReq := map[string]interface{}{
        "name": "Large Library",
        "type": "ebooks",
    }
    libBody, _ := json.Marshal(libReq)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()
    require.Equal(t, http.StatusCreated, resp.StatusCode)

    var libResponse map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&libResponse)
    libraryID := libResponse["id"].(string)

    // Add folder
    folderReq := map[string]interface{}{
        "folder_path": "/app/uploads",
    }
    folderBody, _ := json.Marshal(folderReq)
    req = httptest.NewRequest("POST", fmt.Sprintf("%s/api/libraries/%s/folders", setup.Server.URL, libraryID), bytes.NewBuffer(folderBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)
    resp, err = client.Do(req)
    require.NoError(t, err)
    resp.Body.Close()

    // Add many books (simulate large library)
    const numBooks = 1000
    for i := 0; i < numBooks; i++ {
        mediaReq := map[string]interface{}{
            "library_id": libraryID,
            "title": fmt.Sprintf("Book %d", i),
            "author": "Test Author",
            "file_path": fmt.Sprintf("/tmp/test%d.epub", i),
            "file_size": 1024,
            "mime_type": "application/epub+zip",
        }
        mediaBody, _ := json.Marshal(mediaReq)
        
        req = httptest.NewRequest("POST", setup.Server.URL+"/api/media-items", bytes.NewBuffer(mediaBody))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        resp, err = client.Do(req)
        if err != nil {
            t.Logf("Failed to create book %d: %v", i, err)
            continue
        }
        resp.Body.Close()
    }

    // Get OPDS feed with pagination
    req = httptest.NewRequest("GET", fmt.Sprintf("%s/api/opds/libraries/%s?page=1&limit=100", setup.Server.URL, libraryID), nil)
    req.Header.Set("Authorization", "Bearer "+token)
    
    resp, err = client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()

    // NEW: Verify pagination
    assert.Equal(t, http.StatusOK, resp.StatusCode)
    
    var feed map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&feed)
    
    assert.Contains(t, feed, "entries", "Feed should have entries array")
    entries := feed["entries"].([]interface{})
    assert.LessOrEqual(t, len(entries), 100, "Should respect page limit")
    assert.Contains(t, feed, "total", "Feed should have total count")
    assert.GreaterOrEqual(t, int(feed["total"].(float64)), numBooks-10, "Total should reflect all books")
}

File-by-File Plan

File: cmd/server/tests/opds_test.go

Add new test functions:

1. TestOPDS_EmptyLibrary (after existing tests)

   • Lines: Add ~50 lines
   • Purpose: OPDS feed with no books
   • Verification: Valid feed structure, empty entries

2. TestOPDS_LargeFeed (after previous test)

   • Lines: Add ~90 lines
   • Purpose: Library with 1000+ books
   • Verification: Pagination works, total count accurate

3. TestOPDS_SpecialCharacters (after previous test)

   • Lines: Add ~60 lines
   • Purpose: Book titles with Unicode, emojis, RTL
   • Verification: Proper XML/JSON encoding

4. TestOPDS_CorruptedMetadata (after previous test)

   • Lines: Add ~50 lines
   • Purpose: Book with invalid/malformed metadata
   • Verification: Feed generation continues, skips bad entries

Action: Add 4 new test functions to opds_test.go.

---

3.4 Book Matching Edge Cases

Problem Areas

Scenario	Missing Tests	Risk
No matches found	Book has no close matches	Empty results handling
Multiple equal matches	Same confidence for multiple books	Ambiguity handling
Special characters	Titles with quotes, apostrophes	Query failures
Unicode normalization	Different Unicode representations	Duplicate matches

Test Patterns

Pattern 1: No Matches Found

// NEW TEST: Book matching finds no results
func TestBookMatching_NoMatches(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-no-matches")

    // Device reports book that doesn't exist in library
    unmatchedBook := map[string]interface{}{
        "title": "Nonexistent Book Title That Definitely Doesn't Exist",
        "author": "Unknown Author",
        "file_path": "/mnt/sd/Nonexistent.epub",
        "file_size": 1024,
    }

    jsonData, _ := json.Marshal(unmatchedBook)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/sync/koreader/match", bytes.NewBuffer(jsonData))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+device.AuthToken)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()

    // NEW: Verify handling of no matches
    var response map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&response)
    
    // Should either:
    // 1. Return empty matches array
    // 2. Return null match with confidence 0
    // 3. Suggest manual match
    matches, hasMatches := response["matches"]
    if hasMatches {
        matchArray := matches.([]interface{})
        assert.Empty(t, matchArray, "Should have no matches")
    }
    
    // Or might have suggestion flag
    if suggestion, hasSuggestion := response["suggest_manual"]; hasSuggestion {
        assert.True(t, suggestion.(bool), "Should suggest manual match")
    }
}

Pattern 2: Multiple Equal Matches

// NEW TEST: Multiple books with same match confidence
func TestBookMatching_MultipleEqualConfidence(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    // Create library
    libReq := map[string]interface{}{
        "name": "Test Library",
        "type": "ebooks",
    }
    libBody, _ := json.Marshal(libReq)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()
    require.Equal(t, http.StatusCreated, resp.StatusCode)

    var libResponse map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&libResponse)
    libraryID := libResponse["id"].(string)

    // Add folder
    folderReq := map[string]interface{}{
        "folder_path": "/app/uploads",
    }
    folderBody, _ := json.Marshal(folderReq)
    req = httptest.NewRequest("POST", fmt.Sprintf("%s/api/libraries/%s/folders", setup.Server.URL, libraryID), bytes.NewBuffer(folderBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)
    resp, err = client.Do(req)
    require.NoError(t, err)
    resp.Body.Close()

    // Add multiple books with similar titles
    for _, title := range []string{"Book One", "Book Two", "Book Three"} {
        mediaReq := map[string]interface{}{
            "library_id": libraryID,
            "title": title,
            "author": "Same Author",
            "file_path": "/tmp/test.epub",
            "file_size": 1024,
            "mime_type": "application/epub+zip",
        }
        mediaBody, _ := json.Marshal(mediaReq)
        
        req = httptest.NewRequest("POST", setup.Server.URL+"/api/media-items", bytes.NewBuffer(mediaBody))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        resp, err = client.Do(req)
        require.NoError(t, err)
        resp.Body.Close()
    }

    // Device reports book with ambiguous title
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-ambiguous")
    ambiguousBook := map[string]interface{}{
        "title": "Book",  // Could match any of the three
        "author": "Same Author",
    }

    jsonData, _ := json.Marshal(ambiguousBook)
    req = httptest.NewRequest("POST", setup.Server.URL+"/api/sync/koreader/match", bytes.NewBuffer(jsonData))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+device.AuthToken)

    resp, err = client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()

    // NEW: Verify ambiguous match handling
    assert.Equal(t, http.StatusOK, resp.StatusCode)
    
    var response map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&response)
    
    // Should return multiple matches with similar confidence
    matches, hasMatches := response["matches"]
    require.True(t, hasMatches, "Should have matches")
    matchArray := matches.([]interface{})
    assert.Greater(t, len(matchArray), 1, "Should have multiple matches for ambiguous title")
    
    // All matches should have similar confidence scores
    if len(matchArray) > 1 {
        firstMatch := matchArray[0].(map[string]interface{})
        secondMatch := matchArray[1].(map[string]interface{})
        
        firstConfidence := firstMatch["confidence"].(float64)
        secondConfidence := secondMatch["confidence"].(float64)
        
        // Confidence scores should be similar (within 10%)
        confidenceDiff := firstConfidence - secondConfidence
        assert.Less(t, confidenceDiff, 0.1, "Similar matches should have close confidence")
    }
}

File-by-File Plan

File: cmd/server/tests/book_matching_test.go

Add new test functions:

1. TestBookMatching_NoMatches (after existing tests)

   • Lines: Add ~50 lines
   • Purpose: Book with no close matches
   • Verification: Empty matches or manual match suggestion

2. TestBookMatching_MultipleEqualConfidence (after previous test)

   • Lines: Add ~80 lines
   • Purpose: Multiple books with similar titles
   • Verification: Returns multiple matches, suggests disambiguation

3. TestBookMatching_SpecialCharacters (after previous test)

   • Lines: Add ~60 lines
   • Purpose: Titles with quotes, apostrophes, emojis
   • Verification: Proper query escaping, correct matches

4. TestBookMatching_UnicodeNormalization (after previous test)

   • Lines: Add ~60 lines
   • Purpose: Same title in different Unicode forms (NFD vs NFC)
   • Verification: Correct match despite Unicode differences

Action: Add 4 new test functions to book_matching_test.go.

---

Phase 3 Summary

New Test Functions: 16+
Lines Added: ~1000 lines
Error Path Coverage: WebSocket, sync, OPDS, matching
Edge Case Coverage: Empty, large, corrupted, special characters

Verification Steps:

1. ✅ WebSocket has error path tests
2. ✅ Sync has failure scenario tests
3. ✅ OPDS has edge case tests
4. ✅ Book matching has ambiguity tests

---

Phase 4: Load and Security

Priority: 🟢 LOW - Do after Phase 3
Time Estimate: 10-12 new test functions
Risk Level: Low (new tests only)
ROI: Medium - prevents DoS and encoding issues

Objectives

1. Add very large payload tests
2. Add comprehensive Unicode/special character tests
3. Add max boundary tests

---

4.1 Very Large Payloads

Problem Areas

Area	Current	Missing	Risk
Single test	edge_cases_test.go:327 - 100KB string	Bulk operations with 1000+ items	Memory exhaustion
Response size	Not tested	User with 10,000 books	Timeouts
WebSocket messages	Not tested	Very large sync payloads	Frame size limits
Upload limits	Not tested	Huge book files	Disk space exhaustion

Test Patterns

Pattern 1: Bulk Operation Limits

// NEW TEST: Bulk operation with maximum items
func TestBulk_MaximumItems(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)
    device := setup.CreateDevice(t, "Test Device", "koreader", "test-bulk-max")

    // Create library with folder
    libReq := map[string]interface{}{
        "name": "Bulk Test Library",
        "type": "ebooks",
    }
    libBody, _ := json.Marshal(libReq)
    req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)

    client := &http.Client{}
    resp, err := client.Do(req)
    require.NoError(t, err)
    defer resp.Body.Close()
    require.Equal(t, http.StatusCreated, resp.StatusCode)

    var libResponse map[string]interface{}
    json.NewDecoder(resp.Body).Decode(&libResponse)
    libraryID := libResponse["id"].(string)

    // Add folder
    folderReq := map[string]interface{}{
        "folder_path": "/app/uploads",
    }
    folderBody, _ := json.Marshal(folderReq)
    req = httptest.NewRequest("POST", fmt.Sprintf("%s/api/libraries/%s/folders", setup.Server.URL, libraryID), bytes.NewBuffer(folderBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)
    resp, err = client.Do(req)
    require.NoError(t, err)
    resp.Body.Close()

    // Create many books (1000 items)
    const numBooks = 1000
    var mediaIDs []string
    for i := 0; i < numBooks; i++ {
        mediaReq := map[string]interface{}{
            "library_id": libraryID,
            "title": fmt.Sprintf("Book %d", i),
            "author": "Test Author",
            "file_path": fmt.Sprintf("/tmp/test%d.epub", i),
            "file_size": 1024,
            "mime_type": "application/epub+zip",
        }
        mediaBody, _ := json.Marshal(mediaReq)
        
        req = httptest.NewRequest("POST", setup.Server.URL+"/api/media-items", bytes.NewBuffer(mediaBody))
        req.Header.Set("Content-Type", "application/json")
        req.Header.Set("Authorization", "Bearer "+token)
        resp, err = client.Do(req)
        if err != nil {
            t.Logf("Failed to create book %d: %v", i, err)
            continue
        }
        defer resp.Body.Close()
        
        if resp.StatusCode == http.StatusCreated {
            var mediaResponse map[string]interface{}
            json.NewDecoder(resp.Body).Decode(&mediaResponse)
            mediaIDs = append(mediaIDs, mediaResponse["id"].(string))
        }
    }

    // Test bulk update with all 1000 items
    bulkUpdateReq := map[string]interface{}{
        "media_item_ids": mediaIDs,
        "tags": []string{"bulk-updated"},
    }
    bulkBody, _ := json.Marshal(bulkUpdateReq)
    
    req = httptest.NewRequest("POST", setup.Server.URL+"/api/media-items/bulk-update", bytes.NewBuffer(bulkBody))
    req.Header.Set("Content-Type", "application/json")
    req.Header.Set("Authorization", "Bearer "+token)
    
    start := time.Now()
    resp, err = client.Do(req)
    duration := time.Since(start)
    
    if err != nil {
        t.Logf("Bulk update with %d items failed: %v", numBooks, err)
        return
    }
    defer resp.Body.Close()

    // NEW: Verify behavior under load
    t.Logf("Bulk update of %d items took %v", numBooks, duration)
    
    // Should succeed but might take time
    if resp.StatusCode == http.StatusOK {
        var result map[string]interface{}
        json.NewDecoder(resp.Body).Decode(&result)
        
        // Verify count
        if total, ok := result["total"]; ok {
            assert.Equal(t, numBooks, int(total.(float64)), "Should process all items")
        }
        
        // Verify DB state for sample
        if len(mediaIDs) > 0 {
            pgID := pgtype.UUID{Bytes: [16]byte(mediaIDs[0]), Valid: true}
            media, err := setup.DB.GetMediaItem(context.Background(), pgID)
            require.NoError(t, err)
            assert.Contains(t, media.Tags, "bulk-updated", "Should have updated tag")
        }
    } else if resp.StatusCode == http.StatusRequestEntityTooLarge {
        t.Logf("Bulk update rejected as too large (acceptable)")
    } else {
        t.Logf("Unexpected status: %d", resp.StatusCode)
    }
}

Pattern 2: Response Size Limits

// NEW TEST: API response with very large payload
func TestResponse_VeryLargePayload(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    // Create library with many books (from previous test or reuse)
    // Assuming we have 1000 books from previous test
    
    // Request list of all books (large response)
    req := httptest.NewRequest("GET", setup.Server.URL+"/api/media-items?limit=10000", nil)
    req.Header.Set("Authorization", "Bearer "+token)
    
    client := &http.Client{Timeout: 30 * time.Second}
    start := time.Now()
    resp, err := client.Do(req)
    duration := time.Since(start)
    
    if err != nil {
        t.Logf("Large payload request failed: %v", err)
        return
    }
    defer resp.Body.Close()

    // NEW: Verify large response handling
    t.Logf("Large payload response (%d status) took %v", resp.StatusCode, duration)
    
    if resp.StatusCode == http.StatusOK {
        // Read body (might be large)
        body, err := io.ReadAll(resp.Body)
        if err != nil {
            t.Logf("Failed to read large response: %v", err)
            return
        }
        
        t.Logf("Response size: %d bytes", len(body))
        
        // Should have reasonable response size (< 10MB)
        assert.Less(t, len(body), 10*1024*1024, "Response should be < 10MB")
        
        // Verify response is valid JSON
        var response map[string]interface{}
        err = json.Unmarshal(body, &response)
        assert.NoError(t, err, "Large response should be valid JSON")
        
        // Verify pagination
        if items, ok := response["items"]; ok {
            itemArray := items.([]interface{})
            t.Logf("Returned %d items", len(itemArray))
        }
    } else if resp.StatusCode == http.StatusRequestEntityTooLarge {
        t.Logf("Large payload rejected (acceptable)")
    }
}

File-by-File Plan

File: cmd/server/tests/edge_cases_test.go

Add new test functions:

1. TestBulk_MaximumItems (after existing edge cases)

   • Lines: Add ~120 lines
   • Purpose: Bulk operation with 1000 items
   • Verification: Succeeds or rejected gracefully, no crashes

2. TestResponse_VeryLargePayload (after previous test)

   • Lines: Add ~80 lines
   • Purpose: Request all items (large response)
   • Verification: Reasonable size, valid JSON, proper pagination

3. TestWebSocket_LargeMessage (after previous test)

   • Lines: Add ~60 lines
   • Purpose: Send very large WebSocket message
   • Verification: Rejected or handled gracefully

Action: Add 3 new test functions to edge_cases_test.go.

---

4.2 Comprehensive Unicode Tests

Problem Areas

Character Type	Missing Tests	Risk
Emojis	Only 1 test	Database encoding issues
RTL languages	None	Display corruption
Zero-width characters	None	Silent duplicates
Unicode normalization	None	Same text appears different
Combining characters	None	Search failures

Test Pattern

// NEW TEST: Comprehensive Unicode handling
func TestUnicode_Comprehensive(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    testCases := []struct {
        name      string
        title     string
        author    string
        expectOK  bool
    }{
        {
            name:     "Emojis in title",
            title:    "The Great Adventure 🚀📚✨",
            author:   "Jane Doe",
            expectOK: true,
        },
        {
            name:     "Multiple emojis",
            title:    "📖 Book 🌟 of 🎉 Joy 😊",
            author:   "Happy Author 👩‍🚀",
            expectOK: true,
        },
        {
            name:     "RTL text (Arabic)",
            title:    "كتاب عظيم",
            author:   "مؤلف بارز",
            expectOK: true,
        },
        {
            name:     "RTL text (Hebrew)",
            title:    "ספר נהדר",
            author:   "סופר מפורסם",
            expectOK: true,
        },
        {
            name:     "Mixed RTL and LTR",
            title:    "Hello שלום Bonjour",
            author:   "Author نبي",
            expectOK: true,
        },
        {
            name:     "Zero-width characters",
            title:    "Book\u200BTitle",  // Zero-width space
            author:   "Author\u200CName", // Zero-width non-joiner
            expectOK: true,
        },
        {
            name:     "Combining diacritics",
            title:    "Café",  // Precomposed é
            author:   "cafe\u0301",  // Combining acute
            expectOK: true,
        },
        {
            name:     "Multiple combining marks",
            title:    "ñaïve",  // Precomposed ï
            author:   "nai\u0308ve",  // Combining diaeresis
            expectOK: true,
        },
        {
            name:     "CJK characters",
            title:    "伟大的书",
            author:   "作者",
            expectOK: true,
        },
        {
            name:     "Special punctuation",
            title:    "Book: A Story; Of—Things—Like—This",
            author:   "O'Brien, St. John",
            expectOK: true,
        },
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            // Create library
            libReq := map[string]interface{}{
                "name": fmt.Sprintf("Unicode Library %s", tc.name),
                "type": "ebooks",
            }
            libBody, _ := json.Marshal(libReq)
            req := httptest.NewRequest("POST", setup.Server.URL+"/api/libraries", bytes.NewBuffer(libBody))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)

            client := &http.Client{}
            resp, err := client.Do(req)
            require.NoError(t, err)
            defer resp.Body.Close()
            require.Equal(t, http.StatusCreated, resp.StatusCode)

            var libResponse map[string]interface{}
            json.NewDecoder(resp.Body).Decode(&libResponse)
            libraryID := libResponse["id"].(string)

            // Add folder
            folderReq := map[string]interface{}{
                "folder_path": "/app/uploads",
            }
            folderBody, _ := json.Marshal(folderReq)
            req = httptest.NewRequest("POST", fmt.Sprintf("%s/api/libraries/%s/folders", setup.Server.URL, libraryID), bytes.NewBuffer(folderBody))
            req.Header.Set("Content-Type", "application/json")
            req.Header.Set("Authorization", "Bearer "+token)
            resp, err = client.Do(req)
            require.NoError(t, err)
            resp.Body.Close()

            // Create book with Unicode title/author
            mediaReq := map[string]interface{}{
                "library_id": libraryID,
                "title":      tc.title,
                "author":     tc.author,
                "file_path":  "/tmp/test.epub",
                "file_size":  1024,
                "mime_type":  "application/epub+zip",
            }
            mediaBody, _ := json.Marshal(mediaReq)

            req = httptest.NewRequest("POST", setup.Server.URL+"/api/media-items", bytes.NewBuffer(mediaBody))
            req.Header.Set("Content-Type", "application/json; charset=utf-8")
            req.Header.Set("Authorization", "Bearer "+token)
            resp, err = client.Do(req)
            require.NoError(t, err)
            defer resp.Body.Close()

            if tc.expectOK {
                assert.Equal(t, http.StatusCreated, resp.StatusCode, tc.name)
                
                // NEW: Verify database stores Unicode correctly
                var mediaResponse map[string]interface{}
                json.NewDecoder(resp.Body).Decode(&mediaResponse)
                mediaID := mediaResponse["id"].(string)
                
                pgID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
                media, err := setup.DB.GetMediaItem(context.Background(), pgID)
                require.NoError(t, err, "Should retrieve media item")
                
                // Verify Unicode preserved
                assert.Equal(t, tc.title, media.Title, "Title should preserve Unicode")
                assert.Equal(t, tc.author, media.Author.String, "Author should preserve Unicode")
                
                // Test search with Unicode
                searchReq := httptest.NewRequest("GET", fmt.Sprintf("%s/api/search?q=%s", setup.Server.URL, url.QueryEscape(tc.title)), nil)
                searchReq.Header.Set("Authorization", "Bearer "+token)
                searchResp, err := client.Do(searchReq)
                require.NoError(t, err)
                defer searchResp.Body.Close()
                
                assert.Equal(t, http.StatusOK, searchResp.StatusCode, "Search should work with Unicode")
            } else {
                assert.NotEqual(t, http.StatusCreated, resp.StatusCode, tc.name)
            }
        })
    }
}

File-by-File Plan

File: cmd/server/tests/edge_cases_test.go

Add new test function:

1. TestUnicode_Comprehensive (after existing edge cases)
   • Lines: Add ~150 lines
   • Purpose: Emojis, RTL, zero-width, combining marks, CJK
   • Verification: Stored correctly, searchable

Action: Add new test function to edge_cases_test.go.

---

4.3 Max Boundary Tests

Problem Areas

Field Type	Missing	Risk
Email	Exact boundary (254, 255, 256)	Truncation
Username	Boundary (49, 50, 51, 500, 5000)	Validation inconsistency
Device name	Max length	DB constraint violation
Book title	Max length	Display issues
Array items	Max items in bulk	Performance degradation

Test Pattern

// NEW TEST: Boundary value testing
func TestBoundaries_MaxLengths(t *testing.T) {
    setup := setupTestServer(t)
    token := loginTestUser(t, setup.Server, setup.DB)

    t.Run("Email at exact boundaries", func(t *testing.T) {
        testCases := []struct {
            name     string
            email    string
            expectOK bool
        }{
            {"254 chars (one under limit)", strings.Repeat("a", 250) + "@example.com", true},
            {"255 chars (exact limit)", strings.Repeat("a", 251) + "@example.com", true},
            {"256 chars (one over limit)", strings.Repeat("a", 252) + "@example.com", false},
            {"Very long (5000 chars)", strings.Repeat("a", 4995) + "@example.com", false},
        }

        for _, tc := range testCases {
            t.Run(tc.name, func(t *testing.T) {
                // Create user with boundary email
                // ... test code ...
            })
        }
    })

    t.Run("Username at exact boundaries", func(t *testing.T) {
        testCases := []struct {
            name     string
            username  string
            expectOK bool
        }{
            {"2 chars (one under min)", "ab", false},
            {"3 chars (exact min)", "abc", true},
            {"50 chars (exact max)", strings.Repeat("a", 50), true},
            {"51 chars (one over max)", strings.Repeat("a", 51), false},
            {"500 chars (way over)", strings.Repeat("a", 500), false},
            {"5000 chars (extreme)", strings.Repeat("a", 5000), false},
        }

        for _, tc := range testCases {
            t.Run(tc.name, func(t *testing.T) {
                // Create user with boundary username
                // ... test code ...
            })
        }
    })

    t.Run("Device name at max length", func(t *testing.T) {
        // Test device name at database column limit (likely 255)
        // ... test code ...
    })

    t.Run("Book title at max length", func(t *testing.T) {
        // Test title at database column limit
        // ... test code ...
    })

    t.Run("Bulk array size limits", func(t *testing.T) {
        // Test bulk operations with arrays at limits
        // ... test code ...
    })
}

File-by-File Plan

File: cmd/server/tests/edge_cases_test.go

Add new test function:

1. TestBoundaries_MaxLengths (after Unicode tests)
   • Lines: Add ~200 lines
   • Purpose: Exact boundary testing for all text fields
   • Verification: Consistent validation, no truncation

Action: Add new test function to edge_cases_test.go.

---

Phase 4 Summary

New Test Functions: 5+
Lines Added: ~600 lines
Load Testing: Bulk operations, large responses
Unicode Coverage: Emojis, RTL, zero-width, combining, CJK
Boundary Testing: Exact limits for all text fields

Verification Steps:

1. ✅ Large payloads handled gracefully
2. ✅ Unicode characters work end-to-end
3. ✅ Boundary values consistent

---

Implementation Order

Phase 1: Stop Silent Failures (HIGHEST ROI)

Week 1-2:

1. Replace map-based assertions with structs (12 files)
2. Add database verification (8 files)
3. Add null/empty/missing tests (3 files)

Deliverables:

• All tests use struct-based assertions
• All mutations verify database state
• Critical edge cases covered

Verification:

# All tests compile
go test ./cmd/server/tests/... -v

# No map[string]interface{} in assertions
grep -r "map\[string\]interface" cmd/server/tests/ | wc -l  # Should be 0

# All mutations have DB verification
# (Manual review of test files)

---

Phase 2: Concurrency Protection (HIGH)

Week 3:

1. Add sync race condition tests (4 tests)
2. Add bulk concurrent tests (3 tests)
3. Add profile + device tests (2 tests)

Deliverables:

• All sync operations have concurrent tests
• All bulk operations have concurrent tests
• Profile + device interactions tested

Verification:

# Run with race detector
go test ./cmd/server/tests/... -race -v

# All concurrent tests pass

---

Phase 3: Hardened Error Handling (MEDIUM)

Week 4:

1. Add WebSocket error paths (4 tests)
2. Add sync failures (4 tests)
3. Add OPDS edge cases (4 tests)
4. Add book matching edge cases (4 tests)

Deliverables:

• WebSocket has comprehensive error tests
• Sync has failure scenario tests
• OPDS handles edge cases
• Book matching handles ambiguity

Verification:

# All error path tests pass
go test ./cmd/server/tests/... -v -run "TestWebSocket|TestSync|TestOPDS|TestBookMatching"

---

Phase 4: Load and Security (LOW)

Week 5:

1. Add large payload tests (3 tests)
2. Add Unicode tests (1 comprehensive test)
3. Add boundary tests (1 comprehensive test)

Deliverables:

• Large payloads handled gracefully
• Unicode works end-to-end
• Boundary values consistent

Verification:

# All load/security tests pass
go test ./cmd/server/tests/edge_cases_test.go -v

---

Verification Checklist

Pre-Implementation

• All current tests pass

  go test ./cmd/server/tests/... -v
  

• No compilation errors

  go build ./...
  

• Guidelines verification passes

  bash scripts/verify-guidelines.sh
  

Phase 1 Verification

• All map[string]interface{} replaced with structs

  grep -r "map\[string\]interface" cmd/server/tests/*.go | wc -l  # Should be 0
  

• All mutation operations have DB verification

  # Manual review: Every POST/PUT/DELETE test should query DB after
  

• Null/empty/missing tests added for critical fields

  grep -r "NullVsEmpty" cmd/server/tests/*.go | wc -l  # Should be > 0
  

Phase 2 Verification

• All sync operations have concurrent tests

  grep -r "Concurrent" cmd/server/tests/sync_integration_test.go | wc -l  # Should be > 0
  

• All bulk operations have concurrent tests

  grep -r "Concurrent" cmd/server/tests/*_bulk_test.go | wc -l  # Should be > 0
  

• Race detector passes

  go test ./cmd/server/tests/... -race -v
  

Phase 3 Verification

• WebSocket has error path tests

  grep -r "DisconnectMid\|MalformedFrames\|Timeout" cmd/server/tests/websocket_test.go | wc -l  # Should be > 0
  

• Sync has failure tests

  grep -r "Offline\|QueueOverflow\|Conflicting" cmd/server/tests/sync_integration_test.go | wc -l  # Should be > 0
  

• OPDS has edge case tests

  grep -r "Empty\|Large\|Special" cmd/server/tests/opds_test.go | wc -l  # Should be > 0
  

• Book matching has ambiguity tests

  grep -r "NoMatches\|MultipleEqual\|Unicode" cmd/server/tests/book_matching_test.go | wc -l  # Should be > 0
  

Phase 4 Verification

• Large payload tests exist

  grep -r "Maximum\|VeryLarge" cmd/server/tests/edge_cases_test.go | wc -l  # Should be > 0
  

• Unicode tests exist

  grep -r "Emoji\|RTL\|Zero-width\|Combining" cmd/server/tests/edge_cases_test.go | wc -l  # Should be > 0
  

• Boundary tests exist

  grep -r "Boundaries\|254\|255\|256" cmd/server/tests/edge_cases_test.go | wc -l  # Should be > 0
  

Post-Implementation

• All tests pass

  go test ./cmd/server/tests/... -v
  

• No compilation errors

  go build ./...
  

• Guidelines verification passes

  bash scripts/verify-guidelines.sh
  

• Test coverage increased

  go test ./cmd/server/tests/... -cover
  

---

Success Metrics

Before

• 507+ map-based assertions (brittle)
• 0 concurrency tests
• 0 database verification for mutations
• 0 null/empty/missing edge cases
• Happy path only for WebSocket, sync, OPDS, matching

After (All Phases Complete)

• 0 map-based assertions (100% struct-based)
• 10+ concurrency tests
• 100% database verification for mutations
• 15+ null/empty/missing edge case tests
• Comprehensive error paths for WebSocket, sync, OPDS, matching
• Load testing for bulk operations
• Unicode coverage for emojis, RTL, zero-width, combining marks
• Boundary testing for all text fields

Reliability Score

Area	Before	After	Improvement
API contract safety	❌ 0%	✅ 100%	Compile-time detection
Data integrity	⚠️ 20%	✅ 100%	DB verification
Concurrency	❌ 0%	✅ 80%	Race tests
Edge cases	⚠️ 10%	✅ 90%	Comprehensive
Error handling	⚠️ 30%	✅ 90%	Failure scenarios
Load handling	⚠️ 5%	✅ 70%	Large payloads
Overall	13%	90%	+77%

---

Notes

1. Follow PROJECT_GUIDELINES.md:

   • Use setupTestServer() from test_helpers.go
   • Share one test setup across subtests (call once at function level)
   • Use table-driven tests with t.Run()
   • Configure database with max_conns=1 via pgxpool.ParseConfig()

2. No cascading fixes: Stop on compilation error, review git diff, recover deliberately

3. Post-edit verification: Run go build for affected packages after each edit

4. Multiple logical commits: Commit changes in logical steps with clear messages

5. Test execution speed: If tests become too slow, consider:

   • Parallel test execution (t.Parallel())
   • Test-specific database fixtures
   • Reduced dataset sizes for load tests

---

Appendix: Conversion Examples

Example 1: Simple Response

BEFORE:

var response map[string]interface{}
json.Unmarshal(body, &response)
deviceName := response["device"].(map[string]interface{})["device_name"].(string)
assert.Equal(t, "Test Device", deviceName)

AFTER:

import "bookhoard/internal/handlers"

var response handlers.DeviceListResponse
err := json.Unmarshal(body, &response)
require.NoError(t, err)
assert.Equal(t, "Test Device", response.Devices[0].DeviceName)

Example 2: Array Response

BEFORE:

var response map[string]interface{}
json.Unmarshal(body, &response)
devices := response["devices"].([]interface{})
for _, d := range devices {
    device := d.(map[string]interface{})
    name := device["device_name"].(string)
    // ...
}

AFTER:

import "bookhoard/internal/handlers"

var response handlers.DeviceListResponse
err := json.Unmarshal(body, &response)
require.NoError(t, err)
for _, device := range response.Devices {
    name := device.DeviceName
    // ...
}

Example 3: Database Verification

BEFORE:

assert.Equal(t, http.StatusNoContent, rec.Code)

AFTER:

assert.Equal(t, http.StatusNoContent, rec.Code)

pgID := pgtype.UUID{Bytes: [16]byte(id), Valid: true}
_, err := setup.DB.GetDevice(context.Background(), pgID)
assert.Error(t, err, "Device should be deleted")

---

Appendix: test_helpers.go Improvements

Critical Bug Fixes

Issue 1: Return Type Mismatch (Line 210)

Problem:

// Line 210 - WRONG: Returns UserTestData instead of error
return fmt.Errorf("user already exists: %s", existingUser.Email)

Fix:

// Line 210 - CORRECT: Return error
return fmt.Errorf("user already exists: %s", existingUser.Email), UserTestData{}
// OR remove the early return entirely since user exists check should delete and recreate

Impact: This causes compilation errors and prevents createTestUserOnce from working correctly.

---

Issue 2: Dead Code (Lines 207-214)

Problem:

// Lines 207-214: Early return makes code below unreachable
if err == nil {
    return fmt.Errorf("user already exists: %s", existingUser.Email)
}
return UserTestData{}  // NEVER REACHED

// Lines 216-236: Never executed due to early return
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
user, err := db.CreateUser(ctx, database.CreateUserParams{
    // ...
})

Fix:

// Remove early return and always delete + recreate
ctx := context.Background()

// Check if user exists and delete for fresh state
user, err := db.GetUserByEmail(ctx, "testuser@example.com")
if err == nil {
    // User exists, delete them to ensure fresh password
    err = db.DeleteUser(ctx, user.ID)
    if err != nil {
        // If delete fails (user might be referenced elsewhere), log and continue
        t.Logf("Warning: Could not delete existing test user: %v", err)
    }
}

// Create a fresh test user with a valid password
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
newUser, err := db.CreateUser(ctx, database.CreateUserParams{
    Email:        "testuser@example.com",
    Username:     "testuser",
    PasswordHash: passwordHash,
    FirstName:    pgtype.Text{String: "Test", Valid: true},
    LastName:     pgtype.Text{String: "User", Valid: true},
    Role:         "admin",
})
require.NoError(t, err, "Failed to create test user")

userUUID, err := uuid.FromBytes(newUser.ID.Bytes[:])
require.NoError(t, err, "Failed to parse user UUID")
return UserTestData{
    ID:       userUUID,
    Email:    "testuser@example.com",
    Username:  "testuser",
    Password: "Test@Pass123!",
}

Impact: Dead code prevents test user creation from working properly.

---

New Helper Functions

Add these helpers to test_helpers.go to reduce code duplication across tests and provide consistent database verification.

Helper 1: Database Verification Functions

// Add to test_helpers.go after line 562

// verifyDeviceCreated verifies a device exists in database with expected values
func verifyDeviceCreated(t *testing.T, db *database.Queries, deviceID uuid.UUID, expected DeviceTestData) {
    pgDeviceID := pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}
    device, err := db.GetDevice(context.Background(), pgDeviceID)
    require.NoError(t, err, "Device should exist in database")
    
    assert.Equal(t, expected.Name, device.DeviceName, "Device name should match")
    assert.Equal(t, expected.Type, device.DeviceType, "Device type should match")
    assert.Equal(t, expected.Identifier, device.DeviceIdentifier, "Device identifier should match")
    assert.NotEmpty(t, device.AuthToken, "Device should have auth token")
}

// verifyDeviceDeleted verifies a device does not exist in database
func verifyDeviceDeleted(t *testing.T, db *database.Queries, deviceID uuid.UUID) {
    pgDeviceID := pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}
    _, err := db.GetDevice(context.Background(), pgDeviceID)
    assert.Error(t, err, "Device should be deleted from database")
}

// verifyUserUpdated verifies a user has expected field values in database
func verifyUserUpdated(t *testing.T, db *database.Queries, userID uuid.UUID, expected map[string]interface{}) {
    pgUserID := pgtype.UUID{Bytes: [16]byte(userID), Valid: true}
    user, err := db.GetUser(context.Background(), pgUserID)
    require.NoError(t, err, "User should exist in database")
    
    if firstName, ok := expected["first_name"]; ok {
        if fn, ok := firstName.(string); ok {
            assert.Equal(t, fn, user.FirstName.String, "First name should match")
        }
    }
    if lastName, ok := expected["last_name"]; ok {
        if ln, ok := lastName.(string); ok {
            assert.Equal(t, ln, user.LastName.String, "Last name should match")
        }
    }
    if email, ok := expected["email"]; ok {
        if em, ok := email.(string); ok {
            assert.Equal(t, em, user.Email, "Email should match")
        }
    }
}

// verifyMediaItemUpdated verifies a media item has expected values in database
func verifyMediaItemUpdated(t *testing.T, db *database.Queries, mediaID uuid.UUID, expected map[string]interface{}) {
    pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
    media, err := db.GetMediaItem(context.Background(), pgMediaID)
    require.NoError(t, err, "Media item should exist in database")
    
    if title, ok := expected["title"]; ok {
        if t, ok := title.(string); ok {
            assert.Equal(t, t, media.Title, "Title should match")
        }
    }
    if tags, ok := expected["tags"]; ok {
        if tagArray, ok := tags.([]string); ok {
            assert.ElementsMatch(t, tagArray, media.Tags, "Tags should match")
        }
    }
    // Add more fields as needed
}

Usage in tests:

// Instead of manual DB queries:
device := setup.CreateDevice(t, "Test Device", "koreader", "test-123")

// NEW: Use helper
verifyDeviceCreated(t, setup.DB, device.ID, DeviceTestData{
    Name: "Test Device",
    Type: "koreader",
    Identifier: "test-123",
})

---

Helper 2: Request Builder Functions

// Add to test_helpers.go after database verification functions

// buildLoginRequest creates a login request map
func buildLoginRequest(email, password string) map[string]interface{} {
    return map[string]interface{}{
        "login":    email,
        "password": password,
    }
}

// buildDeviceUpdateRequest creates a device update request map
func buildDeviceUpdateRequest(name string, syncEnabled *bool, syncFreq *int32) map[string]interface{} {
    req := map[string]interface{}{}
    if name != "" {
        req["device_name"] = name
    }
    if syncEnabled != nil {
        req["sync_enabled"] = *syncEnabled
    }
    if syncFreq != nil {
        req["sync_frequency_minutes"] = *syncFreq
    }
    return req
}

// buildMediaItemRequest creates a media item request map
func buildMediaItemRequest(libraryID, title, author, filePath, mimeType string, fileSize int64) map[string]interface{} {
    return map[string]interface{}{
        "library_id": libraryID,
        "title":      title,
        "author":     author,
        "file_path":  filePath,
        "file_size":  fileSize,
        "mime_type":  mimeType,
    }
}

// buildUserUpdateRequest creates a user profile update request map
func buildUserUpdateRequest(firstName, lastName, email, theme string) map[string]interface{} {
    req := map[string]interface{}{}
    if firstName != "" {
        req["first_name"] = firstName
    }
    if lastName != "" {
        req["last_name"] = lastName
    }
    if email != "" {
        req["email"] = email
    }
    if theme != "" {
        req["theme"] = theme
    }
    return req
}

Usage in tests:

// BEFORE: Manually build maps
loginRequest := map[string]interface{}{
    "login":    email,
    "password": password,
}

// AFTER: Use helper
loginRequest := buildLoginRequest(email, password)

---

Helper 3: Struct-Based Response Assertions

// Add to test_helpers.go after request builder functions

import (
    "bookhoard/internal/handlers"
    // ... existing imports
)

// assertDeviceListResponse parses and asserts DeviceListResponse
func assertDeviceListResponse(t *testing.T, body []byte) handlers.DeviceListResponse {
    var response handlers.DeviceListResponse
    err := json.Unmarshal(body, &response)
    require.NoError(t, err, "Response should match DeviceListResponse schema")
    return response
}

// assertDeviceUpdateResponse parses and asserts DeviceUpdateResponse
func assertDeviceUpdateResponse(t *testing.T, body []byte) handlers.DeviceUpdateResponse {
    var response handlers.DeviceUpdateResponse
    err := json.Unmarshal(body, &response)
    require.NoError(t, err, "Response should match DeviceUpdateResponse schema")
    return response
}

// assertLoginResponse parses and asserts LoginResponse
func assertLoginResponse(t *testing.T, body []byte) handlers.LoginResponse {
    var response handlers.LoginResponse
    err := json.Unmarshal(body, &response)
    require.NoError(t, err, "Response should match LoginResponse schema")
    return response
}

// assertMediaListResponse parses and asserts MediaListResponse
func assertMediaListResponse(t *testing.T, body []byte) handlers.MediaListResponse {
    var response handlers.MediaListResponse
    err := json.Unmarshal(body, &response)
    require.NoError(t, err, "Response should match MediaListResponse schema")
    return response
}

// assertLibraryResponse parses and asserts LibraryResponse
func assertLibraryResponse(t *testing.T, body []byte) handlers.LibraryResponse {
    var response handlers.LibraryResponse
    err := json.Unmarshal(body, &response)
    require.NoError(t, err, "Response should match LibraryResponse schema")
    return response
}

Usage in tests:

// BEFORE: Map-based parsing
var response map[string]interface{}
json.Unmarshal(rec.Body.Bytes(), &response)
deviceName := response["device"].(map[string]interface{})["device_name"].(string)
assert.Equal(t, "Test Device", deviceName)

// AFTER: Struct-based parsing
response := assertDeviceListResponse(t, rec.Body.Bytes())
assert.Equal(t, "Test Device", response.Devices[0].DeviceName)

---

Helper 4: Concurrent Test Utilities

// Add to test_helpers.go after assertion helpers

// runConcurrent executes functions concurrently and waits for completion
func runConcurrent(t *testing.T, numWorkers int, fns []func() error) []error {
    errors := make(chan error, len(fns))
    var wg sync.WaitGroup
    
    for i := 0; i < len(fns); i++ {
        wg.Add(1)
        go func(idx int) {
            defer wg.Done()
            if err := fns[idx](); err != nil {
                errors <- err
            }
        }(i)
    }
    
    wg.Wait()
    close(errors)
    
    var allErrors []error
    for err := range errors {
        allErrors = append(allErrors, err)
    }
    return allErrors
}

// retryOperation retries a function with exponential backoff
func retryOperation(maxRetries int, initialDelay time.Duration, fn func() error) error {
    var err error
    delay := initialDelay
    
    for attempt := 0; attempt < maxRetries; attempt++ {
        if err = fn(); err == nil {
            return nil
        }
        
        if attempt < maxRetries-1 {
            time.Sleep(delay)
            delay *= 2 // Exponential backoff
        }
    }
    return err
}

Usage in tests:

// BEFORE: Manual goroutine management
var wg sync.WaitGroup
errors := make(chan error, 3)
for i := 0; i < 3; i++ {
    wg.Add(1)
    go func(idx int) {
        defer wg.Done()
        // ... operation ...
    }(i)
}
wg.Wait()
close(errors)

// AFTER: Helper function
fns := []func() error{
    func() error { /* operation 1 */ return nil },
    func() error { /* operation 2 */ return nil },
    func() error { /* operation 3 */ return nil },
}
errors := runConcurrent(t, len(fns), fns)

---

Updated test_helpers.go Structure

// Line 1: Add handlers import
import (
    "bookhoard/internal/config"
    "bookhoard/internal/database"
    "bookhoard/internal/handlers"  // NEW
    "bookhoard/internal/middleware"
    // ... rest of imports
)

// Line 210: FIX return type
// BEFORE:
return fmt.Errorf("user already exists: %s", existingUser.Email)

// AFTER:
// Remove early return, let code continue to delete + recreate

// Line 213-236: FIX dead code
// These lines are now reachable after removing early return
// Keep the CreateUser logic

// After line 562: ADD new helper functions
// (See Helper Functions section above)

---

File-by-File Changes: test_helpers.go

Lines	Change	Impact
4-31	Add "bookhoard/internal/handlers" import	Enables struct-based assertions
210	Fix return type from UserTestData to error, or remove early return	Fixes compilation error
207-214	Remove early return or fix dead code	Makes user creation code reachable
562+	Add verification helpers (5 functions)	Reduces duplication, ensures DB verification
562+	Add request builders (4 functions)	Reduces map duplication
562+	Add struct assertion helpers (5 functions)	Enables compile-time safety
562+	Add concurrent utilities (2 functions)	Simplifies concurrent tests

Total Lines Added: ~300 lines
Total Lines Modified: ~10 lines
Functions Added: 16 new helper functions

---

Summary of test_helpers.go Improvements

Category	Before	After	Improvement
Critical Bugs	2 compilation errors	0	All tests compile
Code Duplication	Map-based requests repeated everywhere	Shared builder functions	Consistent request building
Database Verification	Manual queries scattered	Helper functions	Guaranteed verification
Type Safety	map[string]interface{}	Struct-based assertions	Compile-time error detection
Concurrent Tests	Manual goroutine management	Utility functions	Consistent patterns
Maintainability	Logic duplicated in tests	Centralized helpers	Easier to update

---

END OF PLAN

---

END OF COMPREHENSIVE REVIEW REPORT

Date: 2025-02-13
Scope: Complete review of entire test system (30 test files, ~10,941 lines)
Purpose: Identify ALL gaps in TEST_RELIABILITY_PLAN.md and ensure complete coverage

---

Executive Summary

The comprehensive review identified 69 individual issues across 30 test files. Critical findings:

Most Critical Gaps

Gap	Impact	Priority
test_helpers.go has 2 critical bugs	Tests fail compilation, user creation broken	🔴 CRITICAL
69% of tests use map-based assertions	Silent API changes, type errors	🔴 CRITICAL
85% of tests lack DB verification	Data corruption, silent failures	🔴 CRITICAL
96% of tests lack concurrency	Race conditions in production	🟠 HIGH
10 test files not in plan	Missing coverage, unknown gaps	🟡 MEDIUM
Zero Unicode edge cases	Encoding failures, search issues	🟠 HIGH
Unit tests give false confidence	Mock handlers != real integration	🟡 MEDIUM

Recommended Priority Change

Original: Phase 1 → 2 → 3 → 4
Revised: Phase 0 (NEW) → 1 → 2 → 3

Phase 0: Fix Test Infrastructure (NEW - 2-3 hours)

1. Fix 2 critical bugs in test_helpers.go
2. Add 6 missing helper functions
3. Move Unicode tests from Phase 4 → Phase 1
4. Create test isolation improvements

Rationale: Tests depend on test_helpers.go. Bugs there block all other work. Unicode too important to delay.