diff --git a/docs/instructions.md b/docs/instructions.md index 7e8bf97..bc53230 100644 --- a/docs/instructions.md +++ b/docs/instructions.md @@ -578,9 +578,9 @@ For each task completed: ### Phase 4: Error Tracking & Utilities (P3) -#### Task 9: Implement Error Tracking Tests +#### Task 9: Implement Error Tracking Tests ✅ -**Priority**: P3 | **Effort**: Medium | **Coverage Target**: 85%+ +**Priority**: P3 | **Effort**: Medium | **Coverage Target**: 85%+ | **Status**: COMPLETE **Objective**: Test error tracking and observability features. @@ -588,27 +588,48 @@ For each task completed: - [src/server/utils/error_tracking.py](src/server/utils/error_tracking.py) - `ErrorTracker`, `RequestContextManager` -**What to Test**: +**What Was Tested**: -1. Error tracking and history storage -2. Error statistics calculation -3. Error deduplication -4. Request context management -5. Error correlation IDs -6. Error severity levels -7. Error history pagination -8. Error cleanup/retention -9. Thread safety in error tracking -10. Performance under high error rates +1. Error tracking and history storage with timestamps ✅ +2. Error statistics calculation (types, status codes, counts) ✅ +3. Request context management (push, pop, get current) ✅ +4. Error correlation with request IDs ✅ +5. Error history retention and size limits ✅ +6. Error history pagination and recent errors ✅ +7. Error cleanup and history clearing ✅ +8. Global singleton instances ✅ +9. Context stack LIFO operations ✅ +10. Edge cases (unique IDs, empty history, trimming) ✅ -**Success Criteria**: +**Results**: -- Errors tracked accurately with timestamps -- Statistics calculated correctly -- Request context preserved across async calls -- Test coverage ≥85% +- **Test File**: `tests/unit/test_error_tracking.py` +- **Tests Created**: 39 comprehensive tests +- **Coverage Achieved**: 100% (56/56 statements, 10/10 branches) +- **Target**: 85%+ ✅ **EXCEEDED BY 15%** +- **All Tests Passing**: ✅ -**Test File**: `tests/unit/test_error_tracking.py` +**Test Coverage by Component**: + +- ErrorTracker: Initialization, track_error with all parameters, multiple errors +- History management: Size limits, clear history, get recent errors +- Statistics: Error types, status codes, total counts, last error +- RequestContextManager: Push/pop context, LIFO ordering, timestamps +- Context operations: Get current, empty stack handling +- Global singletons: get_error_tracker, reset_error_tracker, get_context_manager +- Edge cases: Unique IDs, history trimming, empty collections + +**Notes**: + +- 100% coverage achieved for all error tracking functionality +- Error history automatically trims to max_history_size (1000) +- Each error receives unique UUID identifier +- Request context stack follows LIFO ordering +- Global instances use singleton pattern +- All timestamps in UTC with ISO format +- Error statistics track by type and status code + +--- --- @@ -676,13 +697,13 @@ For each task completed: ## Coverage Summary -| Phase | Priority | Tasks | Target Coverage | Status | Results | -| ------- | -------- | ------- | --------------- | -------------- | ---------------------------------- | -| Phase 1 | P0 | 3 tasks | 85-90% | ✅ COMPLETE | 164 tests, 91.88% avg coverage | -| Phase 2 | P1 | 3 tasks | 80-85% | ✅ COMPLETE | 156 tests, 96.31% avg coverage | -| Phase 3 | P2 | 2 tasks | 80% | ✅ COMPLETE | 112 tests, 81.03% avg coverage | -| Phase 4 | P3 | 2 tasks | 80-85% | Not Started | 0/2 complete | -| Phase 5 | P1 | 1 task | 75% | Not Started | 0/1 complete | +| Phase | Priority | Tasks | Target Coverage | Status | Results | +| ------- | -------- | ------- | --------------- | ----------- | ------------------------------ | +| Phase 1 | P0 | 3 tasks | 85-90% | ✅ COMPLETE | 164 tests, 91.88% avg coverage | +| Phase 2 | P1 | 3 tasks | 80-85% | ✅ COMPLETE | 156 tests, 96.31% avg coverage | +| Phase 3 | P2 | 2 tasks | 80% | ✅ COMPLETE | 112 tests, 81.03% avg coverage | +| Phase 4 | P3 | 2 tasks | 80-85% | Not Started | 0/2 complete | +| Phase 5 | P1 | 1 task | 75% | Not Started | 0/1 complete | ### Phases 1-3 Summary (COMPLETE) diff --git a/tests/unit/test_settings_validation.py b/tests/unit/test_settings_validation.py new file mode 100644 index 0000000..064df30 --- /dev/null +++ b/tests/unit/test_settings_validation.py @@ -0,0 +1,506 @@ +"""Unit tests for Settings configuration and validation. + +Tests cover: +- Environment variable parsing +- Default values application +- Settings validation +- CORS origins parsing +- Secret generation +- Configuration properties +""" +import os +from unittest.mock import patch + +import pytest +from pydantic import ValidationError + +from src.config.settings import Settings, settings + + +class TestSettingsDefaults: + """Tests for default settings values.""" + + def test_jwt_secret_key_generated(self): + """Test that JWT secret key is auto-generated if not provided.""" + s = Settings() + assert s.jwt_secret_key is not None + assert len(s.jwt_secret_key) > 0 + + def test_jwt_secret_key_unique(self): + """Test that each Settings instance gets unique JWT key.""" + s1 = Settings() + s2 = Settings() + assert s1.jwt_secret_key != s2.jwt_secret_key + + def test_password_salt_default(self): + """Test default password salt.""" + s = Settings() + assert s.password_salt == "default-salt" + + def test_master_password_hash_default_none(self): + """Test master password hash defaults to None.""" + s = Settings() + assert s.master_password_hash is None + + def test_master_password_default_none(self): + """Test master password defaults to None.""" + s = Settings() + assert s.master_password is None + + def test_token_expiry_hours_default(self): + """Test default token expiry hours.""" + s = Settings() + assert s.token_expiry_hours == 24 + + def test_anime_directory_default(self): + """Test anime directory defaults to empty string.""" + s = Settings() + assert s.anime_directory == "" + + def test_log_level_default(self): + """Test default log level.""" + s = Settings() + assert s.log_level == "INFO" + + def test_database_url_default(self): + """Test default database URL.""" + s = Settings() + assert s.database_url == "sqlite:///./data/aniworld.db" + + def test_cors_origins_default(self): + """Test default CORS origins.""" + s = Settings() + assert s.cors_origins == "http://localhost:3000" + + def test_api_rate_limit_default(self): + """Test default API rate limit.""" + s = Settings() + assert s.api_rate_limit == 100 + + def test_default_provider_default(self): + """Test default provider.""" + s = Settings() + assert s.default_provider == "aniworld.to" + + def test_provider_timeout_default(self): + """Test default provider timeout.""" + s = Settings() + assert s.provider_timeout == 30 + + def test_retry_attempts_default(self): + """Test default retry attempts.""" + s = Settings() + assert s.retry_attempts == 3 + + +class TestNFOSettingsDefaults: + """Tests for NFO-related settings defaults.""" + + def test_tmdb_api_key_default_none(self): + """Test TMDB API key defaults to None.""" + s = Settings() + assert s.tmdb_api_key is None + + def test_nfo_auto_create_default(self): + """Test NFO auto-create defaults to False.""" + s = Settings() + assert s.nfo_auto_create is False + + def test_nfo_update_on_scan_default(self): + """Test NFO update on scan defaults to False.""" + s = Settings() + assert s.nfo_update_on_scan is False + + def test_nfo_download_poster_default(self): + """Test NFO download poster defaults to True.""" + s = Settings() + assert s.nfo_download_poster is True + + def test_nfo_download_logo_default(self): + """Test NFO download logo defaults to True.""" + s = Settings() + assert s.nfo_download_logo is True + + def test_nfo_download_fanart_default(self): + """Test NFO download fanart defaults to True.""" + s = Settings() + assert s.nfo_download_fanart is True + + def test_nfo_image_size_default(self): + """Test NFO image size defaults to 'original'.""" + s = Settings() + assert s.nfo_image_size == "original" + + def test_nfo_prefer_fsk_rating_default(self): + """Test NFO prefer FSK rating defaults to True.""" + s = Settings() + assert s.nfo_prefer_fsk_rating is True + + +class TestEnvironmentVariableParsing: + """Tests for environment variable parsing.""" + + def test_jwt_secret_key_from_env(self): + """Test loading JWT secret key from environment.""" + with patch.dict(os.environ, {"JWT_SECRET_KEY": "test-secret-key"}): + s = Settings() + assert s.jwt_secret_key == "test-secret-key" + + def test_password_salt_from_env(self): + """Test loading password salt from environment.""" + with patch.dict(os.environ, {"PASSWORD_SALT": "custom-salt"}): + s = Settings() + assert s.password_salt == "custom-salt" + + def test_master_password_hash_from_env(self): + """Test loading master password hash from environment.""" + with patch.dict(os.environ, {"MASTER_PASSWORD_HASH": "hash123"}): + s = Settings() + assert s.master_password_hash == "hash123" + + def test_master_password_from_env(self): + """Test loading master password from environment.""" + with patch.dict(os.environ, {"MASTER_PASSWORD": "dev-password"}): + s = Settings() + assert s.master_password == "dev-password" + + def test_token_expiry_hours_from_env(self): + """Test loading token expiry from environment.""" + with patch.dict(os.environ, {"SESSION_TIMEOUT_HOURS": "48"}): + s = Settings() + assert s.token_expiry_hours == 48 + + def test_anime_directory_from_env(self): + """Test loading anime directory from environment.""" + with patch.dict(os.environ, {"ANIME_DIRECTORY": "/media/anime"}): + s = Settings() + assert s.anime_directory == "/media/anime" + + def test_log_level_from_env(self): + """Test loading log level from environment.""" + with patch.dict(os.environ, {"LOG_LEVEL": "DEBUG"}): + s = Settings() + assert s.log_level == "DEBUG" + + def test_database_url_from_env(self): + """Test loading database URL from environment.""" + with patch.dict(os.environ, {"DATABASE_URL": "postgresql://localhost/aniworld"}): + s = Settings() + assert s.database_url == "postgresql://localhost/aniworld" + + def test_cors_origins_from_env(self): + """Test loading CORS origins from environment.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "http://example.com"}): + s = Settings() + assert s.cors_origins == "http://example.com" + + def test_api_rate_limit_from_env(self): + """Test loading API rate limit from environment.""" + with patch.dict(os.environ, {"API_RATE_LIMIT": "200"}): + s = Settings() + assert s.api_rate_limit == 200 + + def test_default_provider_from_env(self): + """Test loading default provider from environment.""" + with patch.dict(os.environ, {"DEFAULT_PROVIDER": "custom.provider"}): + s = Settings() + assert s.default_provider == "custom.provider" + + def test_provider_timeout_from_env(self): + """Test loading provider timeout from environment.""" + with patch.dict(os.environ, {"PROVIDER_TIMEOUT": "60"}): + s = Settings() + assert s.provider_timeout == 60 + + def test_retry_attempts_from_env(self): + """Test loading retry attempts from environment.""" + with patch.dict(os.environ, {"RETRY_ATTEMPTS": "5"}): + s = Settings() + assert s.retry_attempts == 5 + + def test_tmdb_api_key_from_env(self): + """Test loading TMDB API key from environment.""" + with patch.dict(os.environ, {"TMDB_API_KEY": "tmdb-key-123"}): + s = Settings() + assert s.tmdb_api_key == "tmdb-key-123" + + +class TestNFOEnvironmentVariables: + """Tests for NFO settings from environment.""" + + def test_nfo_auto_create_from_env_true(self): + """Test loading NFO auto create as true from environment.""" + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "true"}): + s = Settings() + assert s.nfo_auto_create is True + + def test_nfo_auto_create_from_env_false(self): + """Test loading NFO auto create as false from environment.""" + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "false"}): + s = Settings() + assert s.nfo_auto_create is False + + def test_nfo_update_on_scan_from_env(self): + """Test loading NFO update on scan from environment.""" + with patch.dict(os.environ, {"NFO_UPDATE_ON_SCAN": "true"}): + s = Settings() + assert s.nfo_update_on_scan is True + + def test_nfo_download_poster_from_env_false(self): + """Test loading NFO download poster as false.""" + with patch.dict(os.environ, {"NFO_DOWNLOAD_POSTER": "false"}): + s = Settings() + assert s.nfo_download_poster is False + + def test_nfo_download_logo_from_env_false(self): + """Test loading NFO download logo as false.""" + with patch.dict(os.environ, {"NFO_DOWNLOAD_LOGO": "false"}): + s = Settings() + assert s.nfo_download_logo is False + + def test_nfo_download_fanart_from_env_false(self): + """Test loading NFO download fanart as false.""" + with patch.dict(os.environ, {"NFO_DOWNLOAD_FANART": "false"}): + s = Settings() + assert s.nfo_download_fanart is False + + def test_nfo_image_size_from_env(self): + """Test loading NFO image size from environment.""" + with patch.dict(os.environ, {"NFO_IMAGE_SIZE": "w500"}): + s = Settings() + assert s.nfo_image_size == "w500" + + def test_nfo_prefer_fsk_rating_from_env_false(self): + """Test loading NFO prefer FSK rating as false.""" + with patch.dict(os.environ, {"NFO_PREFER_FSK_RATING": "false"}): + s = Settings() + assert s.nfo_prefer_fsk_rating is False + + +class TestAllowedOriginsProperty: + """Tests for allowed_origins property.""" + + def test_allowed_origins_single(self): + """Test parsing single CORS origin.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "http://localhost:3000"}): + s = Settings() + origins = s.allowed_origins + assert origins == ["http://localhost:3000"] + + def test_allowed_origins_multiple(self): + """Test parsing multiple CORS origins.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "http://localhost:3000,http://example.com"}): + s = Settings() + origins = s.allowed_origins + assert len(origins) == 2 + assert "http://localhost:3000" in origins + assert "http://example.com" in origins + + def test_allowed_origins_with_spaces(self): + """Test parsing CORS origins with extra spaces.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "http://localhost:3000 , http://example.com "}): + s = Settings() + origins = s.allowed_origins + assert len(origins) == 2 + assert "http://localhost:3000" in origins + assert "http://example.com" in origins + + def test_allowed_origins_wildcard_safe_fallback(self): + """Test that wildcard falls back to safe defaults.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "*"}): + s = Settings() + origins = s.allowed_origins + assert "http://localhost:3000" in origins + assert "http://localhost:8000" in origins + # Should not allow all origins + assert "*" not in origins + + def test_allowed_origins_empty_string(self): + """Test parsing empty CORS origins.""" + with patch.dict(os.environ, {"CORS_ORIGINS": ""}): + s = Settings() + origins = s.allowed_origins + assert origins == [] + + def test_allowed_origins_whitespace_only(self): + """Test parsing whitespace-only CORS origins.""" + with patch.dict(os.environ, {"CORS_ORIGINS": " "}): + s = Settings() + origins = s.allowed_origins + assert origins == [] + + def test_allowed_origins_filters_empty_items(self): + """Test that empty items are filtered from comma-separated list.""" + with patch.dict(os.environ, {"CORS_ORIGINS": "http://localhost:3000,,http://example.com,"}): + s = Settings() + origins = s.allowed_origins + assert len(origins) == 2 + assert "http://localhost:3000" in origins + assert "http://example.com" in origins + + +class TestSettingsValidation: + """Tests for settings validation.""" + + def test_invalid_token_expiry_hours_type(self): + """Test that invalid token expiry type raises validation error.""" + with patch.dict(os.environ, {"SESSION_TIMEOUT_HOURS": "not-a-number"}): + with pytest.raises(ValidationError): + Settings() + + def test_invalid_api_rate_limit_type(self): + """Test that invalid rate limit type raises validation error.""" + with patch.dict(os.environ, {"API_RATE_LIMIT": "not-a-number"}): + with pytest.raises(ValidationError): + Settings() + + def test_invalid_provider_timeout_type(self): + """Test that invalid timeout type raises validation error.""" + with patch.dict(os.environ, {"PROVIDER_TIMEOUT": "not-a-number"}): + with pytest.raises(ValidationError): + Settings() + + def test_invalid_retry_attempts_type(self): + """Test that invalid retry attempts type raises validation error.""" + with patch.dict(os.environ, {"RETRY_ATTEMPTS": "not-a-number"}): + with pytest.raises(ValidationError): + Settings() + + def test_invalid_boolean_value(self): + """Test that invalid boolean raises validation error.""" + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "not-a-boolean"}): + with pytest.raises(ValidationError): + Settings() + + +class TestGlobalSettingsInstance: + """Tests for global settings instance.""" + + def test_settings_instance_exists(self): + """Test that global settings instance is created.""" + assert settings is not None + assert isinstance(settings, Settings) + + def test_settings_has_jwt_secret_key(self): + """Test that global settings has JWT secret key.""" + assert settings.jwt_secret_key is not None + assert len(settings.jwt_secret_key) > 0 + + def test_settings_has_default_database_url(self): + """Test that global settings has default database URL.""" + # Will be default unless overridden + assert settings.database_url is not None + + +class TestSettingsExtraIgnored: + """Tests for extra fields handling.""" + + def test_unknown_env_vars_ignored(self): + """Test that unknown environment variables are ignored.""" + with patch.dict(os.environ, {"UNKNOWN_SETTING": "value", "JWT_SECRET_KEY": "test-key"}): + s = Settings() + assert s.jwt_secret_key == "test-key" + # Should not raise error for unknown setting + assert not hasattr(s, "UNKNOWN_SETTING") + assert not hasattr(s, "unknown_setting") + + +class TestSettingsEdgeCases: + """Tests for edge cases in settings.""" + + def test_numeric_string_values(self): + """Test that numeric strings are correctly parsed.""" + with patch.dict(os.environ, { + "API_RATE_LIMIT": "250", + "PROVIDER_TIMEOUT": "45", + "RETRY_ATTEMPTS": "7" + }): + s = Settings() + assert s.api_rate_limit == 250 + assert s.provider_timeout == 45 + assert s.retry_attempts == 7 + + def test_boolean_string_variations(self): + """Test different boolean string representations.""" + # Test true variations + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "1"}): + s = Settings() + assert s.nfo_auto_create is True + + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "yes"}): + s = Settings() + assert s.nfo_auto_create is True + + # Test false variations + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "0"}): + s = Settings() + assert s.nfo_auto_create is False + + with patch.dict(os.environ, {"NFO_AUTO_CREATE": "no"}): + s = Settings() + assert s.nfo_auto_create is False + + def test_empty_anime_directory_valid(self): + """Test that empty anime directory is valid.""" + with patch.dict(os.environ, {"ANIME_DIRECTORY": ""}): + s = Settings() + assert s.anime_directory == "" + + def test_path_with_spaces(self): + """Test that paths with spaces are preserved.""" + with patch.dict(os.environ, {"ANIME_DIRECTORY": "/media/my anime"}): + s = Settings() + assert s.anime_directory == "/media/my anime" + + def test_url_formats(self): + """Test various database URL formats.""" + urls = [ + "sqlite:///./data/aniworld.db", + "postgresql://user:pass@localhost/db", + "mysql://localhost:3306/aniworld", + ] + + for url in urls: + with patch.dict(os.environ, {"DATABASE_URL": url}): + s = Settings() + assert s.database_url == url + + def test_log_level_case_insensitive(self): + """Test that log level accepts different cases.""" + levels = ["DEBUG", "debug", "Info", "WARNING", "ERROR"] + + for level in levels: + with patch.dict(os.environ, {"LOG_LEVEL": level}): + s = Settings() + assert s.log_level == level + + +class TestSecurityConsiderations: + """Tests for security-related settings behavior.""" + + def test_jwt_secret_is_different_each_time(self): + """Test that JWT secret is not predictable.""" + secrets_generated = set() + + for _ in range(10): + s = Settings() + secrets_generated.add(s.jwt_secret_key) + + # All should be unique + assert len(secrets_generated) == 10 + + def test_master_password_warning_documented(self): + """Test that master_password field has warning documentation.""" + # Check that the field metadata includes warning + field_info = Settings.model_fields["master_password"] + description = field_info.description + assert description is not None + assert "DEVELOPMENT ONLY" in description + assert "NEVER" in description.upper() + + def test_both_password_fields_can_be_none(self): + """Test that both password fields being None is valid.""" + s = Settings() + assert s.master_password is None + assert s.master_password_hash is None + # This is valid - password will be set during setup