mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 13:34:17 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
fe2ec99cc0
commit
97438db0ac
3 changed files with 32 additions and 7 deletions
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -53,6 +53,7 @@ dmypy.json
|
|||
# Local development
|
||||
.python-version
|
||||
.envrc
|
||||
.envrc.private
|
||||
.direnv/
|
||||
|
||||
# Logs and databases
|
||||
|
|
|
|||
|
|
@ -3,9 +3,37 @@ import os
|
|||
import pytest
|
||||
|
||||
|
||||
def _is_rate_limit_error(excinfo) -> bool:
|
||||
"""Check if an exception indicates a rate limit error from GitHub API."""
|
||||
if excinfo is None:
|
||||
return False
|
||||
|
||||
exc = excinfo.value
|
||||
exc_type = excinfo.typename
|
||||
exc_str = str(exc).lower()
|
||||
|
||||
# BrokenResourceError typically indicates connection closed due to rate limit
|
||||
if exc_type == "BrokenResourceError":
|
||||
return True
|
||||
|
||||
# httpx.HTTPStatusError with 429 status
|
||||
if exc_type == "HTTPStatusError":
|
||||
try:
|
||||
if hasattr(exc, "response") and exc.response.status_code == 429:
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Check for rate limit indicators in exception message
|
||||
if "429" in exc_str or "rate limit" in exc_str or "too many requests" in exc_str:
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
@pytest.hookimpl(hookwrapper=True)
|
||||
def pytest_runtest_makereport(item, call):
|
||||
"""Convert BrokenResourceError failures to skips only for GitHub rate limits"""
|
||||
"""Convert rate limit failures to skips for GitHub integration tests."""
|
||||
outcome = yield
|
||||
report = outcome.get_result()
|
||||
|
||||
|
|
@ -14,12 +42,9 @@ def pytest_runtest_makereport(item, call):
|
|||
report.when == "call"
|
||||
and report.failed
|
||||
and not hasattr(report, "wasxfail")
|
||||
and call.excinfo
|
||||
and call.excinfo.typename == "BrokenResourceError"
|
||||
and item.module.__name__ == "tests.integration_tests.test_github_mcp_remote"
|
||||
and _is_rate_limit_error(call.excinfo)
|
||||
):
|
||||
# Only skip if the test is in the GitHub remote test module
|
||||
# This prevents catching unrelated BrokenResourceErrors
|
||||
report.outcome = "skipped"
|
||||
report.longrepr = (
|
||||
os.path.abspath(__file__),
|
||||
|
|
|
|||
|
|
@ -34,7 +34,6 @@ def fixture_streamable_http_client() -> Client[StreamableHttpTransport]:
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.flaky(retries=2, delay=1)
|
||||
class TestGithubMCPRemote:
|
||||
async def test_connect_disconnect(
|
||||
self,
|
||||
|
|
@ -94,7 +93,7 @@ class TestGithubMCPRemote:
|
|||
"""Test calling a non-existing tool"""
|
||||
async with streamable_http_client:
|
||||
assert streamable_http_client.is_connected()
|
||||
with pytest.raises(McpError, match="tool not found"):
|
||||
with pytest.raises(McpError, match=r"unknown tool|tool not found"):
|
||||
await streamable_http_client.call_tool("foo")
|
||||
|
||||
async def test_call_tool_list_commits(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue