From 97438db0ac99d43c0db04a80430cd2170d8dd741 Mon Sep 17 00:00:00 2001
From: Chris Guidry <chris.g@prefect.io>
Date: Thu, 4 Dec 2025 10:34:43 -0500
Subject: [PATCH] Deflake GitHub MCP remote integration tests (#2543)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .gitignore                                    |  1 +
 tests/integration_tests/conftest.py           | 35 ++++++++++++++++---
 .../test_github_mcp_remote.py                 |  3 +-
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index 887877553..ec7cee57b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,6 +53,7 @@ dmypy.json
 # Local development
 .python-version
 .envrc
+.envrc.private
 .direnv/
 
 # Logs and databases
diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py
index d45e7b292..4be7f6a9e 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -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__),
diff --git a/tests/integration_tests/test_github_mcp_remote.py b/tests/integration_tests/test_github_mcp_remote.py
index 6d8b3e2d5..6f86b3611 100644
--- a/tests/integration_tests/test_github_mcp_remote.py
+++ b/tests/integration_tests/test_github_mcp_remote.py
@@ -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(