From d62a9a67205139ce4e426e0de5b526804e8d446e Mon Sep 17 00:00:00 2001 From: Cristhian Zanforlin Lousa Date: Mon, 19 Jan 2026 08:08:38 -0300 Subject: [PATCH] test: Standardize service mocking with shared pytest fixtures for reliable test isolation (#11350) --- src/lfx/tests/unit/services/conftest.py | 54 +++++++++++++++++++ .../services/test_decorator_registration.py | 20 ++++--- .../tests/unit/services/test_integration.py | 30 +++++++++-- .../unit/services/test_minimal_services.py | 21 ++++---- .../unit/services/test_service_manager.py | 15 +++--- 5 files changed, 113 insertions(+), 27 deletions(-) create mode 100644 src/lfx/tests/unit/services/conftest.py diff --git a/src/lfx/tests/unit/services/conftest.py b/src/lfx/tests/unit/services/conftest.py new file mode 100644 index 0000000000..efa23d3d9a --- /dev/null +++ b/src/lfx/tests/unit/services/conftest.py @@ -0,0 +1,54 @@ +"""Shared fixtures for service tests.""" + +from unittest.mock import MagicMock + +import pytest +from lfx.services.base import Service +from lfx.services.manager import ServiceManager +from lfx.services.schema import ServiceType + + +class MockSessionService(Service): + """Mock session service for testing. + + Provides a minimal session service implementation that satisfies + the dependency requirements of services like LocalStorageService. + """ + + name = "session_service" + + def __init__(self): + super().__init__() + self.set_ready() + + async def teardown(self) -> None: + pass + + +@pytest.fixture +def mock_session_service(): + """Create a mock session service using MagicMock.""" + return MagicMock() + + +@pytest.fixture +def mock_settings_service(tmp_path): + """Create a mock settings service with tmp_path as config_dir.""" + mock = MagicMock() + mock.settings.config_dir = str(tmp_path) + return mock + + +@pytest.fixture +def service_manager_with_session(): + """Create a ServiceManager with MockSessionService registered. + + This fixture is useful for tests that need to create services + via the ServiceManager that depend on session_service. + """ + import asyncio + + manager = ServiceManager() + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) + yield manager + asyncio.run(manager.teardown()) diff --git a/src/lfx/tests/unit/services/test_decorator_registration.py b/src/lfx/tests/unit/services/test_decorator_registration.py index d8ebc5b233..375d94478c 100644 --- a/src/lfx/tests/unit/services/test_decorator_registration.py +++ b/src/lfx/tests/unit/services/test_decorator_registration.py @@ -1,5 +1,7 @@ """Tests for decorator-based service registration.""" +from unittest.mock import MagicMock + import pytest from lfx.services.base import Service from lfx.services.manager import ServiceManager @@ -8,15 +10,17 @@ from lfx.services.storage.local import LocalStorageService from lfx.services.telemetry.service import TelemetryService from lfx.services.tracing.service import TracingService +from .conftest import MockSessionService + @pytest.fixture def clean_manager(): """Create a fresh ServiceManager for testing decorators.""" - manager = ServiceManager() - yield manager - # Cleanup import asyncio + manager = ServiceManager() + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) + yield manager asyncio.run(manager.teardown()) @@ -111,7 +115,11 @@ class TestDecoratorRegistration: clean_manager.register_service_class(ServiceType.VARIABLE_SERVICE, LocalStorageService, override=True) # Class should still be usable directly (not just through manager) - direct_instance = LocalStorageService() + # Create mock dependencies for direct instantiation + mock_session = MagicMock() + mock_settings = MagicMock() + mock_settings.settings.config_dir = "/tmp/test" + direct_instance = LocalStorageService(mock_session, mock_settings) assert direct_instance.ready is True assert direct_instance.name == "storage_service" @@ -121,8 +129,8 @@ class TestDecoratorRegistration: clean_manager.register_service_class(ServiceType.TELEMETRY_SERVICE, TelemetryService, override=True) clean_manager.register_service_class(ServiceType.TRACING_SERVICE, TracingService, override=True) - # All should be registered - assert len(clean_manager.service_classes) == 3 + # All should be registered (plus SESSION_SERVICE from fixture) + assert len(clean_manager.service_classes) >= 3 assert ServiceType.STORAGE_SERVICE in clean_manager.service_classes assert ServiceType.TELEMETRY_SERVICE in clean_manager.service_classes assert ServiceType.TRACING_SERVICE in clean_manager.service_classes diff --git a/src/lfx/tests/unit/services/test_integration.py b/src/lfx/tests/unit/services/test_integration.py index b1f21a407d..c796e54f95 100644 --- a/src/lfx/tests/unit/services/test_integration.py +++ b/src/lfx/tests/unit/services/test_integration.py @@ -7,6 +7,8 @@ from lfx.services.base import Service from lfx.services.manager import ServiceManager from lfx.services.schema import ServiceType +from .conftest import MockSessionService + class TestStandaloneLFX: """Test LFX running standalone without langflow.""" @@ -15,6 +17,8 @@ class TestStandaloneLFX: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -92,6 +96,8 @@ cache_service = "lfx.services.cache.service:ThreadingInMemoryCache" def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -126,6 +132,8 @@ class TestServiceOverrideScenarios: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -194,6 +202,8 @@ class TestErrorConditions: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -233,8 +243,9 @@ invalid_service_type = "some.module:SomeClass" # Should not raise, just log warning clean_manager.discover_plugins(config_dir) - # No services should be registered (invalid key) - assert len(clean_manager.service_classes) == 0 + # No services should be registered from config (only SESSION_SERVICE from fixture) + assert len(clean_manager.service_classes) == 1 + assert ServiceType.SESSION_SERVICE in clean_manager.service_classes def test_malformed_toml_in_config(self, clean_manager, tmp_path): """Test handling of malformed TOML.""" @@ -251,8 +262,9 @@ storage_service = "lfx.services.storage.local:LocalStorageService" # Should not raise, just log warning clean_manager.discover_plugins(config_dir) - # No services should be registered - assert len(clean_manager.service_classes) == 0 + # No services should be registered from config (only SESSION_SERVICE from fixture) + assert len(clean_manager.service_classes) == 1 + assert ServiceType.SESSION_SERVICE in clean_manager.service_classes def test_service_without_name_attribute(self, clean_manager): """Test registering a service without name attribute.""" @@ -295,6 +307,8 @@ class TestDependencyResolution: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -390,6 +404,8 @@ class TestConfigFileDiscovery: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio @@ -446,7 +462,9 @@ cache_service = "lfx.services.cache.service:ThreadingInMemoryCache" # Should not raise clean_manager.discover_plugins(config_dir) - assert len(clean_manager.service_classes) == 0 + # No services should be registered from config (only SESSION_SERVICE from fixture) + assert len(clean_manager.service_classes) == 1 + assert ServiceType.SESSION_SERVICE in clean_manager.service_classes class TestEnvironmentVariableIntegration: @@ -456,6 +474,8 @@ class TestEnvironmentVariableIntegration: def clean_manager(self): """Create a clean ServiceManager instance.""" manager = ServiceManager() + # Register mock session service as dependency for LocalStorageService + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) yield manager # Cleanup import asyncio diff --git a/src/lfx/tests/unit/services/test_minimal_services.py b/src/lfx/tests/unit/services/test_minimal_services.py index eef985f663..ee357cd666 100644 --- a/src/lfx/tests/unit/services/test_minimal_services.py +++ b/src/lfx/tests/unit/services/test_minimal_services.py @@ -13,9 +13,9 @@ class TestLocalStorageService: """Tests for LocalStorageService.""" @pytest.fixture - def storage(self, tmp_path): + def storage(self, mock_session_service, mock_settings_service): """Create a storage service with temp directory.""" - return LocalStorageService(data_dir=tmp_path) + return LocalStorageService(mock_session_service, mock_settings_service) @pytest.mark.asyncio async def test_save_and_get_file(self, storage): @@ -61,10 +61,11 @@ class TestLocalStorageService: with pytest.raises(FileNotFoundError): await storage.get_file("flow_123", "nonexistent.txt") - def test_build_full_path(self, storage, tmp_path): + def test_build_full_path(self, storage, mock_settings_service): """Test building full file path.""" path = storage.build_full_path("flow_123", "test.txt") - expected = str(tmp_path / "flow_123" / "test.txt") + config_dir = mock_settings_service.settings.config_dir + expected = f"{config_dir}/flow_123/test.txt" assert path == expected @pytest.mark.asyncio @@ -216,9 +217,9 @@ class TestMinimalServicesIntegration: """Integration tests for minimal services working together.""" @pytest.mark.asyncio - async def test_all_minimal_services_initialize(self, tmp_path): + async def test_all_minimal_services_initialize(self, mock_session_service, mock_settings_service): """Test that all minimal services can be initialized.""" - storage = LocalStorageService(data_dir=tmp_path) + storage = LocalStorageService(mock_session_service, mock_settings_service) telemetry = TelemetryService() tracing = TracingService() variables = VariableService() @@ -229,9 +230,9 @@ class TestMinimalServicesIntegration: assert variables.ready @pytest.mark.asyncio - async def test_minimal_services_teardown_all(self, tmp_path): + async def test_minimal_services_teardown_all(self, mock_session_service, mock_settings_service): """Test tearing down all minimal services.""" - storage = LocalStorageService(data_dir=tmp_path) + storage = LocalStorageService(mock_session_service, mock_settings_service) telemetry = TelemetryService() tracing = TracingService() variables = VariableService() @@ -243,9 +244,9 @@ class TestMinimalServicesIntegration: await variables.teardown() @pytest.mark.asyncio - async def test_storage_with_tracing(self, tmp_path): + async def test_storage_with_tracing(self, mock_session_service, mock_settings_service): """Test using storage with tracing.""" - storage = LocalStorageService(data_dir=tmp_path) + storage = LocalStorageService(mock_session_service, mock_settings_service) tracing = TracingService() tracing.add_log("storage_test", {"operation": "save", "flow_id": "123"}) diff --git a/src/lfx/tests/unit/services/test_service_manager.py b/src/lfx/tests/unit/services/test_service_manager.py index 943e5ad44b..44b50ee327 100644 --- a/src/lfx/tests/unit/services/test_service_manager.py +++ b/src/lfx/tests/unit/services/test_service_manager.py @@ -11,15 +11,17 @@ from lfx.services.telemetry.service import TelemetryService from lfx.services.tracing.service import TracingService from lfx.services.variable.service import VariableService +from .conftest import MockSessionService + @pytest.fixture def service_manager(): """Create a fresh ServiceManager for each test.""" - manager = ServiceManager() - yield manager - # Cleanup import asyncio + manager = ServiceManager() + manager.register_service_class(ServiceType.SESSION_SERVICE, MockSessionService, override=True) + yield manager asyncio.run(manager.teardown()) @@ -48,7 +50,8 @@ class TestServiceRegistration: service_manager.register_service_class(ServiceType.TRACING_SERVICE, TracingService, override=True) service_manager.register_service_class(ServiceType.VARIABLE_SERVICE, VariableService, override=True) - assert len(service_manager.service_classes) == 4 + # 4 services + SESSION_SERVICE from fixture = 5 + assert len(service_manager.service_classes) == 5 assert service_manager.service_classes[ServiceType.STORAGE_SERVICE] == LocalStorageService assert service_manager.service_classes[ServiceType.TELEMETRY_SERVICE] == TelemetryService assert service_manager.service_classes[ServiceType.TRACING_SERVICE] == TracingService @@ -464,8 +467,8 @@ variable_service = "lfx.services.variable.service:VariableService" service_manager.discover_plugins(temp_config_dir) - # All services should be registered - assert len(service_manager.service_classes) == 4 + # All services should be registered (4 from config + SESSION_SERVICE from fixture = 5) + assert len(service_manager.service_classes) == 5 # Create and verify each service storage = service_manager.get(ServiceType.STORAGE_SERVICE)