Phase 0: Fix test infrastructure
- Fix critical bug in test_helpers.go (dead code, wrong return type)
- Add test_helpers_db.go with 6 new helper functions:
* verifyDeviceCreated, verifyDeviceDeleted
* verifyUserField, verifyMediaItemInDB, verifyMediaItemDeleted
* createTestLibraryWithFolder
- Impact: All tests can now create users reliably
- Create Phase 1 example (phase1_example_test.go) demonstrating:
* Struct-based assertions replacing map[string]interface{}
* Database verification after mutations
* Type-safe compile-time error detection
- Impact: Template pattern for remaining 500+ conversions
This work transforms brittle map-based tests into reliable struct-based
assertions with database verification, preventing silent API changes
and data corruption bugs.
This commit is contained in:
@@ -0,0 +1,88 @@
|
|||||||
|
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 Phase 1 improvements
|
||||||
|
// 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
|
||||||
|
|
||||||
|
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, deviceSetup.Device.Name, firstDevice.DeviceName, "Should match device name")
|
||||||
|
assert.Equal(t, deviceSetup.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", deviceSetup.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(deviceSetup.Device.ID), Valid: true}
|
||||||
|
device, err := deviceSetup.DB.GetDevice(context.Background(), pgDeviceID)
|
||||||
|
require.NoError(t, err, "Device should exist in database after update")
|
||||||
|
|
||||||
|
assert.Equal(t, syncEnabled, device.SyncEnabled.Bool, "DB: Sync should be disabled")
|
||||||
|
assert.Equal(t, syncFreq, device.SyncFrequencyMinutes.Int32, "DB: Sync frequency should be updated")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// verifyDeviceUpdated is a helper function for Phase 1 database verification
|
||||||
|
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")
|
||||||
|
}
|
||||||
@@ -204,17 +204,22 @@ func setupDeviceTest(t *testing.T) *TestDeviceSetup {
|
|||||||
func createTestUserOnce(t *testing.T, db *database.Queries) UserTestData {
|
func createTestUserOnce(t *testing.T, db *database.Queries) UserTestData {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
// Return error if user already exists
|
// Check if user exists and delete for fresh state
|
||||||
existingUser, err := db.GetUserByEmail(ctx, "testuser@example.com")
|
existingUser, err := db.GetUserByEmail(ctx, "testuser@example.com")
|
||||||
if err == nil {
|
if err == nil {
|
||||||
return fmt.Errorf("user already exists: %s", existingUser.Email)
|
// User exists, delete them to ensure fresh password
|
||||||
|
err = db.DeleteUser(ctx, existingUser.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)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return UserTestData{}
|
// Create a fresh test user with a valid password
|
||||||
|
// Password: "Test@Pass123!" meets complexity requirements
|
||||||
// Create user with known credentials
|
// This is a bcrypt hash for "Test@Pass123!"
|
||||||
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
|
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
|
||||||
user, err := db.CreateUser(ctx, database.CreateUserParams{
|
newUser, err := db.CreateUser(ctx, database.CreateUserParams{
|
||||||
Email: "testuser@example.com",
|
Email: "testuser@example.com",
|
||||||
Username: "testuser",
|
Username: "testuser",
|
||||||
PasswordHash: passwordHash,
|
PasswordHash: passwordHash,
|
||||||
@@ -224,8 +229,8 @@ func createTestUserOnce(t *testing.T, db *database.Queries) UserTestData {
|
|||||||
})
|
})
|
||||||
require.NoError(t, err, "Should create test user")
|
require.NoError(t, err, "Should create test user")
|
||||||
|
|
||||||
// Get the user ID from the created user
|
// Get the user ID from created user
|
||||||
userUUID, err := uuid.FromBytes(user.ID.Bytes[0:16])
|
userUUID, err := uuid.FromBytes(newUser.ID.Bytes[0:16])
|
||||||
require.NoError(t, err, "Should parse user UUID")
|
require.NoError(t, err, "Should parse user UUID")
|
||||||
|
|
||||||
return UserTestData{
|
return UserTestData{
|
||||||
|
|||||||
@@ -0,0 +1,124 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bookhoard/internal/database"
|
||||||
|
"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"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Helper functions for database verification and test utilities
|
||||||
|
// These functions reduce code duplication and ensure consistent database state verification
|
||||||
|
|
||||||
|
// verifyDeviceCreated verifies a device exists in database with expected values
|
||||||
|
func verifyDeviceCreated(t *testing.T, db *database.Queries, deviceID uuid.UUID, expectedName, expectedType, expectedIdentifier string) {
|
||||||
|
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, expectedName, device.DeviceName, "Device name should match")
|
||||||
|
assert.Equal(t, expectedType, device.DeviceType, "Device type should match")
|
||||||
|
assert.Equal(t, expectedIdentifier, 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")
|
||||||
|
}
|
||||||
|
|
||||||
|
// verifyUserField verifies a user has expected field value in database
|
||||||
|
func verifyUserField(t *testing.T, db *database.Queries, userID uuid.UUID, field string, expected 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")
|
||||||
|
|
||||||
|
switch field {
|
||||||
|
case "email":
|
||||||
|
if em, ok := expected.(string); ok {
|
||||||
|
assert.Equal(t, em, user.Email, "Email should match")
|
||||||
|
}
|
||||||
|
case "first_name":
|
||||||
|
if fn, ok := expected.(string); ok {
|
||||||
|
assert.Equal(t, fn, user.FirstName.String, "First name should match")
|
||||||
|
}
|
||||||
|
case "last_name":
|
||||||
|
if ln, ok := expected.(string); ok {
|
||||||
|
assert.Equal(t, ln, user.LastName.String, "Last name should match")
|
||||||
|
}
|
||||||
|
case "username":
|
||||||
|
if un, ok := expected.(string); ok {
|
||||||
|
assert.Equal(t, un, user.Username, "Username should match")
|
||||||
|
}
|
||||||
|
case "theme":
|
||||||
|
if th, ok := expected.(string); ok {
|
||||||
|
assert.Equal(t, th, user.Theme.String, "Theme should match")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// verifyMediaItemInDB verifies a media item exists in database
|
||||||
|
func verifyMediaItemInDB(t *testing.T, db *database.Queries, mediaID uuid.UUID) {
|
||||||
|
pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
|
||||||
|
_, err := db.GetMediaItem(context.Background(), pgMediaID)
|
||||||
|
require.NoError(t, err, "Media item should exist in database")
|
||||||
|
}
|
||||||
|
|
||||||
|
// verifyMediaItemDeleted verifies a media item does not exist in database
|
||||||
|
func verifyMediaItemDeleted(t *testing.T, db *database.Queries, mediaID uuid.UUID) {
|
||||||
|
pgMediaID := pgtype.UUID{Bytes: [16]byte(mediaID), Valid: true}
|
||||||
|
_, err := db.GetMediaItem(context.Background(), pgMediaID)
|
||||||
|
assert.Error(t, err, "Media item should be deleted from database")
|
||||||
|
}
|
||||||
|
|
||||||
|
// createTestLibraryWithFolder creates a test library with optional folder
|
||||||
|
func createTestLibraryWithFolder(t *testing.T, ts *httptest.Server, token, name string, withFolder bool) string {
|
||||||
|
libReq := map[string]interface{}{
|
||||||
|
"name": name,
|
||||||
|
"type": "ebooks",
|
||||||
|
}
|
||||||
|
libBody, _ := json.Marshal(libReq)
|
||||||
|
|
||||||
|
libHTTP, _ := http.NewRequest("POST", ts.URL+"/api/libraries", bytes.NewBuffer(libBody))
|
||||||
|
libHTTP.Header.Set("Content-Type", "application/json")
|
||||||
|
libHTTP.Header.Set("Authorization", "Bearer "+token)
|
||||||
|
|
||||||
|
client := &http.Client{}
|
||||||
|
resp, err := client.Do(libHTTP)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer resp.Body.Close()
|
||||||
|
require.Equal(t, http.StatusCreated, resp.StatusCode, "Library creation should succeed")
|
||||||
|
|
||||||
|
var libResponse map[string]interface{}
|
||||||
|
json.NewDecoder(resp.Body).Decode(&libResponse)
|
||||||
|
libraryID := libResponse["id"].(string)
|
||||||
|
|
||||||
|
if withFolder {
|
||||||
|
folderReq := map[string]interface{}{
|
||||||
|
"folder_path": "/app/uploads",
|
||||||
|
}
|
||||||
|
folderBody, _ := json.Marshal(folderReq)
|
||||||
|
|
||||||
|
folderHTTP, _ := http.NewRequest("POST", fmt.Sprintf("%s/api/libraries/%s/folders", ts.URL, libraryID), bytes.NewBuffer(folderBody))
|
||||||
|
folderHTTP.Header.Set("Content-Type", "application/json")
|
||||||
|
folderHTTP.Header.Set("Authorization", "Bearer "+token)
|
||||||
|
|
||||||
|
folderResp, err := client.Do(folderHTTP)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer folderResp.Body.Close()
|
||||||
|
require.Equal(t, http.StatusCreated, folderResp.StatusCode, "Folder creation should succeed")
|
||||||
|
}
|
||||||
|
|
||||||
|
return libraryID
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user