test(bedrock): make the botocore stub windows airtight — kills the vendored-import flake
CI flake mechanism (PR #92617 red, reproduced standalone): tests plant fake botocore modules via patch.dict; when the REAL botocore.exceptions is first imported in an interpreter state where a fake parent is (or was) installed, its 'from botocore.vendored import requests' resolves against a module with no __path__ and every exception test in the worker dies with "No module named 'botocore.vendored'" — ordering-dependent, so green locally, red in CI workers. Defenses (both, in depth): - test_bedrock_adapter.py pre-imports the real botocore.exceptions at module scope, before any test can stub sys.modules — later imports are cache hits that can never re-execute the vendored import under a poisoned parent. Proven standalone: fake-parent repro fails without the pre-import, succeeds with it. - autouse _boto_sys_modules_hygiene fixtures in all three files that plant fake boto* modules (adapter, integration, model-picker): snapshot every boto* sys.modules entry before each test, evict+restore after — no stub window can leak state into a later test regardless of worker ordering. - importorskip targets botocore.exceptions (the module the tests actually need) instead of bare botocore, so a torn install skips instead of erroring. 148/148 across the four affected suites.
This commit is contained in:
@@ -16,6 +16,57 @@ from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# botocore import hygiene (anti-flake, Aug 2026)
|
||||
#
|
||||
# Several tests in this file plant fake ``botocore`` modules in sys.modules
|
||||
# (to run without the real package / without touching the AWS credential
|
||||
# chain). The real package's ``botocore.exceptions`` lazily executes
|
||||
# ``from botocore.vendored import requests`` on FIRST import — if that first
|
||||
# import happens while a fake parent is (or was) installed, the chain
|
||||
# resolves against a module with no real __path__ and the whole file's
|
||||
# exception tests die with ``No module named 'botocore.vendored'`` — but
|
||||
# only in interpreter states where nothing imported it earlier (the exact
|
||||
# CI-vs-local flake on PR #92617).
|
||||
#
|
||||
# Two defenses, both required:
|
||||
# 1. Import the real exception types HERE, at module scope, before any
|
||||
# test can stub sys.modules. Once cached, later ``from
|
||||
# botocore.exceptions import X`` is a dict hit and can never
|
||||
# re-execute the vendored import under a poisoned parent.
|
||||
# 2. An autouse fixture snapshots every boto* sys.modules entry before
|
||||
# each test and restores it after, so no stub window can leak state
|
||||
# into a later test regardless of ordering.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
try: # pragma: no cover - exercised implicitly by every exception test
|
||||
from botocore.exceptions import ( # noqa: F401
|
||||
ClientError as _RealClientError,
|
||||
ConnectionClosedError as _RealConnectionClosedError,
|
||||
)
|
||||
except Exception: # botocore genuinely not installed / torn — tests skip
|
||||
_RealClientError = _RealConnectionClosedError = None
|
||||
|
||||
_BOTO_PREFIXES = ("botocore", "boto3")
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _boto_sys_modules_hygiene():
|
||||
"""Restore every boto* sys.modules entry after each test (see above)."""
|
||||
import sys as _sys
|
||||
|
||||
saved = {
|
||||
name: mod
|
||||
for name, mod in _sys.modules.items()
|
||||
if name.split(".", 1)[0] in _BOTO_PREFIXES
|
||||
}
|
||||
yield
|
||||
for name in [
|
||||
n for n in _sys.modules if n.split(".", 1)[0] in _BOTO_PREFIXES
|
||||
]:
|
||||
_sys.modules.pop(name, None)
|
||||
_sys.modules.update(saved)
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _mock_botocore_session(*, return_value=None, side_effect=None):
|
||||
@@ -923,7 +974,7 @@ class TestIsStaleConnectionError:
|
||||
|
||||
|
||||
def test_detects_botocore_read_timeout(self):
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock exception tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from agent.bedrock_adapter import is_stale_connection_error
|
||||
from botocore.exceptions import ReadTimeoutError
|
||||
exc = ReadTimeoutError(endpoint_url="https://bedrock.example")
|
||||
@@ -962,7 +1013,7 @@ class TestCallConverseInvalidatesOnStaleError:
|
||||
|
||||
|
||||
def test_converse_stream_evicts_client_on_stale_error(self):
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock exception tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from agent.bedrock_adapter import (
|
||||
_bedrock_runtime_client_cache,
|
||||
call_converse_stream,
|
||||
@@ -988,7 +1039,7 @@ class TestCallConverseInvalidatesOnStaleError:
|
||||
|
||||
def test_converse_does_not_evict_on_non_stale_error(self):
|
||||
"""Non-stale errors (e.g. ValidationException) leave the client cache alone."""
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock exception tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from agent.bedrock_adapter import (
|
||||
_bedrock_runtime_client_cache,
|
||||
call_converse,
|
||||
@@ -1040,7 +1091,7 @@ class TestStreamingAccessDeniedDetection:
|
||||
)
|
||||
|
||||
def test_matches_access_denied_client_error(self):
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock exception tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from agent.bedrock_adapter import is_streaming_access_denied_error
|
||||
assert is_streaming_access_denied_error(self._denied_client_error()) is True
|
||||
|
||||
@@ -1060,7 +1111,7 @@ class TestCallConverseStreamIamFallback:
|
||||
streaming action — InvokeModel-only policies keep working."""
|
||||
|
||||
def test_falls_back_to_converse_on_streaming_denial(self):
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock exception tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from agent.bedrock_adapter import (
|
||||
_bedrock_runtime_client_cache,
|
||||
call_converse_stream,
|
||||
|
||||
@@ -14,6 +14,33 @@ from unittest.mock import MagicMock, patch
|
||||
import pytest
|
||||
|
||||
|
||||
_BOTO_PREFIXES = ("botocore", "boto3")
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _boto_sys_modules_hygiene():
|
||||
"""Snapshot/restore boto* sys.modules around every test.
|
||||
|
||||
Tests here plant fake botocore/boto3 modules; a fake that leaks (or a
|
||||
real submodule first-imported inside a stub window) poisons later
|
||||
imports of the real ``botocore.exceptions`` with
|
||||
``No module named 'botocore.vendored'`` (PR #92617 CI flake). This
|
||||
fixture makes stub windows airtight regardless of test ordering.
|
||||
"""
|
||||
import sys as _sys
|
||||
|
||||
saved = {
|
||||
name: mod
|
||||
for name, mod in _sys.modules.items()
|
||||
if name.split(".", 1)[0] in _BOTO_PREFIXES
|
||||
}
|
||||
yield
|
||||
for name in [n for n in _sys.modules if n.split(".", 1)[0] in _BOTO_PREFIXES]:
|
||||
_sys.modules.pop(name, None)
|
||||
_sys.modules.update(saved)
|
||||
|
||||
|
||||
|
||||
class TestProviderRegistry:
|
||||
"""Verify Bedrock is registered in PROVIDER_REGISTRY."""
|
||||
|
||||
|
||||
@@ -20,6 +20,34 @@ from contextlib import contextmanager
|
||||
from types import ModuleType
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
_BOTO_PREFIXES = ("botocore", "boto3")
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _boto_sys_modules_hygiene():
|
||||
"""Snapshot/restore boto* sys.modules around every test.
|
||||
|
||||
Tests here plant fake botocore/boto3 modules; a fake that leaks (or a
|
||||
real submodule first-imported inside a stub window) poisons later
|
||||
imports of the real ``botocore.exceptions`` with
|
||||
``No module named 'botocore.vendored'`` (PR #92617 CI flake). This
|
||||
fixture makes stub windows airtight regardless of test ordering.
|
||||
"""
|
||||
import sys as _sys
|
||||
|
||||
saved = {
|
||||
name: mod
|
||||
for name, mod in _sys.modules.items()
|
||||
if name.split(".", 1)[0] in _BOTO_PREFIXES
|
||||
}
|
||||
yield
|
||||
for name in [n for n in _sys.modules if n.split(".", 1)[0] in _BOTO_PREFIXES]:
|
||||
_sys.modules.pop(name, None)
|
||||
_sys.modules.update(saved)
|
||||
|
||||
|
||||
from agent.bedrock_adapter import BEDROCK_OPENAI_RESPONSES_MODEL_IDS
|
||||
|
||||
_MANTLE_MODELS = list(BEDROCK_OPENAI_RESPONSES_MODEL_IDS)
|
||||
|
||||
@@ -1490,7 +1490,7 @@ class TestBedrockIamStreamingFallback:
|
||||
return agent
|
||||
|
||||
def test_iam_denial_falls_back_inline_and_disables_streaming(self):
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
from botocore.exceptions import ClientError
|
||||
|
||||
agent = self._make_bedrock_agent()
|
||||
@@ -1614,7 +1614,7 @@ class TestBedrockStreamLivenessWatchdog:
|
||||
"""A Bedrock stream that opens then stops yielding events trips the
|
||||
watchdog: it bumps the cross-turn stale streak and raises TimeoutError
|
||||
instead of hanging forever."""
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
import threading as _t
|
||||
|
||||
# Tiny stale timeout so the watchdog trips quickly; give-up threshold
|
||||
@@ -1647,7 +1647,7 @@ class TestBedrockStreamLivenessWatchdog:
|
||||
def test_pre_elevated_streak_aborts_before_streaming(self, monkeypatch):
|
||||
"""A streak already past the give-up threshold aborts at entry with
|
||||
RuntimeError — Bedrock never even opens a stream (cross-turn breaker)."""
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
|
||||
monkeypatch.setenv("HERMES_STREAM_STALE_GIVEUP", "5")
|
||||
|
||||
@@ -1669,7 +1669,7 @@ class TestBedrockStreamLivenessWatchdog:
|
||||
def test_successful_stream_resets_streak(self, monkeypatch):
|
||||
"""A Bedrock stream that completes normally clears any prior stale
|
||||
streak so a recovered provider doesn't carry it into later turns."""
|
||||
pytest.importorskip("botocore", reason="botocore required for Bedrock tests")
|
||||
pytest.importorskip("botocore.exceptions", reason="botocore (with working exceptions module) required")
|
||||
|
||||
monkeypatch.setenv("HERMES_STREAM_STALE_TIMEOUT", "60")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user