-
Notifications
You must be signed in to change notification settings - Fork 0
test: añadir tests para security, telemetry y logging #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||||||||
| """Tests for logging module.""" | ||||||||||||
|
|
||||||||||||
| import logging | ||||||||||||
| import os | ||||||||||||
| from unittest.mock import patch | ||||||||||||
|
|
||||||||||||
| import pytest | ||||||||||||
|
|
||||||||||||
| from app.utils.logging import get_logger, setup_logging | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| class TestSetupLogging: | ||||||||||||
| """Tests for setup_logging function.""" | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_returns_logger(self): | ||||||||||||
| """Test that setup_logging returns a logger instance.""" | ||||||||||||
| logger = setup_logging() | ||||||||||||
| assert isinstance(logger, logging.Logger) | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_default_level(self): | ||||||||||||
| """Test setup_logging with default INFO level.""" | ||||||||||||
| with patch.dict(os.environ, {}, clear=False): | ||||||||||||
| os.environ.pop("LOG_LEVEL", None) | ||||||||||||
| logger = setup_logging() | ||||||||||||
| assert logger.level == logging.INFO | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_debug_level(self): | ||||||||||||
| """Test setup_logging with DEBUG level.""" | ||||||||||||
| with patch.dict(os.environ, {"LOG_LEVEL": "DEBUG"}, clear=False): | ||||||||||||
| logger = setup_logging() | ||||||||||||
| assert logger.level == logging.DEBUG | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_warning_level(self): | ||||||||||||
| """Test setup_logging with WARNING level.""" | ||||||||||||
| with patch.dict(os.environ, {"LOG_LEVEL": "WARNING"}, clear=False): | ||||||||||||
| logger = setup_logging() | ||||||||||||
| assert logger.level == logging.WARNING | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_invalid_level_defaults_to_info(self): | ||||||||||||
| """Test setup_logging with invalid level defaults to INFO.""" | ||||||||||||
| with patch.dict(os.environ, {"LOG_LEVEL": "INVALID"}, clear=False): | ||||||||||||
| logger = setup_logging() | ||||||||||||
| assert logger.level == logging.INFO | ||||||||||||
|
|
||||||||||||
| def test_setup_logging_clears_existing_handlers(self): | ||||||||||||
| """Test that setup_logging clears existing handlers.""" | ||||||||||||
| # Add a dummy handler | ||||||||||||
| root = logging.getLogger() | ||||||||||||
| dummy_handler = logging.StreamHandler() | ||||||||||||
| root.addHandler(dummy_handler) | ||||||||||||
|
|
||||||||||||
| # Setup logging should clear it | ||||||||||||
| setup_logging() | ||||||||||||
|
|
||||||||||||
| # Verify only one handler exists | ||||||||||||
| assert len(root.handlers) == 1 | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
Comment on lines
+55
to
+58
|
||||||||||||
| # Verify only one handler exists | |
| assert len(root.handlers) == 1 | |
| # Verify only one handler exists and dummy_handler is not present | |
| assert len(root.handlers) == 1 | |
| assert dummy_handler not in root.handlers |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,140 @@ | ||||||||||||
| """Tests for security module.""" | ||||||||||||
|
|
||||||||||||
| import os | ||||||||||||
| from unittest.mock import patch | ||||||||||||
|
|
||||||||||||
| import pytest | ||||||||||||
|
||||||||||||
| import pytest |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test patches the environment with a SECRET_KEY and API_KEY but then immediately removes API_KEY with os.environ.pop(). This creates confusion about the test's intent. The environment should either be set up correctly from the start (without API_KEY) or the patch.dict should not include it. The current approach unnecessarily patches then removes the value.
| env = {"SECRET_KEY": "test-secret", "API_KEY": ""} | |
| with patch.dict(os.environ, env, clear=False): | |
| os.environ.pop("API_KEY", None) | |
| env = {"SECRET_KEY": "test-secret"} | |
| with patch.dict(os.environ, env, clear=False): |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion message checks for "16 characters" in the warning, but according to the security.py source code (line 47), the actual warning message says "at least 16 characters long". The test should check for the complete or more specific phrase to ensure it matches the actual warning message exactly.
| assert any("16 characters" in w for w in result["warnings"]) | |
| assert any("at least 16 characters long" in w for w in result["warnings"]) |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the test_missing_api_key test, this test patches environment variables but then immediately removes them with os.environ.pop(). This is unnecessarily complex. The environment variables should simply not be included in the patch.dict in the first place if the goal is to test their absence.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,69 @@ | ||||||
| """Tests for telemetry module.""" | ||||||
|
|
||||||
| import logging | ||||||
| from unittest.mock import MagicMock, patch | ||||||
|
||||||
| from unittest.mock import MagicMock, patch |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.
| import pytest |
Copilot
AI
Dec 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion on line 69 uses an OR condition that will almost always pass. Since the test is checking for an error status code (500), the assertion should verify that "500" appears in the log output, not fall back to checking for "GET" which would be present in all GET requests. This makes the test less effective at catching issues with error status code logging.
| assert "500" in caplog.text or "GET" in caplog.text | |
| assert "500" in caplog.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'pytest' is not used.