-
Notifications
You must be signed in to change notification settings - Fork 2.4k
#552 #707
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
Merged
Merged
#552 #707
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
9a797c2
Add Windows-specific test for process creation to prevent hanging
DanielAvdar 1fba897
Merge branch 'modelcontextprotocol:main' into #552
DanielAvdar 2f25562
Merge branch 'main' into #552
DanielAvdar 0ec5bc0
Merge branch 'main' into #552
DanielAvdar 289281b
Merge branch 'main' into #552
DanielAvdar 7a84ecb
Fix Windows hang in test by properly using stdio_client
DanielAvdar 90f2cc1
Update timeout in test_552_windows_hang.py and handle additional exce…
DanielAvdar 8c6fc2a
Update error assertions in test_552_windows_hang to check for expecte…
DanielAvdar bd6cc71
Add version check for asyncio.timeout in Windows-specific test
DanielAvdar fb09415
Merge branch 'main' into #552
DanielAvdar 8ba9b5e
Merge branch 'main' into #552
DanielAvdar 947a37a
Refactor test to use parameterized arguments.
DanielAvdar a5b53cf
Merge branch 'main' into #552
DanielAvdar e64a939
Remove Windows-specific skip condition from test_552_windows_hang.py
DanielAvdar 94f1f4b
Apply review feedback
felixweinberger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import asyncio | ||
import sys | ||
|
||
import pytest | ||
|
||
from mcp import ClientSession, StdioServerParameters | ||
from mcp.client.stdio import stdio_client | ||
|
||
# @pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific test") | ||
@pytest.mark.skipif(sys.version_info < (3, 11), reason="asyncio.timeout in 3.11+") | ||
felixweinberger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@pytest.mark.parametrize( | ||
"args", | ||
[ | ||
["/C", "echo", '{"jsonrpc": "2.0", "id": 1, "result": null}'], | ||
["dfghfgh"], | ||
["/C", "echo"], | ||
], | ||
) | ||
@pytest.mark.anyio | ||
async def test_windows_process_creation(args): | ||
""" | ||
Test that directly tests the process creation function that was fixed in issue #552. | ||
This simpler test verifies that Windows process creation works without hanging. | ||
""" | ||
# Use a simple command that should complete quickly on Windows | ||
params = StdioServerParameters( | ||
command="cmd", | ||
# Echo a valid JSON-RPC response message that will be parsed correctly | ||
args=args, | ||
) | ||
|
||
# Directly test the fixed function that was causing the hanging issue | ||
try: | ||
# Set a timeout to prevent hanging | ||
async with asyncio.timeout(5): | ||
felixweinberger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Test the actual process creation function that was fixed | ||
async with stdio_client(params) as (read, write): | ||
print("inside client") | ||
async with ClientSession(read, write) as c: | ||
print("inside ClientSession") | ||
await c.initialize() | ||
|
||
except asyncio.TimeoutError: | ||
pytest.xfail("Process creation timed out, indicating a hang issue") | ||
except ProcessLookupError: | ||
pytest.xfail("Process creation failed with ProcessLookupError") | ||
except Exception as e: | ||
assert "ExceptionGroup" in repr(e), f"Unexpected error: {e}" | ||
assert "ProcessLookupError" in repr(e), f"Unexpected error: {e}" | ||
pytest.xfail(f"Expected error: {e}") | ||
felixweinberger marked this conversation as resolved.
Show resolved
Hide resolved
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.