From fd760435c6688a2b6c6b7436dde30e267237baef Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 22 Aug 2026 19:06:45 -0700 Subject: [PATCH] =?UTF-8?q?test(bedrock):=20make=20the=20botocore=20stub?= =?UTF-8?q?=20windows=20airtight=20=E2=80=94=20kills=20the=20vendored-impo?= =?UTF-8?q?rt=20flake?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/agent/test_bedrock_adapter.py | 61 +++++++++++++++++-- tests/agent/test_bedrock_integration.py | 27 ++++++++ tests/hermes_cli/test_bedrock_model_picker.py | 28 +++++++++ tests/run_agent/test_streaming.py | 8 +-- 4 files changed, 115 insertions(+), 9 deletions(-) diff --git a/tests/agent/test_bedrock_adapter.py b/tests/agent/test_bedrock_adapter.py index e6c2c3c3c4..ff6a8b1329 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -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, diff --git a/tests/agent/test_bedrock_integration.py b/tests/agent/test_bedrock_integration.py index c8fc15d3b7..bd9da1cfe0 100644 --- a/tests/agent/test_bedrock_integration.py +++ b/tests/agent/test_bedrock_integration.py @@ -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.""" diff --git a/tests/hermes_cli/test_bedrock_model_picker.py b/tests/hermes_cli/test_bedrock_model_picker.py index 8022ea9ec6..f20d6e9297 100644 --- a/tests/hermes_cli/test_bedrock_model_picker.py +++ b/tests/hermes_cli/test_bedrock_model_picker.py @@ -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) diff --git a/tests/run_agent/test_streaming.py b/tests/run_agent/test_streaming.py index b971f9ec82..da6014320d 100644 --- a/tests/run_agent/test_streaming.py +++ b/tests/run_agent/test_streaming.py @@ -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")