test(agent): fold the aux create_client() seam tests to two contracts
Two parametrized invariant tests (native client sync/async; None or raising profile falls back to the standard client) replace four near-duplicates, and the hook's kwargs are pinned exactly to the mapping openai.OpenAI would have received. The fixture isolates both provider registries through monkeypatch instead of a hand-rolled snapshot/restore, drops the HERMES_HOME override that tests/conftest.py already provides, and replaces the tuple-truthiness lambda with a plain function. Call-site comment trimmed to the ordering WHY.
This commit is contained in:
kshitijk4poor
@@ -4919,11 +4919,7 @@ def _resolve_api_key_branch(req: _ResolveRequest, pconfig: Any, resolve_creds: C
|
||||
if req.explicit_base_url and provider != "actual":
|
||||
base_url = _to_openai_base_url(req.explicit_base_url.strip().rstrip("/"))
|
||||
final_model = _normalize_resolved_model(req.model or _get_aux_model_for_provider(provider), provider)
|
||||
# Provider-supplied client, consulted before the built-in gemini/OpenAI ladder so an
|
||||
# out-of-tree provider registered with auth_type="api_key" keeps its native transport for
|
||||
# auxiliary tasks — the same hook the main-agent path honors (#112384). ``None`` (the
|
||||
# profile default) falls through to the standard construction, so built-in providers
|
||||
# without a registered profile are untouched.
|
||||
# Consulted before the built-in gemini/OpenAI ladder so a registered native transport wins (#112384).
|
||||
profile_client = _api_key_profile_supplied_client(provider, api_key=api_key, base_url=base_url)
|
||||
if profile_client is not None:
|
||||
logger.debug("resolve_provider_client: %s native client from provider profile (%s)", provider, final_model)
|
||||
|
||||
@@ -5,7 +5,7 @@ so an out-of-tree provider registered with ``auth_type="api_key"`` lost its nati
|
||||
for auxiliary tasks even though the main-agent path
|
||||
(``agent_runtime_helpers._provider_supplied_client``) honors the same hook (#112384).
|
||||
These tests pin the seam through the real resolution entry point: the native client
|
||||
(sync + async), the ``None`` fall-through for ordinary providers, and failure isolation
|
||||
(sync + async), and the fall-through to the standard client for ordinary providers and
|
||||
for a broken plugin.
|
||||
"""
|
||||
|
||||
@@ -56,95 +56,57 @@ def _probe_profile(cls, name: str) -> ProviderProfile:
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def registered(tmp_path, monkeypatch):
|
||||
"""Register provider profiles for one test; restore both registries and the secret scope after.
|
||||
def registered(monkeypatch):
|
||||
"""Register provider profiles for one test on copies of both registries.
|
||||
|
||||
Mirrors the import-time synthesis ``hermes_cli.auth`` performs for plugin ``api_key``
|
||||
profiles, which is what routes them into ``_resolve_api_key_branch`` in the first place.
|
||||
"""
|
||||
import hermes_cli.auth as _auth
|
||||
from agent import secret_scope as _secret_scope
|
||||
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||
|
||||
_providers._discover_providers()
|
||||
providers_snapshot = (
|
||||
dict(_providers._REGISTRY),
|
||||
dict(_providers._ALIASES),
|
||||
_providers._PROVIDER_LIST_CACHE,
|
||||
)
|
||||
auth_snapshot = dict(_auth.PROVIDER_REGISTRY)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
home_token = set_hermes_home_override(str(tmp_path))
|
||||
monkeypatch.setattr(_providers, "_REGISTRY", dict(_providers._REGISTRY))
|
||||
monkeypatch.setattr(_providers, "_ALIASES", dict(_providers._ALIASES))
|
||||
monkeypatch.setattr(_providers, "_PROVIDER_LIST_CACHE", None)
|
||||
monkeypatch.setattr(_auth, "PROVIDER_REGISTRY", dict(_auth.PROVIDER_REGISTRY))
|
||||
scope_token = _secret_scope.set_secret_scope({_PROBE_ENV_VAR: _PROBE_KEY})
|
||||
|
||||
yield (
|
||||
lambda profile: (
|
||||
(
|
||||
_providers.register_provider(profile),
|
||||
_auth._register_plugin_provider(profile),
|
||||
)
|
||||
and profile
|
||||
)
|
||||
)
|
||||
def _register(profile: ProviderProfile) -> None:
|
||||
_providers.register_provider(profile)
|
||||
_auth._register_plugin_provider(profile)
|
||||
|
||||
yield _register
|
||||
_secret_scope.reset_secret_scope(scope_token)
|
||||
reset_hermes_home_override(home_token)
|
||||
_providers._REGISTRY.clear()
|
||||
_providers._REGISTRY.update(providers_snapshot[0])
|
||||
_providers._ALIASES.clear()
|
||||
_providers._ALIASES.update(providers_snapshot[1])
|
||||
_providers._PROVIDER_LIST_CACHE = providers_snapshot[2]
|
||||
_auth.PROVIDER_REGISTRY.clear()
|
||||
_auth.PROVIDER_REGISTRY.update(auth_snapshot)
|
||||
|
||||
|
||||
def test_native_profile_supplies_the_auxiliary_client(registered):
|
||||
@pytest.mark.parametrize(
|
||||
"extra",
|
||||
[{"task": "title_generation"}, {"async_mode": True}],
|
||||
ids=["sync", "async-skips-AsyncOpenAI-rewrap"],
|
||||
)
|
||||
def test_native_profile_supplies_the_auxiliary_client(registered, extra):
|
||||
from agent.auxiliary_client import resolve_provider_client
|
||||
|
||||
registered(_probe_profile(_NativeProfile, "aux-seam-native"))
|
||||
client, model = resolve_provider_client(
|
||||
"aux-seam-native", "probe-model", task="title_generation"
|
||||
)
|
||||
client, model = resolve_provider_client("aux-seam-native", "probe-model", **extra)
|
||||
|
||||
assert isinstance(client, _FakeNativeClient)
|
||||
assert model == "probe-model"
|
||||
# The hook receives the same mapping the branch would have passed to openai.OpenAI.
|
||||
assert client.kwargs["api_key"] == _PROBE_KEY
|
||||
assert client.kwargs["base_url"] == "https://aux-seam-native.invalid"
|
||||
assert client.kwargs == {"api_key": _PROBE_KEY, "base_url": "https://aux-seam-native.invalid"}
|
||||
|
||||
|
||||
def test_native_profile_client_survives_the_async_route(registered):
|
||||
from agent.auxiliary_client import resolve_provider_client
|
||||
|
||||
registered(_probe_profile(_NativeProfile, "aux-seam-native"))
|
||||
client, model = resolve_provider_client(
|
||||
"aux-seam-native", "probe-model", async_mode=True
|
||||
)
|
||||
|
||||
# HERMES_SKIP_ASYNC_WRAP: a native transport is not rebuilt as AsyncOpenAI.
|
||||
assert isinstance(client, _FakeNativeClient)
|
||||
assert model == "probe-model"
|
||||
|
||||
|
||||
def test_profile_returning_none_falls_through_to_the_standard_client(registered):
|
||||
@pytest.mark.parametrize(
|
||||
"profile_cls", [_PassThroughProfile, _ExplodingProfile], ids=["returns-None", "raises"]
|
||||
)
|
||||
def test_profile_without_a_client_falls_back_to_the_standard_client(registered, profile_cls):
|
||||
from openai import OpenAI
|
||||
|
||||
from agent.auxiliary_client import resolve_provider_client
|
||||
|
||||
registered(_probe_profile(_PassThroughProfile, "aux-seam-passthrough"))
|
||||
client, model = resolve_provider_client("aux-seam-passthrough", "probe-model")
|
||||
|
||||
assert isinstance(client, OpenAI)
|
||||
assert model == "probe-model"
|
||||
|
||||
|
||||
def test_a_broken_profile_falls_back_to_the_standard_client(registered):
|
||||
from openai import OpenAI
|
||||
|
||||
from agent.auxiliary_client import resolve_provider_client
|
||||
|
||||
registered(_probe_profile(_ExplodingProfile, "aux-seam-boom"))
|
||||
client, model = resolve_provider_client("aux-seam-boom", "probe-model")
|
||||
registered(_probe_profile(profile_cls, "aux-seam-fallback"))
|
||||
client, model = resolve_provider_client("aux-seam-fallback", "probe-model")
|
||||
|
||||
# A raising plugin can only fail to provide a client, never break auxiliary resolution.
|
||||
assert isinstance(client, OpenAI)
|
||||
|
||||
Reference in New Issue
Block a user