-
Notifications
You must be signed in to change notification settings - Fork 54
test: upgrade integration harnesses for dual-server execution #706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mcp-v202606
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,7 @@ test = [ | |
| "pytest-aioresponses==0.3.0", | ||
| "pytest-asyncio==1.4.0", | ||
| "pytest-cov==7.1.0", | ||
| "numpy<2.2.0", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment explaining why numpy<2.2.0 is needed? Unexplained upper bounds tend to rot nobody knows when it's safe to lift, and since caps propagate, the moment another test dep requires numpy>=2.2 the resolver can't satisfy both and silently backtracks to old versions. A one-line note would keep this from becoming a mystery pin later. |
||
| "pytest-mock==3.15.1", | ||
| "google-cloud-secret-manager==2.28.0", | ||
| "google-cloud-storage==3.10.1", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,7 @@ | |
| from typing import Generator | ||
|
|
||
| import google | ||
| import pytest_asyncio | ||
| import pytest | ||
| from google.auth import compute_engine | ||
| from google.cloud import secretmanager, storage | ||
|
|
||
|
|
@@ -75,7 +75,8 @@ def get_toolbox_binary_url(toolbox_version: str) -> str: | |
| arch = ( | ||
| "arm64" if os_system == "darwin" and platform.machine() == "arm64" else "amd64" | ||
| ) | ||
| return f"v{toolbox_version}/{os_system}/{arch}/toolbox" | ||
| ext = ".exe" if os_system == "windows" else "" | ||
| return f"{toolbox_version}/{os_system}/{arch}/toolbox{ext}" | ||
|
|
||
|
|
||
| def get_auth_token(client_id: str) -> str: | ||
|
|
@@ -92,17 +93,17 @@ def get_auth_token(client_id: str) -> str: | |
|
|
||
|
|
||
| #### Define Fixtures | ||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def project_id() -> str: | ||
| return get_env_var("GOOGLE_CLOUD_PROJECT") | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def toolbox_version() -> str: | ||
| return get_env_var("TOOLBOX_VERSION") | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def tools_file_path(project_id: str) -> Generator[str]: | ||
| """Provides a temporary file path containing the tools manifest.""" | ||
| tools_manifest = access_secret_version( | ||
|
|
@@ -115,54 +116,103 @@ def tools_file_path(project_id: str) -> Generator[str]: | |
| os.remove(tools_file_path) | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def auth_token1(project_id: str) -> str: | ||
| client_id = access_secret_version( | ||
| project_id=project_id, secret_id="sdk_testing_client1" | ||
| ) | ||
| return get_auth_token(client_id) | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def auth_token2(project_id: str) -> str: | ||
| client_id = access_secret_version( | ||
| project_id=project_id, secret_id="sdk_testing_client2" | ||
| ) | ||
| return get_auth_token(client_id) | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture(scope="session") | ||
| @pytest.fixture(scope="session") | ||
| def toolbox_server(toolbox_version: str, tools_file_path: str) -> Generator[None]: | ||
| """Starts the toolbox server as a subprocess.""" | ||
| print("Downloading toolbox binary from gcs bucket...") | ||
| source_blob_name = get_toolbox_binary_url(toolbox_version) | ||
| download_blob("mcp-toolbox-for-databases", source_blob_name, "toolbox") | ||
| bucket_name = ( | ||
| "mcp-toolbox-for-databases-dev" | ||
| if toolbox_version in ("main", "mcp-v202606") | ||
| else "mcp-toolbox-for-databases" | ||
| ) | ||
| download_blob(bucket_name, source_blob_name, "toolbox") | ||
| print("Toolbox binary downloaded successfully.") | ||
| try: | ||
| print("Opening toolbox server process...") | ||
| # Make toolbox executable | ||
| os.chmod("toolbox", 0o700) | ||
| # Run toolbox binary | ||
| toolbox_server = subprocess.Popen( | ||
| ["./toolbox", "--tools-file", tools_file_path] | ||
| toolbox_server_1 = subprocess.Popen( | ||
| ["./toolbox", "--port", "5000", "--tools-file", tools_file_path] | ||
| ) | ||
| toolbox_server_2 = subprocess.Popen( | ||
| [ | ||
| "./toolbox", | ||
| "--port", | ||
| "5001", | ||
| "--tools-file", | ||
| tools_file_path, | ||
| "--enable-draft-specs", | ||
| ] | ||
| ) | ||
|
|
||
| # Wait for server to start | ||
| # Retry logic with a timeout | ||
| for _ in range(5): # retries | ||
| time.sleep(2) | ||
| print("Checking if toolbox is successfully started...") | ||
| if toolbox_server.poll() is None: | ||
| print("Toolbox server started successfully.") | ||
| print("Checking if both toolbox servers are successfully started...") | ||
| if toolbox_server_1.poll() is None and toolbox_server_2.poll() is None: | ||
| print("Toolbox servers started successfully.") | ||
| break | ||
| else: | ||
| raise RuntimeError("Toolbox server failed to start after 5 retries.") | ||
| raise RuntimeError("Toolbox servers failed to start after 5 retries.") | ||
| except subprocess.CalledProcessError as e: | ||
| print(e.stderr.decode("utf-8")) | ||
| print(e.stdout.decode("utf-8")) | ||
| raise RuntimeError(f"{e}\n\n{e.stderr.decode('utf-8')}") from e | ||
| yield | ||
|
|
||
| # Clean up toolbox server | ||
| toolbox_server.terminate() | ||
| toolbox_server.wait(timeout=5) | ||
| toolbox_server_1.terminate() | ||
| toolbox_server_2.terminate() | ||
| toolbox_server_1.wait(timeout=5) | ||
| toolbox_server_2.wait(timeout=5) | ||
|
|
||
|
|
||
| @pytest.fixture( | ||
| params=["http://localhost:5000", "http://localhost:5001"], scope="session" | ||
| ) | ||
| def toolbox_server_url(request) -> str: | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: For core, langchain, llamaindex |
||
| def patch_toolbox_client_url(toolbox_server_url): | ||
| from toolbox_core.client import ToolboxClient | ||
| from toolbox_core.sync_client import ToolboxSyncClient | ||
|
|
||
| original_init = ToolboxClient.__init__ | ||
| original_sync_init = ToolboxSyncClient.__init__ | ||
|
|
||
| def new_init(self, url="http://localhost:5000", *args, **kwargs): | ||
| if url == "http://localhost:5000": | ||
| url = toolbox_server_url | ||
| original_init(self, url, *args, **kwargs) | ||
|
|
||
| def new_sync_init(self, url="http://localhost:5000", *args, **kwargs): | ||
| if url == "http://localhost:5000": | ||
| url = toolbox_server_url | ||
| original_sync_init(self, url, *args, **kwargs) | ||
|
|
||
| from unittest.mock import patch | ||
|
|
||
| with patch.object(ToolboxClient, "__init__", new_init, create=True): | ||
| with patch.object(ToolboxSyncClient, "__init__", new_sync_init, create=True): | ||
| yield | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work for windows? Since we're adding windows support to the get_toolbox_binary_url method, should these commands be fixed everywhere?
Currently I am also okay with removing the windows support since our CI runs on linux.