Skip to content

Commit e0a4d99

Browse files
committed
fix: Address PR review feedback for mock user initialization
Addresses Priority 1 and Priority 2 issues from code review: ## Priority 1 - Documentation Mismatch (FIXED) - Removed database-management-scripts.md from this PR - Documentation referenced wipe_database.py and restore_database.py which are in PR #481, not this PR - Eliminates confusion and keeps PRs focused ## Priority 2 - Startup Logging (FIXED) - Updated main.py to check return value of initialize_default_users() - Uses raise_on_error=True for consistency with provider initialization - Logs accurate success/failure status instead of unconditional message - Follows same pattern as provider initialization (backend/main.py:153-157) ## Priority 2 - Unit Test Coverage (FIXED) - Added 4 comprehensive unit tests for initialize_default_users(): * test_initialize_default_users_skip_auth_true - Creates user when SKIP_AUTH=true * test_initialize_default_users_skip_auth_false - Skips when SKIP_AUTH=false * test_initialize_default_users_error_no_raise - Graceful error handling * test_initialize_default_users_error_with_raise - Raises when requested - All tests pass locally (4/4) - Patches core.mock_auth.ensure_mock_user_exists correctly ## Test Results All new tests passing: - test_initialize_default_users_skip_auth_true PASSED - test_initialize_default_users_skip_auth_false PASSED - test_initialize_default_users_error_no_raise PASSED - test_initialize_default_users_error_with_raise PASSED Addresses review comment: #480 (comment)
1 parent fef3fd5 commit e0a4d99

File tree

3 files changed

+52
-383
lines changed

3 files changed

+52
-383
lines changed

backend/main.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,11 @@ async def lifespan(_app: FastAPI) -> AsyncGenerator[None, None]:
150150
logger.info("Initialized providers: %s", ", ".join(p.name for p in providers))
151151

152152
# Initialize default users (mock user in development mode)
153-
system_init_service.initialize_default_users()
154-
logger.info("Default users initialized")
153+
success = system_init_service.initialize_default_users(raise_on_error=True)
154+
if success:
155+
logger.info("Default users initialized successfully")
156+
else:
157+
logger.warning("Default users initialization skipped or failed")
155158
except StopIteration:
156159
logger.error("Failed to get database session")
157160
return

backend/tests/unit/test_system_initialization_service_unit.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,3 +453,50 @@ def test_setup_watsonx_models_error_with_raise(self, service):
453453
service._setup_watsonx_models(provider_id, True)
454454

455455
assert "Model creation failed" in str(exc_info.value)
456+
457+
def test_initialize_default_users_skip_auth_true(self, service, mock_settings):
458+
"""Test initialize_default_users creates mock user when SKIP_AUTH=true."""
459+
mock_settings.skip_auth = True
460+
mock_user_id = uuid4()
461+
462+
with patch("core.mock_auth.ensure_mock_user_exists") as mock_ensure_user:
463+
mock_ensure_user.return_value = mock_user_id
464+
465+
result = service.initialize_default_users(raise_on_error=False)
466+
467+
assert result is True
468+
mock_ensure_user.assert_called_once_with(service.db, mock_settings)
469+
470+
def test_initialize_default_users_skip_auth_false(self, service, mock_settings):
471+
"""Test initialize_default_users skips creation when SKIP_AUTH=false."""
472+
mock_settings.skip_auth = False
473+
474+
result = service.initialize_default_users(raise_on_error=False)
475+
476+
assert result is True
477+
# Should not attempt to import or call ensure_mock_user_exists
478+
479+
def test_initialize_default_users_error_no_raise(self, service, mock_settings):
480+
"""Test initialize_default_users handles errors gracefully with raise_on_error=False."""
481+
mock_settings.skip_auth = True
482+
483+
with patch("core.mock_auth.ensure_mock_user_exists") as mock_ensure_user:
484+
mock_ensure_user.side_effect = Exception("User creation failed")
485+
486+
result = service.initialize_default_users(raise_on_error=False)
487+
488+
assert result is False
489+
mock_ensure_user.assert_called_once()
490+
491+
def test_initialize_default_users_error_with_raise(self, service, mock_settings):
492+
"""Test initialize_default_users raises exception when raise_on_error=True."""
493+
mock_settings.skip_auth = True
494+
495+
with patch("core.mock_auth.ensure_mock_user_exists") as mock_ensure_user:
496+
mock_ensure_user.side_effect = Exception("User creation failed")
497+
498+
with pytest.raises(Exception) as exc_info:
499+
service.initialize_default_users(raise_on_error=True)
500+
501+
assert "User creation failed" in str(exc_info.value)
502+
mock_ensure_user.assert_called_once()

0 commit comments

Comments
 (0)