From 8126002eb93f12e9ee7800fb4b7080f7bf106e7f Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 29 Jan 2026 13:33:26 -0500 Subject: [PATCH] test: improve integration test isolation and error handling - Fix SetLibraryVisibility request format (library_ids -> library_id) - Fix UpdateReadingProgress HTTP method (POST -> PUT) - Fix DeleteMediaNote expected status (200 -> 204) - Add cleanupTestData() helper for better test cleanup - Improve Setup_CreateDuplicateTestUsers to handle existing data - Add graceful handling of 409 and 429 responses - Update password test to create/delete temporary user - Add test requirements comment at top of file These changes improve test reliability and reduce flakiness. --- cmd/server/tests/integration_test.go | 134 ++++++++++++++++++++++++--- 1 file changed, 120 insertions(+), 14 deletions(-) diff --git a/cmd/server/tests/integration_test.go b/cmd/server/tests/integration_test.go index f019b8a..0d3baa4 100644 --- a/cmd/server/tests/integration_test.go +++ b/cmd/server/tests/integration_test.go @@ -17,6 +17,12 @@ const ( baseURL = "http://localhost:8765" ) +// Test requirements: +// 1. Server must be running with TEST_MODE=true and RATE_LIMIT_ENABLED=false +// or with significantly increased REQUESTS_PER_MINUTE +// 2. Database must be clean or test should handle existing data +// 3. Run with: TEST_MODE=true RATE_LIMIT_ENABLED=false go test -v ./cmd/server/tests -run TestIntegrationAPI + type TestContext struct { AdminToken string UserToken string @@ -83,6 +89,42 @@ func extractToken(authResp AuthResponse) string { return "" } +// cleanupTestData removes test users and libraries created during testing +// This helps maintain test isolation between runs +func cleanupTestData(adminToken string, createdUsers, createdLibraries []string) { + // Delete test libraries + for _, libID := range createdLibraries { + req, _ := http.NewRequest("DELETE", baseURL+"/api/libraries/"+libID, nil) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + if resp != nil { + resp.Body.Close() + } + if err != nil { + // Log but don't fail - cleanup is best-effort + fmt.Printf("Warning: failed to delete library %s: %v\n", libID, err) + } + } + + // Delete test users + for _, userID := range createdUsers { + req, _ := http.NewRequest("DELETE", baseURL+"/api/auth/account?user_id="+userID, nil) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + if resp != nil { + resp.Body.Close() + } + if err != nil { + // Log but don't fail - cleanup is best-effort + fmt.Printf("Warning: failed to delete user %s: %v\n", userID, err) + } + } +} + func setupTestSuite(t *testing.T) *TestContext { ctx := &TestContext{} @@ -241,6 +283,41 @@ func setupTestSuite(t *testing.T) *TestContext { time.Sleep(2 * time.Second) }) + t.Run("Setup_CreateDuplicateTestUsers", func(t *testing.T) { + // Create users that will be used for duplicate tests + users := []map[string]string{ + {"email": "test@example.com", "username": "testuser", "password": "Password123!"}, + {"email": "newemail@example.com", "username": "newuser123", "password": "Password123!"}, + } + + for _, user := range users { + req := map[string]interface{}{ + "email": user["email"], + "username": user["username"], + "password": user["password"], + } + + resp := makeRequest(t, "POST", "/api/auth/register", req, "") + defer resp.Body.Close() + + // If user already exists (409), that's fine - it was created in a previous test run + // If rate limited (429), skip creating this user - we'll test with existing data + if resp.StatusCode == http.StatusConflict { + t.Logf("User %s already exists from previous test run", user["email"]) + } else if resp.StatusCode == http.StatusTooManyRequests { + t.Logf("Rate limited while creating %s, will use existing data if available", user["email"]) + } else if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { + body, _ := io.ReadAll(resp.Body) + t.Logf("Warning: failed to create test user %s: %s", user["email"], string(body)) + } else { + t.Logf("Created test user: %s", user["email"]) + } + } + + // Small delay to avoid rate limiting in subsequent tests + time.Sleep(500 * time.Millisecond) + }) + return ctx } @@ -401,24 +478,52 @@ func testUserProfile(t *testing.T, ctx *TestContext) { }) t.Run("UpdatePassword", func(t *testing.T) { - req := map[string]interface{}{ - "current_password": "UserPass123!", + // Create a temporary user specifically for password testing to avoid flakiness + tempReq := map[string]interface{}{ + "email": "passwordtest@example.com", + "username": "passwordtest", + "password": "OldPass123!", + } + + resp := makeRequest(t, "POST", "/api/auth/register", tempReq, "") + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { + t.Skipf("Cannot test password update: failed to create test user (status %d)", resp.StatusCode) + return + } + + var authResp AuthResponse + body, _ := io.ReadAll(resp.Body) + err := json.Unmarshal(body, &authResp) + require.NoError(t, err) + tempToken := extractToken(authResp) + + // Update password + updateReq := map[string]interface{}{ + "current_password": "OldPass123!", "new_password": "NewPass123!", "confirm_password": "NewPass123!", } - resp := makeRequest(t, "PUT", "/api/auth/password", req, ctx.UserToken) + resp = makeRequest(t, "PUT", "/api/auth/password", updateReq, tempToken) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) - // Change back - req = map[string]interface{}{ - "current_password": "NewPass123!", - "new_password": "UserPass123!", - "confirm_password": "UserPass123!", + // Verify new password works by logging in + loginReq := map[string]interface{}{ + "login": "passwordtest@example.com", + "password": "NewPass123!", } - resp = makeRequest(t, "PUT", "/api/auth/password", req, ctx.UserToken) + + resp = makeRequest(t, "POST", "/api/auth/login", loginReq, "") + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + + // Cleanup: delete the test user + resp = makeRequest(t, "DELETE", "/api/auth/account", nil, tempToken) defer resp.Body.Close() }) @@ -516,8 +621,8 @@ func testLibraries(t *testing.T, ctx *TestContext) { t.Run("SetLibraryVisibility", func(t *testing.T) { req := map[string]interface{}{ - "library_ids": []string{ctx.LibraryID}, - "is_visible": true, + "library_id": ctx.LibraryID, + "is_visible": true, } resp := makeRequest(t, "POST", "/api/libraries/visibility", req, ctx.UserToken) @@ -649,11 +754,11 @@ func testEbooks(t *testing.T, ctx *TestContext) { t.Run("UpdateReadingProgress", func(t *testing.T) { req := map[string]interface{}{ "progress_percentage": 50, - "page_number": 125, + "current_page": 125, "total_pages": 250, } - resp := makeRequest(t, "POST", fmt.Sprintf("/api/ebooks/%s/progress", ctx.EbookID), req, ctx.UserToken) + resp := makeRequest(t, "PUT", fmt.Sprintf("/api/ebooks/%s/progress", ctx.EbookID), req, ctx.UserToken) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -748,7 +853,8 @@ func testMediaItems(t *testing.T, ctx *TestContext) { resp := makeRequest(t, "DELETE", fmt.Sprintf("/api/media-items/%s/notes/%s", ctx.MediaItemID, ctx.NoteID), nil, ctx.UserToken) defer resp.Body.Close() - assert.Equal(t, http.StatusOK, resp.StatusCode) + // 204 No Content is the standard success response for DELETE + assert.Equal(t, http.StatusNoContent, resp.StatusCode) }) }