Improve rate limit detection for integration tests

When GitHub's API rate limits cause asyncio shutdown issues, the test
times out rather than failing with the underlying 429 error. Updated the
detection logic to check for 429 indicators in the captured output when
a timeout occurs.

Also increased the per-test timeout from 15s to 30s to give remote API
calls more breathing room.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Chris Guidry 2025-12-04 11:52:40 -05:00
commit a8bbd745fe
2 changed files with 24 additions and 5 deletions

View file

@ -99,8 +99,8 @@ jobs:
run: uv sync --upgrade
- name: Run integration tests
# use longer per-test timeout than the default 3s
run: uv run pytest tests -m "integration" --timeout=15 --numprocesses auto --maxprocesses 2 --dist worksteal
# use longer per-test timeout for remote API calls (default is 5s)
run: uv run pytest tests -m "integration" --timeout=30 --numprocesses auto --maxprocesses 2 --dist worksteal
env:
FASTMCP_GITHUB_TOKEN: ${{ secrets.FASTMCP_GITHUB_TOKEN }}
FASTMCP_TEST_AUTH_GITHUB_CLIENT_ID: ${{ secrets.FASTMCP_TEST_AUTH_GITHUB_CLIENT_ID }}

View file

@ -3,8 +3,13 @@ import os
import pytest
def _is_rate_limit_error(excinfo) -> bool:
"""Check if an exception indicates a rate limit error from GitHub API."""
def _is_rate_limit_error(excinfo, report=None) -> bool:
"""Check if an exception indicates a rate limit error from GitHub API.
Args:
excinfo: The exception info from pytest
report: Optional test report for additional context (captured output, longrepr)
"""
if excinfo is None:
return False
@ -28,6 +33,20 @@ def _is_rate_limit_error(excinfo) -> bool:
if "429" in exc_str or "rate limit" in exc_str or "too many requests" in exc_str:
return True
# Timeout exceptions may indicate rate limiting when the 429 causes asyncio
# shutdown issues. Check if it's a timeout and look for 429 in the captured output.
if "timeout" in exc_type.lower() or "timeout" in exc_str:
# Check captured output for 429 indicators
if report is not None:
longrepr_str = str(report.longrepr).lower() if report.longrepr else ""
if "429" in longrepr_str or "too many requests" in longrepr_str:
return True
# Check captured stdout/stderr
for section_name, content in getattr(report, "sections", []):
if "429" in content or "too many requests" in content.lower():
return True
return False
@ -43,7 +62,7 @@ def pytest_runtest_makereport(item, call):
and report.failed
and not hasattr(report, "wasxfail")
and item.module.__name__ == "tests.integration_tests.test_github_mcp_remote"
and _is_rate_limit_error(call.excinfo)
and _is_rate_limit_error(call.excinfo, report)
):
report.outcome = "skipped"
report.longrepr = (