diff --git a/api/tests/test_containers_integration_tests/services/test_end_user_service.py b/api/tests/test_containers_integration_tests/services/test_end_user_service.py index 998b3378e2..af6fb879ac 100644 --- a/api/tests/test_containers_integration_tests/services/test_end_user_service.py +++ b/api/tests/test_containers_integration_tests/services/test_end_user_service.py @@ -1,7 +1,6 @@ from __future__ import annotations import logging -from unittest.mock import patch from uuid import uuid4 import pytest @@ -259,9 +258,8 @@ class TestEndUserServiceGetOrCreateEndUserByType: assert len(matching_logs) == 1 - @patch("services.end_user_service.logger") def test_get_existing_end_user_matching_type( - self, mock_logger, db_session_with_containers: Session, factory: TestEndUserServiceFactory + self, db_session_with_containers: Session, factory: TestEndUserServiceFactory, caplog ): """Test retrieving existing end user with matching type.""" # Arrange @@ -279,17 +277,19 @@ class TestEndUserServiceGetOrCreateEndUserByType: ) # Act - Request with same type - result = EndUserService.get_or_create_end_user_by_type( - type=InvokeFrom.SERVICE_API, - tenant_id=tenant_id, - app_id=app_id, - user_id=user_id, - ) + with caplog.at_level(logging.INFO, logger="services.end_user_service"): + result = EndUserService.get_or_create_end_user_by_type( + type=InvokeFrom.SERVICE_API, + tenant_id=tenant_id, + app_id=app_id, + user_id=user_id, + ) # Assert assert result.id == existing_user.id assert result.type == InvokeFrom.SERVICE_API - mock_logger.info.assert_not_called() + # No legacy-upgrade log should be emitted when the existing user's type already matches. + assert [record for record in caplog.records if record.levelno == logging.INFO] == [] def test_create_anonymous_user_with_default_session( self, db_session_with_containers: Session, factory: TestEndUserServiceFactory diff --git a/api/tests/test_containers_integration_tests/tasks/test_remove_app_and_related_data_task.py b/api/tests/test_containers_integration_tests/tasks/test_remove_app_and_related_data_task.py index 204f533978..0ec1b87f59 100644 --- a/api/tests/test_containers_integration_tests/tasks/test_remove_app_and_related_data_task.py +++ b/api/tests/test_containers_integration_tests/tasks/test_remove_app_and_related_data_task.py @@ -1,5 +1,6 @@ +import logging import uuid -from unittest.mock import ANY, call, patch +from unittest.mock import call, patch import pytest from sqlalchemy import delete, func, select @@ -146,10 +147,7 @@ class TestDeleteDraftVariablesBatch: assert db_session_with_containers.scalar(select(func.count()).select_from(WorkflowDraftVariable)) == 0 @patch("tasks.remove_app_and_related_data_task._delete_draft_variable_offload_data") - @patch("tasks.remove_app_and_related_data_task.logger") - def test_delete_draft_variables_batch_logs_progress( - self, mock_logger, mock_offload_cleanup, db_session_with_containers - ): + def test_delete_draft_variables_batch_logs_progress(self, mock_offload_cleanup, db_session_with_containers, caplog): """Test that batch deletion logs progress correctly.""" tenant, app = _create_tenant_and_app(db_session_with_containers) offload_data = _create_offload_data(db_session_with_containers, tenant_id=tenant.id, app_id=app.id, count=10) @@ -163,14 +161,15 @@ class TestDeleteDraftVariablesBatch: mock_offload_cleanup.return_value = len(file_id_by_index) - result = delete_draft_variables_batch(app.id, 50) + with caplog.at_level(logging.INFO, logger="tasks.remove_app_and_related_data_task"): + result = delete_draft_variables_batch(app.id, 50) assert result == 30 mock_offload_cleanup.assert_called_once() _, called_file_ids = mock_offload_cleanup.call_args.args assert {str(file_id) for file_id in called_file_ids} == {str(file_id) for file_id in file_id_by_index.values()} - assert mock_logger.info.call_count == 2 - mock_logger.info.assert_any_call(ANY) + info_records = [record for record in caplog.records if record.levelno == logging.INFO] + assert len(info_records) == 2 class TestDeleteDraftVariableOffloadData: @@ -204,10 +203,7 @@ class TestDeleteDraftVariableOffloadData: assert remaining_upload_files_count == 0 @patch("extensions.ext_storage.storage") - @patch("tasks.remove_app_and_related_data_task.logging") - def test_delete_draft_variable_offload_data_storage_failure( - self, mock_logging, mock_storage, db_session_with_containers - ): + def test_delete_draft_variable_offload_data_storage_failure(self, mock_storage, db_session_with_containers, caplog): """Test handling of storage deletion failures.""" tenant, app = _create_tenant_and_app(db_session_with_containers) offload_data = _create_offload_data(db_session_with_containers, tenant_id=tenant.id, app_id=app.id, count=2) @@ -217,11 +213,12 @@ class TestDeleteDraftVariableOffloadData: mock_storage.delete.side_effect = [Exception("Storage error"), None] - with session_factory.create_session() as session, session.begin(): - result = _delete_draft_variable_offload_data(session, file_ids) + with caplog.at_level(logging.ERROR): + with session_factory.create_session() as session, session.begin(): + result = _delete_draft_variable_offload_data(session, file_ids) assert result == 1 - mock_logging.exception.assert_called_once_with("Failed to delete storage object %s", storage_keys[0]) + assert f"Failed to delete storage object {storage_keys[0]}" in caplog.text remaining_var_files_count = db_session_with_containers.scalar( select(func.count()) diff --git a/api/tests/unit_tests/controllers/service_api/app/test_file_preview.py b/api/tests/unit_tests/controllers/service_api/app/test_file_preview.py index f5e8453c5c..6fe1cbb71d 100644 --- a/api/tests/unit_tests/controllers/service_api/app/test_file_preview.py +++ b/api/tests/unit_tests/controllers/service_api/app/test_file_preview.py @@ -2,6 +2,7 @@ Unit tests for Service API File Preview endpoint """ +import logging import uuid from unittest.mock import Mock, patch @@ -348,8 +349,7 @@ class TestFilePreviewApi: assert "Storage error" in str(exc_info.value) - @patch("controllers.service_api.app.file_preview.logger") - def test_validate_file_ownership_unexpected_error_logging(self, mock_logger, file_preview_api: FilePreviewApi): + def test_validate_file_ownership_unexpected_error_logging(self, file_preview_api: FilePreviewApi, caplog): """Test that unexpected errors are logged properly""" file_id = str(uuid.uuid4()) app_id = str(uuid.uuid4()) @@ -359,14 +359,18 @@ class TestFilePreviewApi: mock_db.session.scalar.side_effect = Exception("Unexpected database error") # Execute and assert exception - with pytest.raises(FileAccessDeniedError) as exc_info: - file_preview_api._validate_file_ownership(file_id, app_id) + with caplog.at_level(logging.ERROR, logger="controllers.service_api.app.file_preview"): + with pytest.raises(FileAccessDeniedError) as exc_info: + file_preview_api._validate_file_ownership(file_id, app_id) # Verify error message assert "File access validation failed" in str(exc_info.value) - # Verify logging was called - mock_logger.exception.assert_called_once_with( - "Unexpected error during file ownership validation", - extra={"file_id": file_id, "app_id": app_id, "error": "Unexpected database error"}, - ) + # Verify logging was called with the structured context fields. The ``extra`` keys + # are attached to the LogRecord as attributes, so they are not in ``caplog.text``. + assert len(caplog.records) == 1 + record = caplog.records[0] + assert record.getMessage() == "Unexpected error during file ownership validation" + assert record.file_id == file_id + assert record.app_id == app_id + assert record.error == "Unexpected database error" diff --git a/api/tests/unit_tests/core/mcp/session/test_base_session.py b/api/tests/unit_tests/core/mcp/session/test_base_session.py index 1dd916bcf1..7215551551 100644 --- a/api/tests/unit_tests/core/mcp/session/test_base_session.py +++ b/api/tests/unit_tests/core/mcp/session/test_base_session.py @@ -1,3 +1,4 @@ +import logging import queue import time from concurrent.futures import Future, ThreadPoolExecutor @@ -511,10 +512,8 @@ def test_receive_loop_http_error_unknown_id(streams): @pytest.mark.timeout(10) -def test_receive_loop_validation_error_notification(streams): - from core.mcp.session.base_session import logger - - with patch.object(logger, "warning") as mock_warning: +def test_receive_loop_validation_error_notification(streams, caplog): + with caplog.at_level(logging.WARNING, logger="core.mcp.session.base_session"): read_stream, write_stream = streams session = MockSession(read_stream, write_stream, ReceiveRequest, RootModel[MockNotification]) @@ -523,7 +522,7 @@ def test_receive_loop_validation_error_notification(streams): read_stream.put(SessionMessage(message=JSONRPCMessage.model_validate(notif_payload))) time.sleep(1.0) - assert mock_warning.called + assert "Failed to validate notification" in caplog.text @pytest.mark.timeout(5) @@ -571,16 +570,16 @@ def test_session_exit_timeout(streams): @pytest.mark.timeout(10) -def test_receive_loop_fatal_exception(streams): +def test_receive_loop_fatal_exception(streams, caplog): read_stream, write_stream = streams session = MockSession(read_stream, write_stream, ReceiveRequest, ReceiveNotification) with patch.object(read_stream, "get", side_effect=RuntimeError("Fatal loop error")): - with patch("core.mcp.session.base_session.logger") as mock_logger: + with caplog.at_level(logging.ERROR, logger="core.mcp.session.base_session"): with pytest.raises(RuntimeError, match="Fatal loop error"): with session: pass - mock_logger.exception.assert_called_with("Error in message processing loop") + assert "Error in message processing loop" in caplog.text @pytest.mark.timeout(5) diff --git a/api/tests/unit_tests/enterprise/telemetry/test_exporter.py b/api/tests/unit_tests/enterprise/telemetry/test_exporter.py index 6bdae13923..674a202613 100644 --- a/api/tests/unit_tests/enterprise/telemetry/test_exporter.py +++ b/api/tests/unit_tests/enterprise/telemetry/test_exporter.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from datetime import UTC, datetime from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -88,11 +89,10 @@ def test_api_key_and_custom_headers_merge(mock_metric_exporter: MagicMock, mock_ assert ("x-custom", "foo") in headers -@patch("enterprise.telemetry.exporter.logger") @patch("enterprise.telemetry.exporter.GRPCSpanExporter") @patch("enterprise.telemetry.exporter.GRPCMetricExporter") def test_api_key_overrides_conflicting_header( - mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock, mock_logger: MagicMock + mock_metric_exporter: MagicMock, mock_span_exporter: MagicMock, caplog ) -> None: """Test that API key overrides conflicting authorization header and logs warning.""" mock_config = SimpleNamespace( @@ -105,7 +105,8 @@ def test_api_key_overrides_conflicting_header( ENTERPRISE_OTLP_API_KEY="test-key", ) - EnterpriseExporter(mock_config) + with caplog.at_level(logging.WARNING, logger="enterprise.telemetry.exporter"): + EnterpriseExporter(mock_config) # Verify Bearer header takes precedence assert mock_span_exporter.call_args is not None @@ -116,11 +117,8 @@ def test_api_key_overrides_conflicting_header( assert ("authorization", "Basic old") not in headers # Verify warning was logged - mock_logger.warning.assert_called_once() - assert mock_logger.warning.call_args is not None - warning_message = mock_logger.warning.call_args[0][0] - assert "ENTERPRISE_OTLP_API_KEY is set" in warning_message - assert "authorization" in warning_message + assert "ENTERPRISE_OTLP_API_KEY is set" in caplog.text + assert "authorization" in caplog.text @patch("enterprise.telemetry.exporter.GRPCSpanExporter") @@ -535,33 +533,33 @@ def test_export_span_cross_workflow_parent_context() -> None: assert kwargs["context"] is not None -@patch("enterprise.telemetry.exporter.logger") -def test_export_span_logs_exception_on_error(mock_logger: MagicMock) -> None: +def test_export_span_logs_exception_on_error(caplog) -> None: """If the span block raises, the exception is logged and context is still cleared.""" exporter, mock_tracer, mock_span = _make_exporter_with_mock_tracer() mock_tracer.start_as_current_span.side_effect = RuntimeError("boom") - exporter.export_span(name="bad.span", attributes={}) # must not raise + with caplog.at_level(logging.ERROR, logger="enterprise.telemetry.exporter"): + exporter.export_span(name="bad.span", attributes={}) # must not raise - mock_logger.exception.assert_called_once() - assert "bad.span" in mock_logger.exception.call_args[0][1] + assert "Failed to export span" in caplog.text + assert "bad.span" in caplog.text -@patch("enterprise.telemetry.exporter.logger") -def test_export_span_invalid_trace_correlation_logs_warning(mock_logger: MagicMock) -> None: +def test_export_span_invalid_trace_correlation_logs_warning(caplog) -> None: """Invalid UUID for trace_correlation_override triggers a warning log.""" exporter, mock_tracer, mock_span = _make_exporter_with_mock_tracer() parent_uid = "987fbc97-4bed-5078-9f07-9141ba07c9f3" - exporter.export_span( - name="link.span", - attributes={}, - correlation_id="not-a-valid-uuid", - parent_span_id_source=parent_uid, - ) + with caplog.at_level(logging.WARNING, logger="enterprise.telemetry.exporter"): + exporter.export_span( + name="link.span", + attributes={}, + correlation_id="not-a-valid-uuid", + parent_span_id_source=parent_uid, + ) - mock_logger.warning.assert_called() + assert "Invalid trace correlation UUID for cross-workflow link" in caplog.text # --------------------------------------------------------------------------- diff --git a/api/tests/unit_tests/tasks/test_remove_app_and_related_data_task.py b/api/tests/unit_tests/tasks/test_remove_app_and_related_data_task.py index 3fb673198b..a91b61111c 100644 --- a/api/tests/unit_tests/tasks/test_remove_app_and_related_data_task.py +++ b/api/tests/unit_tests/tasks/test_remove_app_and_related_data_task.py @@ -50,8 +50,7 @@ class TestDeleteDraftVariableOffloadData: assert result == 0 mock_conn.execute.assert_not_called() - @patch("tasks.remove_app_and_related_data_task.logging") - def test_delete_draft_variable_offload_data_database_failure(self, mock_logging): + def test_delete_draft_variable_offload_data_database_failure(self, caplog): """Test handling of database operation failures.""" mock_conn = MagicMock() file_ids = ["file-1"] @@ -60,13 +59,14 @@ class TestDeleteDraftVariableOffloadData: mock_conn.execute.side_effect = Exception("Database error") # Execute function - should not raise, but log error - result = _delete_draft_variable_offload_data(mock_conn, file_ids) + with caplog.at_level(logging.ERROR): + result = _delete_draft_variable_offload_data(mock_conn, file_ids) # Should return 0 when error occurs assert result == 0 # Verify error was logged - mock_logging.exception.assert_called_once_with("Error deleting draft variable offload data:") + assert "Error deleting draft variable offload data:" in caplog.text class TestDeleteWorkflowArchiveLogs: