test: replace logger patch with caplog in remaining test files (#37562)

This commit is contained in:
Prajeeth Channa 2026-06-16 23:37:09 -04:00 committed by GitHub
parent 6ab5cf109b
commit f992ede836
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 66 additions and 68 deletions

View File

@ -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

View File

@ -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())

View File

@ -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"

View File

@ -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)

View File

@ -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
# ---------------------------------------------------------------------------

View File

@ -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: