Test expectations in TestMediaItemISBNNormalization were incorrect:
- Tests were expecting 12-digit outputs for 13-digit inputs
- Updated to expect correct 13-digit normalized outputs
This fixes the failing normalization tests.
Enhance NormalizeISBN to validate and convert ISBNs:
- Validate length (10 or 13 digits), return error if invalid
- Convert ISBN-10 to ISBN-13 by prefixing '978' and recalculating checksum
- Add NormalizeISBNSafe for backward compatibility in scanners
This ensures all ISBNs stored in database are valid ISBN-13 format.
Update test assertions in TestCollectionsBulkOperations to expect
'added' instead of 'success' in the response, matching the handler
change made in the bulk operations rename.
Fixes:
- BulkAddBooks_InvalidCollectionID: assert 'added' field exists
- BulkAddBooks_SingleOperation: assert 'added' field exists
- Update all test URLs from /api/books/ to /api/media-items/
- Update request structures: book_ids → media_item_ids
- Update bulk-update request format to array of operations
- Update response assertions: success → deleted/updated
- Update result assertions: book_id → media_item_id
The test was sending a single object with a "meta" field, but the handler
expects an array of KoboAnalyticsTest objects (matching other Kobo endpoints).
Changed:
- Removed unsupported "meta" field
- Converted single object to array format
- Now matches Kobo protocol pattern used by /markup and /bookmark endpoints
All Kobo tests now pass:
- TestKoboInitialization ✅
- TestKoboLibrarySync ✅
- TestKoboMarkupSync ✅
- TestKoboBookmarkSync ✅
- TestKoboAnalyticsGettests ✅
- TestKoboDeviceHeaderParsing ✅
The TestKoboInitialization test was calling getTestUserID() explicitly
on line 25, but loginTestUser() already calls this internally. This caused
the test user to be deleted and recreated after login, leading to
inconsistent state and HTTP 500 errors when creating libraries.
After removing the redundant call:
- TestKoboInitialization now passes
- All Kobo sync tests pass successfully
- Add schema initialization call after database connection
- Initialize schema before handler creation
- Fatal on failure (schema is critical for app to function)
- Clear log messages show initialization progress
Server startup flow:
1. Load config
2. Connect to database
3. Initialize schema (NEW - ensures all tables/functions exist)
4. Create handlers and services
5. Start server
The first GetReadingStats_WithAuth_DefaultDates test was expecting
'total_books' and 'total_reading_time' fields that don't exist in the
API response. The second duplicate test correctly expects
'total_books_read' and 'total_reading_time_minutes'.
This resolves the TestAnalyticsReadingStats failure.
Finish migrating all test files to the new TestServerSetup pattern
introduced by the goroutine cleanup refactoring. This resolves all
remaining compilation errors in the test suite.
Changes:
- device_cap_test.go: Fix undefined ts references (7 instances)
* Replace ts.URL with setup.Server.URL in all test functions
* Fix URL references in t.Run subtest closures
- queue_test.go: Fix undefined db and helper function issues (5 instances)
* Replace db.CreateDevice with setup.DB.CreateDevice
* Fix loginAdminUser() to use ts/db parameters instead of setup
* Fix loginUserWithID() to use ts parameter instead of setup
- websocket_test.go: Convert 5 tests to new TestServerSetup pattern
* Replace old pattern (ts, queries, _) with new pattern (setup)
* Update all resource references to use setup.Server and setup.DB
* Fix getTestUserID calls to include t parameter
Build Impact:
- All compilation errors resolved
- Integration tests now compile successfully
- No functional changes to test logic
Related: TestServerSetup cleanup pattern (TEST_CLEANUP_PATTERN.md)
BREAKING CHANGE: setupTestServer() now returns *TestServerSetup instead of (*httptest.Server, *database.Queries, *config.Config)
This fixes the database connection and goroutine leak issues where:
- Each test created a new pgxpool (default max_conns = 4)
- connManager.StartCleanupTask() goroutine was never stopped
- queueProcessor.Start() goroutine was never stopped
- ~160 tests = potential 640+ leaked connections
New TestServerSetup struct provides:
- Automatic cleanup via t.Cleanup()
- Proper goroutine cancellation
- Database pool closing
- Thread-safe close() method with mutex
Phase 1 of test cleanup refactor.
- Fix TestKoboInitialization: use setupDeviceTest() for device creation
- Fix TestKoboLibrarySync: remove /test-token/ route path, add device auth
- Fix TestKoboMarkupSync: add device auth and last-read-place test case
- Fix TestKoboBookmarkSync: add device auth and last-read-place test case
- Fix TestKoboAnalyticsGettests: add device authentication
- Add debug logging to all test functions
- Remove unused imports (config, database, middleware, router, services, sync)
Phase 3 of KOBO_IMPLEMENTATION_PLAN.md completed (Steps 7-12).
All tests now use proper device authentication (Bearer tokens + x-kobo-device headers)
and include test cases for the new last-read-place bookmark feature.
- Fix TestUpdateDevice: Use correct JSON field name and handle float64 type
- Fix TestRejectDeviceRegistration: Expect message response instead of boolean
- Update approve device docs: Add missing response fields
- Update reject device docs: Correct message text and format
- Update Bruno API: Fix example response for reject endpoint
Both integration tests now pass while maintaining API consistency.
System Settings Tests (new file):
- Create system_settings_test.go with comprehensive test coverage
- Test admin-only access control
- Test validation (15-1440 minute range)
- Test error handling scenarios
- Test integration with scheduler
User Tests Cleanup:
- Remove old TestScanSettings from user_test.go
- Scan settings moved to system-wide (no longer per-user)
Device Cap Tests Enhancement:
- Update TestListUsersIncludesMaxDevices
- Add assertion for device_count field
- Verify both max_devices and device_count in response
All tests verify the migration from per-user to system-wide scan settings.
Add SystemSettingsHandler initialization in main.go:
- Create systemSettingsHandler instance with queries
- Add to router.Config for route registration
- Properly wired with existing dependencies
This enables the system settings endpoints to be registered and functional.
- API validates folder exists before adding to library
- /app/uploads is mounted volume in container
- Avoids need to create subdirectories
- Simplifies test setup
- Update createTestMediaItemID to add folder after creating library
- Ensures proper test data setup
- Tests now reflect real-world usage pattern
- Also fixed existing syntax error in mime_type line
Fixes: TestCollectionsBulkOperations and related test failures
Related: Handler validation commit
- Check library has folders before creating media items
- Return HTTP 400 with clear error message if no folders
- Proper error code (400) instead of generic 500
- Improved user feedback for invalid operations
- Inject LibraryService into MediaHandler
Fixes: TestCollectionsBulkOperations HTTP 500 errors
Related: Service layer validation commit
Critical fixes to prevent goroutine leaks during application shutdown:
1. Sync Queue Processor:
- Changed StartCleanupTask() to return context.CancelFunc
- Modified to accept and watch cancellable context
- Added queue context/cancel to Handler struct
- Created StartBackgroundTasks() method for main handler instance
- Cancel queue processor during shutdown in StopScheduler()
2. Connection Manager:
- Modified StartCleanupTask() to use cancellable context
- Returns cancel function that can be called during shutdown
- Goroutine now properly exits when context is cancelled
3. Handler Lifecycle:
- Added StartBackgroundTasks() to Handler
- Only main handler instance starts background goroutines
- Temporary handler instances (library/sync routes) don't start tasks
- StopScheduler() now properly shuts down all background goroutines
4. Router Integration:
- Updated SetupRoutes to accept queueProcessor parameter
- Main scanner handler starts background tasks after creation
- Library and sync route handlers don't start duplicate tasks
Impact:
- Fixes 2 major goroutine leaks (queue processor + connection cleanup)
- Application now properly shuts down all goroutines on exit
- No more resource leaks from long-running goroutines
- Test added to detect future goroutine regressions
Test: TestGoroutineCleanup verifies background services can be stopped.
Move database-dependent sync tests from internal/sync/ to
cmd/server/tests/ where they belong:
- TestSyncIntegration_OfflineDetector_* tests
- TestSyncIntegration_QueueProcessor_EnqueueProgress test
- Helper functions: setupSyncTestDB, createSyncTestUser/Device
These tests require a running PostgreSQL database and are
properly categorized as integration tests now.
Unit tests that remain in internal/sync/:
- TestOfflineDetector_ConstantValues (no DB needed)
- TestCalculateNextRetry (logic only)
- TestSyncTypeConstants, TestSyncStatusConstants (constants)
- TestPriorityConstants (constants)
All unit tests now pass with `make test` (no DB required).
- Rename createTestEbookID to createTestMediaItemID in test_helpers.go
- Update test helper comments and variable names (ebookReq -> mediaItemReq, etc.)
- Update all test file references:
- analytics_test.go
- book_matching_test.go
- collections_bulk_test.go
- kobo_test.go
- media_bulk_test.go
- opds_test.go
- Rename ebookID variable to mediaItemID in kobo_test.go
- Update test data to use 'Test Media Item' instead of 'Test Ebook'
Remove the getJSONInt helper function and update all test assertions to
expect float64 instead of int for JSON numeric fields, as Go's JSON
decoder unmarshals all numbers to float64 by default.
This simplifies the codebase by removing an unnecessary conversion
helper and makes tests more accurate to the actual JSON format.
Changes:
- Remove getJSONInt function from book_matching_test.go
- Update 5 assertions in book_matching_test.go to use float64
- Update 2 assertions in collections_bulk_test.go to use float64
- Update 2 assertions in media_bulk_test.go to use float64
- Add nil checks for optional numeric fields to prevent panics
Affected tests:
- TestBookMatchingBulkLink
- TestBookMatchingAutoLink
- TestCollectionsBulkOperations
- TestMediaBulkOperations
Note: Some test failures remain (API returning 400 instead of 200) but
these are legitimate test issues unrelated to type assertions.
Fix nil pointer panics in integration tests by initializing the
four refactored handlers (MediaHandler, SearchHandler, MatchingHandler,
CollectionHandler) that were added during Phase 6 refactoring but
never added to the test setup.
These handlers were properly instantiated in cmd/server/main.go
(commit 9fd8a39) but were missing from cmd/server/tests/test_helpers.go,
causing panics when tests tried to use /api/collections and /api/media-items
endpoints.
Changes:
- Create worker with 3 concurrent workers
- Initialize CollectionHandler with queries and connManager
- Initialize MediaHandler with queries and worker
- Initialize SearchHandler with queries
- Initialize MatchingHandler with queries and connManager
- Add all four handlers to router.Config struct
Fixes panic errors:
- internal/handlers/collections.go:78 (CreateCollection nil pointer)
- internal/handlers/media.go:891 (CreateMediaItem nil pointer)
Tests now pass:
- TestCollectionsBulkOperations: PASS
- TestAnalytics*: PASS (all analytics tests)
Note: Bruno API tests and frontend were NOT affected as they use the
real running application (which has complete handler setup).
- Replace hardcoded /app/uploads with getUploadPath()
- Update assertions to use dynamic paths
- Improve test log message to show actual path used
- Ensures tests work in both container and host environments
- Add isRunningInContainer() to detect test runtime environment
- Add getUploadPath() to resolve upload paths (container vs host)
- Add getCachePath() to resolve cache paths appropriately
- Update setupTestServer() to use dynamic path helpers
- Support environment variable overrides for flexibility
- Add os import for file system checks
- Create worker for background tasks (3 concurrent workers)
- Create CollectionHandler for collection endpoints
- Create MediaHandler with worker for media CRUD operations
- Create SearchHandler for query operations
- Create MatchingHandler for book matching/linking operations
- Update routerConfig to include new handlers instead of EbookHandler
All handlers properly initialized and passed to router package.
System is fully operational with new handler architecture.
This is Phase 6 of the ebook.go refactoring plan.
Remove the CollectionHandler field from router.Config struct literal
in cmd/server/tests/test_helpers.go. This field was removed from
the Config struct in a previous commit.
The collection routes are registered directly in handlers.SetupRoutes()
and don't need to be passed through the router config.
Remove the CollectionHandler field from router.Config struct and its
initialization in main.go. This field was never used - collections are
registered directly in handlers.SetupRoutes() where a CollectionHandler
is created locally.
Changes:
- Remove CollectionHandler field from internal/router/router.go Config
- Remove CollectionHandler: nil line from cmd/server/main.go
This cleans up dead code from the router refactoring. Collections
continue to work correctly as they are registered in SetupRoutes().
Related: Router refactoring completion
Phase 5: Application Lifecycle Management
Creates internal/app package for proper lifecycle management, signal
handling, and graceful shutdown of all services.
Changes:
- Create internal/app/app.go with App lifecycle manager
- Handles SIGINT, SIGTERM, SIGQUIT signals
- Graceful shutdown with 30-second timeout
- Manages HTTP server shutdown
- Manages scheduler start/stop
- Update cmd/server/main.go to use app lifecycle manager
- Replace defer-based cleanup with proper signal handling
- Server starts in background goroutine
- Blocks on app.Start() until shutdown signal
- Clean shutdown of all services
Benefits:
- Proper signal handling (Ctrl+C, kill, docker stop)
- Graceful shutdown prevents data corruption
- No more os.Exit(1) bypassing defer cleanup
- All services stopped in correct order
- Server stops accepting new connections first
- Then scheduler and background services stopped
Technical details:
- Uses sync.Mutex for shutdown safety
- Context with timeout for shutdown operations
- Channel-based coordination for shutdown completion
- Logs all lifecycle events for debugging
Fixes issue where e.Logger.Fatal() would call os.Exit(1)
immediately, skipping defer cleanup and causing unclean shutdown.
Phase 1 of scanner restoration plan
Changes:
- cmd/server/main.go: Capture ebookHandler from router.RegisterRoutes
- cmd/server/main.go: Start scheduler in background goroutine
- cmd/server/main.go: Defer StopScheduler() for graceful shutdown
- cmd/server/main.go: Start watch mode for all libraries after 2-second delay
- internal/router/router.go: Return ebookHandler from RegisterRoutes
This restores critical functionality that was removed during router refactor:
- Auto-scanning now works again
- Watch mode starts automatically for all libraries
- Graceful shutdown properly stops scheduler
Fixes issue where scheduler and watch mode were not starting on server boot.
- Rename phase1_integration_test.go to universal_progress_integration_test.go
(tests universal reading progress feature)
- Rename ebook_scanner_phase2_test.go to ebook_scanner_hash_test.go
(tests hash calculation and file identification utilities)
These renames make the test suite more maintainable and self-documenting.
- Fix float64 type assertions for JSON numbers in conflicts bulk operations
- Create fresh HTTP request body for duplicate book tests
- Add nil checks for type assertions in device cap tests
- Properly extract user_id from JWT for existing users
- Trim trailing whitespace from response bodies
- All 3 previously failing tests now passing
Test results: 19/22 passing (86.4%)
Fixes: TestCollectionsBulkOperations, TestConflictsBulkDismiss, TestUpdateUserMaxDevices
- Remove handler parameter from test function calls
- Update test signatures to match new setupTestServer return values
- Fix compilation errors after test helper refactoring
- Maintain websocket test functionality
- Remove handler parameter from test function calls
- Update test signatures to match new setupTestServer return values
- Fix compilation errors after test helper refactoring
- Ensure test consistency for opds, queue, and auth endpoints
- Remove handler parameter from test function calls
- Update test signatures to match new setupTestServer return values
- Fix compilation errors after test helper refactoring
- Maintain test functionality for kobo and media endpoints
- Remove handler parameter from test function calls
- Update test signatures to match new setupTestServer return values
- Fix compilation errors after test helper refactoring
- Ensure test consistency across all test files
- Remove handler parameter from test function calls
- Update test signatures to use new return values from setupTestServer
- Fix compilation errors after test helper refactoring
- Maintain test functionality while simplifying setup
- Replace manual config construction with config.LoadConfig()
- Remove problematic password validation logic
- Apply test-specific overrides after loading config
- Clean up unused imports (os, strings)
- Tests now use same configuration method as main application
- Fixes database authentication issues in integration tests
Changed login test password from 'Test@Pass123!' to 'testpass123' and updated
bcrypt hash to use Go's golang.org/x/crypto/bcrypt library instead of Python's bcrypt.
john-okeefe