From 2ebdfced3c47d67ab86eb683b49337e1704c6849 Mon Sep 17 00:00:00 2001 From: Mustafa Elbehery Date: Thu, 14 Aug 2025 11:32:36 +0200 Subject: [PATCH] fix(test): update LlamaStackAsLibraryClient initialization tests after removing initialize method The recent refactor (3778a4c3) introduced automatic initialization for LlamaStackAsLibraryClient but the unit tests were expecting manual initalization and _is_initialized. This caused test failure. Changes: - Update test assertions to check route_impls is not None instead of _is_initialized - Add proper mocking in tests to avoid external provider dependencies - Maintain test coverage for automatic initialization behavior - Ensure backward compatibility testing for deprecated initialize() method Signed-off-by: Mustafa Elbehery --- llama_stack/core/library_client.py | 6 +- tests/integration/fixtures/common.py | 1 - .../non_ci/responses/fixtures/fixtures.py | 1 - .../test_library_client_initialization.py | 97 ++++++++++++++----- 4 files changed, 78 insertions(+), 27 deletions(-) diff --git a/llama_stack/core/library_client.py b/llama_stack/core/library_client.py index 0a47ac652..9e7a8006c 100644 --- a/llama_stack/core/library_client.py +++ b/llama_stack/core/library_client.py @@ -149,7 +149,6 @@ class LlamaStackAsLibraryClient(LlamaStackClient): config_path_or_distro_name, custom_provider_registry, provider_data, skip_logger_removal ) self.pool_executor = ThreadPoolExecutor(max_workers=4) - self.skip_logger_removal = skip_logger_removal self.provider_data = provider_data self.loop = asyncio.new_event_loop() @@ -247,7 +246,7 @@ class AsyncLlamaStackAsLibraryClient(AsyncLlamaStackClient): async def initialize(self) -> bool: """ - Initialize the async client. Can be called multiple times safely. + Initialize the async client. Returns: bool: True if initialization was successful @@ -312,6 +311,9 @@ class AsyncLlamaStackAsLibraryClient(AsyncLlamaStackClient): stream=False, stream_cls=None, ): + if self.route_impls is None: + raise ValueError("Client not initialized. Please call initialize() first.") + # Create headers with provider data if available headers = options.headers or {} if self.provider_data: diff --git a/tests/integration/fixtures/common.py b/tests/integration/fixtures/common.py index 2e623336b..ee4c5755a 100644 --- a/tests/integration/fixtures/common.py +++ b/tests/integration/fixtures/common.py @@ -256,7 +256,6 @@ def instantiate_llama_stack_client(session): provider_data=get_provider_data(), skip_logger_removal=True, ) - # Client is automatically initialized during construction return client diff --git a/tests/integration/non_ci/responses/fixtures/fixtures.py b/tests/integration/non_ci/responses/fixtures/fixtures.py index 9ec0d8ada..1783a5622 100644 --- a/tests/integration/non_ci/responses/fixtures/fixtures.py +++ b/tests/integration/non_ci/responses/fixtures/fixtures.py @@ -113,7 +113,6 @@ def openai_client(base_url, api_key, provider): raise ValueError(f"Invalid config for Llama Stack: {provider}, it must be of the form 'stack:'") config = parts[1] client = LlamaStackAsLibraryClient(config, skip_logger_removal=True) - # Client is automatically initialized during construction return client return OpenAI( diff --git a/tests/unit/distribution/test_library_client_initialization.py b/tests/unit/distribution/test_library_client_initialization.py index 2108b4676..b7e7a1857 100644 --- a/tests/unit/distribution/test_library_client_initialization.py +++ b/tests/unit/distribution/test_library_client_initialization.py @@ -15,60 +15,111 @@ from llama_stack.core.library_client import ( AsyncLlamaStackAsLibraryClient, LlamaStackAsLibraryClient, ) +from llama_stack.core.server.routes import RouteImpls class TestLlamaStackAsLibraryClientAutoInitialization: """Test automatic initialization of library clients.""" - def test_sync_client_auto_initialization(self): + def test_sync_client_auto_initialization(self, monkeypatch): """Test that sync client is automatically initialized after construction.""" - client = LlamaStackAsLibraryClient("nvidia") + # Mock the stack construction to avoid dependency issues + mock_impls = {} + mock_route_impls = RouteImpls({}) + + async def mock_construct_stack(config, custom_provider_registry): + return mock_impls + + def mock_initialize_route_impls(impls): + return mock_route_impls + + monkeypatch.setattr("llama_stack.core.library_client.construct_stack", mock_construct_stack) + monkeypatch.setattr("llama_stack.core.library_client.initialize_route_impls", mock_initialize_route_impls) + + client = LlamaStackAsLibraryClient("ci-tests") - # Client should be automatically initialized - assert client.async_client._is_initialized is True assert client.async_client.route_impls is not None - async def test_async_client_auto_initialization(self): + async def test_async_client_auto_initialization(self, monkeypatch): """Test that async client can be initialized and works properly.""" - client = AsyncLlamaStackAsLibraryClient("nvidia") + # Mock the stack construction to avoid dependency issues + mock_impls = {} + mock_route_impls = RouteImpls({}) + + async def mock_construct_stack(config, custom_provider_registry): + return mock_impls + + def mock_initialize_route_impls(impls): + return mock_route_impls + + monkeypatch.setattr("llama_stack.core.library_client.construct_stack", mock_construct_stack) + monkeypatch.setattr("llama_stack.core.library_client.initialize_route_impls", mock_initialize_route_impls) + + client = AsyncLlamaStackAsLibraryClient("ci-tests") # Initialize the client result = await client.initialize() assert result is True - assert client._is_initialized is True assert client.route_impls is not None - def test_initialize_method_backward_compatibility(self): + def test_initialize_method_backward_compatibility(self, monkeypatch): """Test that initialize() method still works for backward compatibility.""" - client = LlamaStackAsLibraryClient("nvidia") + # Mock the stack construction to avoid dependency issues + mock_impls = {} + mock_route_impls = RouteImpls({}) + + async def mock_construct_stack(config, custom_provider_registry): + return mock_impls + + def mock_initialize_route_impls(impls): + return mock_route_impls + + monkeypatch.setattr("llama_stack.core.library_client.construct_stack", mock_construct_stack) + monkeypatch.setattr("llama_stack.core.library_client.initialize_route_impls", mock_initialize_route_impls) + + client = LlamaStackAsLibraryClient("ci-tests") - # initialize() should return None (historical behavior) and not cause errors result = client.initialize() assert result is None - # Multiple calls should be safe result2 = client.initialize() assert result2 is None - async def test_async_initialize_method_idempotent(self): + async def test_async_initialize_method_idempotent(self, monkeypatch): """Test that async initialize() method can be called multiple times safely.""" - client = AsyncLlamaStackAsLibraryClient("nvidia") + mock_impls = {} + mock_route_impls = RouteImpls({}) + + async def mock_construct_stack(config, custom_provider_registry): + return mock_impls + + def mock_initialize_route_impls(impls): + return mock_route_impls + + monkeypatch.setattr("llama_stack.core.library_client.construct_stack", mock_construct_stack) + monkeypatch.setattr("llama_stack.core.library_client.initialize_route_impls", mock_initialize_route_impls) + + client = AsyncLlamaStackAsLibraryClient("ci-tests") - # First initialization result1 = await client.initialize() assert result1 is True - assert client._is_initialized is True - # Second initialization should be safe and return True result2 = await client.initialize() assert result2 is True - assert client._is_initialized is True - def test_route_impls_automatically_set(self): + def test_route_impls_automatically_set(self, monkeypatch): """Test that route_impls is automatically set during construction.""" - # Test sync client - should be auto-initialized - sync_client = LlamaStackAsLibraryClient("nvidia") - assert sync_client.async_client.route_impls is not None + mock_impls = {} + mock_route_impls = RouteImpls({}) - # Test that the async client is marked as initialized - assert sync_client.async_client._is_initialized is True + async def mock_construct_stack(config, custom_provider_registry): + return mock_impls + + def mock_initialize_route_impls(impls): + return mock_route_impls + + monkeypatch.setattr("llama_stack.core.library_client.construct_stack", mock_construct_stack) + monkeypatch.setattr("llama_stack.core.library_client.initialize_route_impls", mock_initialize_route_impls) + + sync_client = LlamaStackAsLibraryClient("ci-tests") + assert sync_client.async_client.route_impls is not None