-
Notifications
You must be signed in to change notification settings - Fork 2.6k
tests: add lowest dependency tests #1067
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
Changes from all commits
66f2fba
34c53f9
30e4dd2
8642aba
511b4d8
ea5c521
45187fc
5eabbd0
fffd9d9
96e0b55
717c626
bc1c986
994fd80
1226aa3
2d9e348
301aa96
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 |
---|---|---|
|
@@ -29,6 +29,7 @@ jobs: | |
test: | ||
runs-on: ${{ matrix.os }} | ||
timeout-minutes: 10 | ||
continue-on-error: true | ||
strategy: | ||
matrix: | ||
python-version: ["3.10", "3.11", "3.12", "3.13"] | ||
|
@@ -48,8 +49,14 @@ jobs: | |
|
||
- name: Run pytest | ||
run: uv run --frozen --no-sync pytest | ||
continue-on-error: true | ||
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. This shouldn't be set, should it be? 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. I don't think it's needed, but I figured out why it's here: so if other tests on the matrix fail, it will continue to run. |
||
|
||
# This must run last as it modifies the environment! | ||
- name: Run pytest with lowest versions | ||
run: | | ||
uv sync --all-extras --upgrade | ||
uv run --no-sync pytest | ||
env: | ||
UV_RESOLUTION: lowest-direct | ||
readme-snippets: | ||
runs-on: ubuntu-latest | ||
steps: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ dependencies = [ | |
"anyio>=4.5", | ||
"httpx>=0.27", | ||
"httpx-sse>=0.4", | ||
"pydantic>=2.7.2,<3.0.0", | ||
"pydantic>=2.8.0,<3.0.0", | ||
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. 2.7.2 resolves one of the tests wrong, and I found that pushing up 1 minor is not that damaging - pushing it to 2.11+ would be too much right now. 2.8.0 is from July 2024. |
||
"starlette>=0.27", | ||
"python-multipart>=0.0.9", | ||
"sse-starlette>=1.6.1", | ||
|
@@ -36,14 +36,13 @@ dependencies = [ | |
|
||
[project.optional-dependencies] | ||
rich = ["rich>=13.9.4"] | ||
cli = ["typer>=0.12.4", "python-dotenv>=1.0.0"] | ||
cli = ["typer>=0.16.0", "python-dotenv>=1.0.0"] | ||
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. I had to bump typer because click broke it recently. I don't think bumping this package is an issue given that people can usually easily bump this without coding changes on their applications. 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. pydantic or starlette would be more concerning. |
||
ws = ["websockets>=15.0.1"] | ||
|
||
[project.scripts] | ||
mcp = "mcp.cli:app [cli]" | ||
|
||
[tool.uv] | ||
resolution = "lowest-direct" | ||
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. Now, we actually want the resolution to be the "highest" - which is the default. Because we have a test job in the pipeline that sets this resolution. This way we can have the highest resolution on the lockfile, and actually test both highest and lowest-direct. |
||
default-groups = ["dev", "docs"] | ||
required-version = ">=0.7.2" | ||
|
||
|
@@ -58,6 +57,7 @@ dev = [ | |
"pytest-examples>=0.0.14", | ||
"pytest-pretty>=1.2.0", | ||
"inline-snapshot>=0.23.0", | ||
"dirty-equals>=0.9.0", | ||
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. This goes with the same line as 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. This is not necessary - but it did make the test cleaner. I can remove it if requested. |
||
] | ||
docs = [ | ||
"mkdocs>=1.6.1", | ||
|
@@ -124,5 +124,5 @@ filterwarnings = [ | |
# This should be fixed on Uvicorn's side. | ||
"ignore::DeprecationWarning:websockets", | ||
"ignore:websockets.server.WebSocketServerProtocol is deprecated:DeprecationWarning", | ||
"ignore:Returning str or bytes.*:DeprecationWarning:mcp.server.lowlevel" | ||
"ignore:Returning str or bytes.*:DeprecationWarning:mcp.server.lowlevel", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,60 +1,46 @@ | ||
import anyio | ||
import pytest | ||
from pydantic import AnyUrl | ||
|
||
from mcp.server.fastmcp import FastMCP | ||
from mcp.shared.memory import ( | ||
create_connected_server_and_client_session as create_session, | ||
) | ||
|
||
_sleep_time_seconds = 0.01 | ||
_resource_name = "slow://slow_resource" | ||
from mcp.shared.memory import create_connected_server_and_client_session as create_session | ||
|
||
|
||
@pytest.mark.anyio | ||
async def test_messages_are_executed_concurrently(): | ||
server = FastMCP("test") | ||
call_timestamps = [] | ||
event = anyio.Event() | ||
tool_started = anyio.Event() | ||
call_order = [] | ||
|
||
@server.tool("sleep") | ||
async def sleep_tool(): | ||
call_timestamps.append(("tool_start_time", anyio.current_time())) | ||
await anyio.sleep(_sleep_time_seconds) | ||
call_timestamps.append(("tool_end_time", anyio.current_time())) | ||
call_order.append("waiting_for_event") | ||
tool_started.set() | ||
await event.wait() | ||
call_order.append("tool_end") | ||
return "done" | ||
|
||
@server.resource(_resource_name) | ||
async def slow_resource(): | ||
call_timestamps.append(("resource_start_time", anyio.current_time())) | ||
await anyio.sleep(_sleep_time_seconds) | ||
call_timestamps.append(("resource_end_time", anyio.current_time())) | ||
@server.tool("trigger") | ||
async def trigger(): | ||
# Wait for tool to start before setting the event | ||
await tool_started.wait() | ||
call_order.append("trigger_started") | ||
event.set() | ||
call_order.append("trigger_end") | ||
return "slow" | ||
|
||
async with create_session(server._mcp_server) as client_session: | ||
# First tool will wait on event, second will set it | ||
async with anyio.create_task_group() as tg: | ||
for _ in range(10): | ||
tg.start_soon(client_session.call_tool, "sleep") | ||
tg.start_soon(client_session.read_resource, AnyUrl(_resource_name)) | ||
|
||
active_calls = 0 | ||
max_concurrent_calls = 0 | ||
for call_type, _ in sorted(call_timestamps, key=lambda x: x[1]): | ||
if "start" in call_type: | ||
active_calls += 1 | ||
max_concurrent_calls = max(max_concurrent_calls, active_calls) | ||
else: | ||
active_calls -= 1 | ||
print(f"Max concurrent calls: {max_concurrent_calls}") | ||
assert max_concurrent_calls > 1, "No concurrent calls were executed" | ||
|
||
|
||
def main(): | ||
anyio.run(test_messages_are_executed_concurrently) | ||
|
||
|
||
if __name__ == "__main__": | ||
import logging | ||
|
||
logging.basicConfig(level=logging.DEBUG) | ||
|
||
main() | ||
Comment on lines
-49
to
-60
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. This shouldn't have been merged - it seems it was here for debugging purposes. |
||
# Start the tool first (it will wait on event) | ||
tg.start_soon(client_session.call_tool, "sleep") | ||
# Then the trigger tool will set the event to allow the first tool to continue | ||
await client_session.call_tool("trigger") | ||
|
||
# Verify that both ran concurrently | ||
assert call_order == [ | ||
"waiting_for_event", | ||
"trigger_started", | ||
"trigger_end", | ||
"tool_end", | ||
], f"Expected concurrent execution, but got: {call_order}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
|
||
import annotated_types | ||
import pytest | ||
from dirty_equals import IsPartialDict | ||
from pydantic import BaseModel, Field | ||
|
||
from mcp.server.fastmcp.utilities.func_metadata import func_metadata | ||
|
@@ -202,11 +203,8 @@ def func_dict_any() -> dict[str, Any]: | |
return {"a": 1, "b": "hello", "c": [1, 2, 3]} | ||
|
||
meta = func_metadata(func_dict_any) | ||
assert meta.output_schema == { | ||
"additionalProperties": True, | ||
"type": "object", | ||
"title": "func_dict_anyDictOutput", | ||
} | ||
|
||
assert meta.output_schema == IsPartialDict(type="object", title="func_dict_anyDictOutput") | ||
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. This means that the |
||
|
||
# Test dict[str, str] | ||
def func_dict_str() -> dict[str, str]: | ||
|
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.
I put it up because it makes more sense since it's a job configuration, not a step one.