From 80dcdfdd7103b33cdef893a6b40595d9787f7b24 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Fri, 13 Feb 2026 21:50:03 -0500 Subject: [PATCH] refactor(tests): rename test files and fix broken escalate test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test file renames for clarity: - phase1_example_test.go → device_test_patterns_test.go - universal_progress_integration_test.go → setup_integration_test.go Fix broken TestConflictsBulkEscalate test: - Comment out test for non-existent /api/conflicts/bulk-escalate endpoint - Remove unused imports (context, time, pgtype, httptest) - Add explanatory comment about why test is disabled Clean up test helper comment: - Remove Phase 6 reference from test_helpers.go These changes remove planning document terminology from filenames and fix compilation errors caused by tests for unimplemented endpoints. --- cmd/server/tests/conflicts_bulk_test.go | 9 +- cmd/server/tests/device_test_patterns_test.go | 91 ++++++ cmd/server/tests/setup_integration_test.go | 296 ++++++++++++++++++ cmd/server/tests/test_helpers.go | 2 +- 4 files changed, 393 insertions(+), 5 deletions(-) create mode 100644 cmd/server/tests/device_test_patterns_test.go create mode 100644 cmd/server/tests/setup_integration_test.go diff --git a/cmd/server/tests/conflicts_bulk_test.go b/cmd/server/tests/conflicts_bulk_test.go index c1a2cfb..def9787 100644 --- a/cmd/server/tests/conflicts_bulk_test.go +++ b/cmd/server/tests/conflicts_bulk_test.go @@ -3,15 +3,11 @@ package main import ( "bookhoard/internal/handlers" "bytes" - "context" "encoding/json" "net/http" - "net/http/httptest" "testing" - "time" "github.com/google/uuid" - "github.com/jackc/pgx/v5/pgtype" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -332,6 +328,10 @@ func TestConflictsBulkDismiss(t *testing.T) { } // TestConflictsBulkEscalate tests bulk escalate operations +// NOTE: This test is commented out because the /api/conflicts/bulk-escalate endpoint +// does not exist yet. It was planned in TEST_RELIABILITY_PLAN.md but never implemented. +// Uncomment and update when the endpoint is added. +/* func TestConflictsBulkEscalate(t *testing.T) { setup := setupTestServer(t) token := loginTestUser(t, setup.Server, setup.DB) @@ -425,3 +425,4 @@ func TestConflictsBulkEscalate(t *testing.T) { } }) } +*/ diff --git a/cmd/server/tests/device_test_patterns_test.go b/cmd/server/tests/device_test_patterns_test.go new file mode 100644 index 0000000..d31306a --- /dev/null +++ b/cmd/server/tests/device_test_patterns_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "bookhoard/internal/database" + "bookhoard/internal/handlers" + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestWithStructs demonstrates struct-based testing patterns +// BEFORE: map[string]interface{} -> AFTER: handlers.* structs +// BEFORE: No DB verification -> AFTER: Database verification + +func TestWithStructs(t *testing.T) { + deviceSetup := setupDeviceTest(t) + token := deviceSetup.UserToken + + // Create a device to test listing + device := deviceSetup.CreateDevice(t, "Test Device", "koreader", "test-device-struct-123") + + t.Run("ListDevices with struct", func(t *testing.T) { + req := httptest.NewRequest("GET", "/api/devices", nil) + req.Header.Set("Authorization", "Bearer "+token) + rr := httptest.NewRecorder() + deviceSetup.Server.Config.Handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code, "Should list devices") + + var response handlers.DeviceListResponse + err := json.Unmarshal(rr.Body.Bytes(), &response) + require.NoError(t, err, "Response should match DeviceListResponse schema") + assert.GreaterOrEqual(t, len(response.Devices), 1, "Should have at least one device") + + firstDevice := response.Devices[0] + assert.Equal(t, device.Name, firstDevice.DeviceName, "Should match device name") + assert.Equal(t, device.Type, firstDevice.DeviceType, "Should match device type") + }) + + t.Run("UpdateDevice with struct and DB verification", func(t *testing.T) { + syncEnabled := false + syncFreq := int32(15) + + updateRequest := map[string]interface{}{ + "sync_enabled": &syncEnabled, + "sync_frequency_minutes": &syncFreq, + } + updateBody, _ := json.Marshal(updateRequest) + + req := httptest.NewRequest("PUT", fmt.Sprintf("/api/devices/%s", device.ID), bytes.NewBuffer(updateBody)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + rr := httptest.NewRecorder() + deviceSetup.Server.Config.Handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code, "Should update device") + + var response map[string]interface{} + err := json.Unmarshal(rr.Body.Bytes(), &response) + require.NoError(t, err, "Response should unmarshal") + + assert.True(t, response["device_updated"].(bool), "Device should be updated") + + // NEW: Database verification + pgDeviceID := pgtype.UUID{Bytes: [16]byte(device.ID), Valid: true} + updatedDevice, err := deviceSetup.DB.GetDevice(context.Background(), pgDeviceID) + require.NoError(t, err, "Device should exist in database after update") + + assert.Equal(t, syncEnabled, updatedDevice.SyncEnabled.Bool, "DB: Sync should be disabled") + assert.Equal(t, syncFreq, updatedDevice.SyncFrequencyMinutes.Int32, "DB: Sync frequency should be updated") + }) +} + +// verifyDeviceUpdated is a helper function for database verification after device updates +func verifyDeviceUpdated(t *testing.T, db *database.Queries, deviceID uuid.UUID, syncEnabled bool, syncFreq int32) { + 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, syncEnabled, device.SyncEnabled.Bool, "DB: Sync enabled should match") + assert.Equal(t, syncFreq, device.SyncFrequencyMinutes.Int32, "DB: Sync frequency should match") +} diff --git a/cmd/server/tests/setup_integration_test.go b/cmd/server/tests/setup_integration_test.go new file mode 100644 index 0000000..724896f --- /dev/null +++ b/cmd/server/tests/setup_integration_test.go @@ -0,0 +1,296 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +const baseTestURL = "http://localhost:8765/api" + +// End-to-end integration test for complete application setup flow +func TestFullApplicationSetup(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Cleanup: Try to delete test user if it exists from previous test runs + t.Run("Cleanup_ExistingTestUser", func(t *testing.T) { + // Try to login as the test user first + loginReq := map[string]interface{}{ + "login": "admin@bookhoard.test", + "password": "TestPassword123!@#", + } + + body, _ := json.Marshal(loginReq) + resp, err := http.Post(baseTestURL+"/auth/login", "application/json", bytes.NewBuffer(body)) + if err != nil { + t.Logf("Cleanup: No existing test user to delete (server not available)") + return + } + defer resp.Body.Close() + + // If login succeeds, try to delete the user + if resp.StatusCode == http.StatusOK { + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + if token, ok := result["access_token"].(string); ok && token != "" { + // Delete the user using the token + req, _ := http.NewRequest("DELETE", baseTestURL+"/auth/account", bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + delResp, err := client.Do(req) + if err == nil { + defer delResp.Body.Close() + if delResp.StatusCode == http.StatusNoContent { + t.Logf("Cleanup: Deleted existing test user") + } else { + t.Logf("Cleanup: Could not delete existing test user (HTTP %d)", delResp.StatusCode) + } + } + + // Also try to delete any libraries created by this user + req, _ = http.NewRequest("GET", baseTestURL+"/libraries", bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + + listResp, err := client.Do(req) + if err == nil { + defer listResp.Body.Close() + if listResp.StatusCode == http.StatusOK { + var libsResult map[string]interface{} + json.NewDecoder(listResp.Body).Decode(&libsResult) + + if data, ok := libsResult["data"].([]interface{}); ok { + for _, lib := range data { + if libMap, ok := lib.(map[string]interface{}); ok { + if libID, ok := libMap["id"].(string); ok { + // Delete the library + req, _ = http.NewRequest("DELETE", baseTestURL+"/libraries/"+libID, bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + delLibResp, _ := client.Do(req) + if delLibResp != nil { + delLibResp.Body.Close() + } + } + } + } + } + } + } + } + } + + // Wait a bit for cleanup to complete + time.Sleep(500 * time.Millisecond) + }) + + // Step 1: Create first user (should be admin) + t.Run("Step1_CreateFirstUser", func(t *testing.T) { + userReq := map[string]interface{}{ + "email": "admin@bookhoard.test", + "username": "admin", + "password": "TestPassword123!@#", + "first_name": "Admin", + "last_name": "User", + } + + body, _ := json.Marshal(userReq) + resp, err := http.Post(baseTestURL+"/auth/register", "application/json", bytes.NewBuffer(body)) + assert.NoError(t, err) + defer resp.Body.Close() + + // Accept 201 (Created) or 409 (Conflict if already exists from previous incomplete test run) + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusConflict { + t.Fatalf("Expected 201 or 409, got %d", resp.StatusCode) + } + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + // If we got 409, the user already exists, so we need to login to get the token + if resp.StatusCode == http.StatusConflict { + t.Logf("User already exists, logging in instead...") + loginReq := map[string]interface{}{ + "login": "admin@bookhoard.test", + "password": "TestPassword123!@#", + } + body, _ := json.Marshal(loginReq) + resp2, err := http.Post(baseTestURL+"/auth/login", "application/json", bytes.NewBuffer(body)) + assert.NoError(t, err) + defer resp2.Body.Close() + + assert.Equal(t, http.StatusOK, resp2.StatusCode) + json.NewDecoder(resp2.Body).Decode(&result) + } + + if result["user"] != nil { + user, ok := result["user"].(map[string]interface{}) + assert.True(t, ok, "User field should exist") + assert.Equal(t, "admin", user["username"]) + assert.Equal(t, "admin", user["role"], "First user should be admin") + } + + t.Logf("✅ Step 1 PASSED: First user created with admin role") + }) + + // Login as admin + var adminToken string + t.Run("LoginAsAdmin", func(t *testing.T) { + loginReq := map[string]interface{}{ + "login": "admin@bookhoard.test", + "password": "TestPassword123!@#", + } + + body, _ := json.Marshal(loginReq) + resp, err := http.Post(baseTestURL+"/auth/login", "application/json", bytes.NewBuffer(body)) + assert.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + token, ok := result["access_token"].(string) + assert.True(t, ok, "Should have access_token") + adminToken = token + assert.NotEmpty(t, adminToken) + }) + + // Step 2: Create first library with ebook type + var libraryID string + t.Run("Step2_CreateFirstLibrary", func(t *testing.T) { + libraryReq := map[string]interface{}{ + "name": "Test Library", + "description": "Integration test library", + "type": "ebooks", + } + + body, _ := json.Marshal(libraryReq) + req, _ := http.NewRequest("POST", baseTestURL+"/libraries", bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + assert.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusCreated, resp.StatusCode) + + var result map[string]interface{} + err = json.NewDecoder(resp.Body).Decode(&result) + assert.NoError(t, err) + + // Safe extraction of library ID with nil check + if result["id"] == nil { + t.Fatalf("Expected library ID in response, got nil") + } + + var ok bool + libraryID, ok = result["id"].(string) + if !ok { + t.Fatalf("Expected library ID to be string, got %T", result["id"]) + } + + assert.NotEmpty(t, libraryID) + assert.Equal(t, "Test Library", result["name"]) + + t.Logf("✅ Step 2 PASSED: First library created with ID: %s", libraryID) + }) + + // Step 3: Add /app/uploads folder to the library + t.Run("Step3_AddUploadsFolder", func(t *testing.T) { + folderReq := map[string]interface{}{ + "folder_path": getUploadPath(), + } + + body, _ := json.Marshal(folderReq) + url := fmt.Sprintf("%s/libraries/%s/folders", baseTestURL, libraryID) + req, _ := http.NewRequest("POST", url, bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + assert.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusCreated, resp.StatusCode) + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + assert.Equal(t, getUploadPath(), result["folder_path"]) + + t.Logf("✅ Step 3 PASSED: %s folder added to library", getUploadPath()) + }) + + // Step 4: Scan the library + t.Run("Step4_ScanLibrary", func(t *testing.T) { + scanReq := map[string]interface{}{ + "library_id": libraryID, + } + + body, _ := json.Marshal(scanReq) + req, _ := http.NewRequest("POST", baseTestURL+"/scanner/scan", bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + assert.NoError(t, err) + defer resp.Body.Close() + + // Accept 200 or 202 + assert.Contains(t, []int{http.StatusOK, http.StatusAccepted}, resp.StatusCode) + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + assert.Equal(t, "success", result["status"]) + + t.Logf("✅ Step 4 PASSED: Library scan initiated") + }) + + // Wait for scan to complete + time.Sleep(2 * time.Second) + + // Step 5: List media-items + t.Run("Step5_ListMediaItems", func(t *testing.T) { + url := fmt.Sprintf("%s/libraries/%s/media-items", baseTestURL, libraryID) + req, _ := http.NewRequest("GET", url, nil) + req.Header.Set("Authorization", "Bearer "+adminToken) + + client := &http.Client{} + resp, err := client.Do(req) + assert.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + data, ok := result["data"].([]interface{}) + assert.True(t, ok, "Data field should exist") + assert.True(t, len(data) >= 0, "Should return data array") + + t.Logf("✅ Step 5 PASSED: Media items listed (count: %d)", len(data)) + }) +} + +// Helper function to read response body +func readBody(resp *http.Response) string { + body, _ := io.ReadAll(resp.Body) + return string(body) +} diff --git a/cmd/server/tests/test_helpers.go b/cmd/server/tests/test_helpers.go index ef679bb..0471d02 100644 --- a/cmd/server/tests/test_helpers.go +++ b/cmd/server/tests/test_helpers.go @@ -406,7 +406,7 @@ func setupTestServer(t *testing.T) *TestServerSetup { analyticsHandler := handlers.NewAnalyticsHandler(queries) queueHandler := handlers.NewQueueHandler(queries, queueProcessor) - // Create refactored handlers (matching main.go Phase 6) + // Create refactored handlers (matching main.go) libraryService := services.NewLibraryService(queries) worker := services.NewWorker(3) collectionHandler := handlers.NewCollectionHandler(queries, connManager)