Skip to content

Commit 97438db

Browse files
chrisguidryclaude
andauthored
Deflake GitHub MCP remote integration tests (#2543)
The pytest-retry plugin was causing teardown crashes due to a bug with pytest's tmp_path fixture stash. Removing `@pytest.mark.flaky` and instead improving the rate limit detection to properly skip tests on 429 errors. Also fixed a brittle error message regex - GitHub changed their error format from "tool not found" to "unknown tool". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]>
1 parent fe2ec99 commit 97438db

File tree

3 files changed

+32
-7
lines changed

3 files changed

+32
-7
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ dmypy.json
5353
# Local development
5454
.python-version
5555
.envrc
56+
.envrc.private
5657
.direnv/
5758

5859
# Logs and databases

tests/integration_tests/conftest.py

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,37 @@
33
import pytest
44

55

6+
def _is_rate_limit_error(excinfo) -> bool:
7+
"""Check if an exception indicates a rate limit error from GitHub API."""
8+
if excinfo is None:
9+
return False
10+
11+
exc = excinfo.value
12+
exc_type = excinfo.typename
13+
exc_str = str(exc).lower()
14+
15+
# BrokenResourceError typically indicates connection closed due to rate limit
16+
if exc_type == "BrokenResourceError":
17+
return True
18+
19+
# httpx.HTTPStatusError with 429 status
20+
if exc_type == "HTTPStatusError":
21+
try:
22+
if hasattr(exc, "response") and exc.response.status_code == 429:
23+
return True
24+
except Exception:
25+
pass
26+
27+
# Check for rate limit indicators in exception message
28+
if "429" in exc_str or "rate limit" in exc_str or "too many requests" in exc_str:
29+
return True
30+
31+
return False
32+
33+
634
@pytest.hookimpl(hookwrapper=True)
735
def pytest_runtest_makereport(item, call):
8-
"""Convert BrokenResourceError failures to skips only for GitHub rate limits"""
36+
"""Convert rate limit failures to skips for GitHub integration tests."""
937
outcome = yield
1038
report = outcome.get_result()
1139

@@ -14,12 +42,9 @@ def pytest_runtest_makereport(item, call):
1442
report.when == "call"
1543
and report.failed
1644
and not hasattr(report, "wasxfail")
17-
and call.excinfo
18-
and call.excinfo.typename == "BrokenResourceError"
1945
and item.module.__name__ == "tests.integration_tests.test_github_mcp_remote"
46+
and _is_rate_limit_error(call.excinfo)
2047
):
21-
# Only skip if the test is in the GitHub remote test module
22-
# This prevents catching unrelated BrokenResourceErrors
2348
report.outcome = "skipped"
2449
report.longrepr = (
2550
os.path.abspath(__file__),

tests/integration_tests/test_github_mcp_remote.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ def fixture_streamable_http_client() -> Client[StreamableHttpTransport]:
3434
)
3535

3636

37-
@pytest.mark.flaky(retries=2, delay=1)
3837
class TestGithubMCPRemote:
3938
async def test_connect_disconnect(
4039
self,
@@ -94,7 +93,7 @@ async def test_call_tool_ko(
9493
"""Test calling a non-existing tool"""
9594
async with streamable_http_client:
9695
assert streamable_http_client.is_connected()
97-
with pytest.raises(McpError, match="tool not found"):
96+
with pytest.raises(McpError, match=r"unknown tool|tool not found"):
9897
await streamable_http_client.call_tool("foo")
9998

10099
async def test_call_tool_list_commits(

0 commit comments

Comments
 (0)