diff --git a/api/tests/unit_tests/services/test_billing_service.py b/api/tests/unit_tests/services/test_billing_service.py index 67d1cc02913..f244b69407f 100644 --- a/api/tests/unit_tests/services/test_billing_service.py +++ b/api/tests/unit_tests/services/test_billing_service.py @@ -14,6 +14,7 @@ Tests follow the Arrange-Act-Assert pattern for clarity. """ import json +import logging from unittest.mock import MagicMock, patch import httpx @@ -170,7 +171,9 @@ class TestBillingServiceSendRequest: @pytest.mark.parametrize( "status_code", [httpx.codes.BAD_REQUEST, httpx.codes.INTERNAL_SERVER_ERROR, httpx.codes.NOT_FOUND] ) - def test_delete_request_non_200_with_valid_json(self, mock_httpx_request, mock_billing_config, status_code): + def test_delete_request_non_200_with_valid_json( + self, mock_httpx_request, mock_billing_config, status_code, caplog: pytest.LogCaptureFixture + ): """Test DELETE request with non-200 status code raises ValueError. DELETE now checks status code and raises ValueError for non-200 responses. @@ -184,13 +187,11 @@ class TestBillingServiceSendRequest: mock_httpx_request.return_value = mock_response # Act & Assert - with patch("services.billing_service.logger") as mock_logger: + with caplog.at_level(logging.ERROR, logger="services.billing_service"): with pytest.raises(ValueError) as exc_info: BillingService._send_request("DELETE", "/test", json={"key": "value"}) assert "Unable to process delete request" in str(exc_info.value) - # Verify error logging - mock_logger.error.assert_called_once() - assert "DELETE response" in str(mock_logger.error.call_args) + assert "DELETE response" in caplog.text @pytest.mark.parametrize( "status_code", [httpx.codes.BAD_REQUEST, httpx.codes.INTERNAL_SERVER_ERROR, httpx.codes.NOT_FOUND] @@ -213,7 +214,9 @@ class TestBillingServiceSendRequest: @pytest.mark.parametrize( "status_code", [httpx.codes.BAD_REQUEST, httpx.codes.INTERNAL_SERVER_ERROR, httpx.codes.NOT_FOUND] ) - def test_delete_request_non_200_with_invalid_json(self, mock_httpx_request, mock_billing_config, status_code): + def test_delete_request_non_200_with_invalid_json( + self, mock_httpx_request, mock_billing_config, status_code, caplog: pytest.LogCaptureFixture + ): """Test DELETE request with non-200 status code raises ValueError before JSON parsing. DELETE now checks status code before calling response.json(), so ValueError is raised @@ -227,13 +230,11 @@ class TestBillingServiceSendRequest: mock_httpx_request.return_value = mock_response # Act & Assert - with patch("services.billing_service.logger") as mock_logger: + with caplog.at_level(logging.ERROR, logger="services.billing_service"): with pytest.raises(ValueError) as exc_info: BillingService._send_request("DELETE", "/test", json={"key": "value"}) assert "Unable to process delete request" in str(exc_info.value) - # Verify error logging - mock_logger.error.assert_called_once() - assert "DELETE response" in str(mock_logger.error.call_args) + assert "DELETE response" in caplog.text def test_retry_on_request_error(self, mock_httpx_request, mock_billing_config): """Test that _send_request retries on httpx.RequestError.""" @@ -1511,7 +1512,7 @@ class TestBillingServiceSubscriptionOperations: assert isinstance(result["tenant-1"]["expiration_date"], int) assert result["tenant-1"]["expiration_date"] == 1735689600 - def test_get_plan_bulk_with_invalid_tenant_plan_skipped(self, mock_send_request): + def test_get_plan_bulk_with_invalid_tenant_plan_skipped(self, mock_send_request, caplog: pytest.LogCaptureFixture): """Test bulk plan retrieval when one tenant has invalid plan data (should skip that tenant).""" # Arrange tenant_ids = ["tenant-valid-1", "tenant-invalid", "tenant-valid-2"] @@ -1526,7 +1527,7 @@ class TestBillingServiceSubscriptionOperations: } # Act - with patch("services.billing_service.logger") as mock_logger: + with caplog.at_level(logging.ERROR, logger="services.billing_service"): result = BillingService.get_plan_bulk(tenant_ids) # Assert - should only contain valid tenants @@ -1542,10 +1543,11 @@ class TestBillingServiceSubscriptionOperations: assert result["tenant-valid-2"]["expiration_date"] == 1767225600 # Verify exception was logged for the invalid tenant - mock_logger.exception.assert_called_once() - log_call_args = mock_logger.exception.call_args[0] - assert "get_plan_bulk: failed to validate subscription plan for tenant" in log_call_args[0] - assert "tenant-invalid" in log_call_args[1] + exception_records = [r for r in caplog.records if r.levelname == "ERROR"] + assert len(exception_records) == 1 + formatted = exception_records[0].getMessage() + assert "get_plan_bulk: failed to validate subscription plan for tenant" in formatted + assert "tenant-invalid" in formatted def test_get_expired_subscription_cleanup_whitelist_success(self, mock_send_request): """Test successful retrieval of expired subscription cleanup whitelist.""" diff --git a/api/tests/unit_tests/services/test_vector_service.py b/api/tests/unit_tests/services/test_vector_service.py index a78a033f4d3..e6cc59144b3 100644 --- a/api/tests/unit_tests/services/test_vector_service.py +++ b/api/tests/unit_tests/services/test_vector_service.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from dataclasses import dataclass from typing import Any from unittest.mock import MagicMock @@ -268,7 +269,7 @@ def test_create_segments_vector_parent_child_uses_default_embedding_model_when_p def test_create_segments_vector_parent_child_missing_document_logs_warning_and_continues( - monkeypatch: pytest.MonkeyPatch, + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: dataset = _make_dataset(doc_form=vector_service_module.IndexStructureType.PARENT_CHILD_INDEX) segment = _make_segment() @@ -280,18 +281,16 @@ def test_create_segments_vector_parent_child_missing_document_logs_warning_and_c _mock_parent_child_queries(dataset_document=None, processing_rule=processing_rule), ) - logger_mock = MagicMock() - monkeypatch.setattr(vector_service_module, "logger", logger_mock) - index_processor = MagicMock() factory_instance = MagicMock() factory_instance.init_index_processor.return_value = index_processor monkeypatch.setattr(vector_service_module, "IndexProcessorFactory", MagicMock(return_value=factory_instance)) - VectorService.create_segments_vector( - None, [segment], dataset, vector_service_module.IndexStructureType.PARENT_CHILD_INDEX - ) - logger_mock.warning.assert_called_once() + with caplog.at_level(logging.WARNING, logger="services.vector_service"): + VectorService.create_segments_vector( + None, [segment], dataset, vector_service_module.IndexStructureType.PARENT_CHILD_INDEX + ) + assert "Expected DatasetDocument record to exist, but none was found" in caplog.text index_processor.load.assert_not_called() @@ -615,7 +614,7 @@ def test_update_multimodel_vector_commits_when_no_upload_files_found(monkeypatch def test_update_multimodel_vector_adds_bindings_and_vectors_and_skips_missing_upload_files( - monkeypatch: pytest.MonkeyPatch, + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: dataset = _make_dataset(indexing_technique=IndexTechniqueType.HIGH_QUALITY, is_multimodal=True) segment = _make_segment(segment_id="seg-1", tenant_id="tenant-1", attachments=[{"id": "old-1"}]) @@ -630,12 +629,10 @@ def test_update_multimodel_vector_adds_bindings_and_vectors_and_skips_missing_up monkeypatch.setattr(vector_service_module, "delete", MagicMock()) monkeypatch.setattr(vector_service_module, "select", MagicMock()) - logger_mock = MagicMock() - monkeypatch.setattr(vector_service_module, "logger", logger_mock) + with caplog.at_level(logging.WARNING, logger="services.vector_service"): + VectorService.update_multimodel_vector(segment=segment, attachment_ids=["file-1", "missing"], dataset=dataset) - VectorService.update_multimodel_vector(segment=segment, attachment_ids=["file-1", "missing"], dataset=dataset) - - logger_mock.warning.assert_called_once() + assert "Upload file not found for attachment_id" in caplog.text db_mock.session.add_all.assert_called_once() bindings = db_mock.session.add_all.call_args.args[0] assert len(bindings) == 1 @@ -673,7 +670,9 @@ def test_update_multimodel_vector_updates_bindings_without_multimodal_vector_ops db_mock.session.commit.assert_called_once() -def test_update_multimodel_vector_rolls_back_and_reraises_on_error(monkeypatch: pytest.MonkeyPatch) -> None: +def test_update_multimodel_vector_rolls_back_and_reraises_on_error( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: dataset = _make_dataset(indexing_technique=IndexTechniqueType.HIGH_QUALITY, is_multimodal=True) segment = _make_segment(segment_id="seg-1", tenant_id="tenant-1", attachments=[{"id": "old-1"}]) @@ -688,11 +687,11 @@ def test_update_multimodel_vector_rolls_back_and_reraises_on_error(monkeypatch: monkeypatch.setattr(vector_service_module, "delete", MagicMock()) monkeypatch.setattr(vector_service_module, "select", MagicMock()) - logger_mock = MagicMock() - monkeypatch.setattr(vector_service_module, "logger", logger_mock) + with caplog.at_level(logging.ERROR, logger="services.vector_service"): + with pytest.raises(RuntimeError, match="boom"): + VectorService.update_multimodel_vector(segment=segment, attachment_ids=["file-1"], dataset=dataset) - with pytest.raises(RuntimeError, match="boom"): - VectorService.update_multimodel_vector(segment=segment, attachment_ids=["file-1"], dataset=dataset) - - logger_mock.exception.assert_called_once() + exception_records = [r for r in caplog.records if r.levelname == "ERROR"] + assert len(exception_records) == 1 + assert "Failed to update multimodal vector for segment" in exception_records[0].getMessage() db_mock.session.rollback.assert_called_once()