* test(#6630): isolate auxiliary client fixtures * test(#6630): route both aux-title suites through one scoped module stub * test(#6630): drop dead import and record the patch.dict restore constraint * Release exp-v0.52.177: scope auxiliary-title test module stubs, stop cross-file sys.modules leak (#6661) --------- Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
4 files changed
+132
-18
No files matched your search
@@ -11,6 +11,8 @@
|
||||
|
||||
- **Internal: removed a dead `rowIndex` parameter from the settled-scene row projector.** `attachLiveStream`'s `pushRow` helper carried an unused positional index left over from before final-segment eligibility moved to a row-identity `WeakSet`; dropping it is a pure no-op refactor (differential execution confirms byte-identical settled scenes — ordering, dedupe, final-prefix suppression, and sequence assignment all unchanged). Thanks @webtecnica. (#6258)
|
||||
|
||||
- **Internal: auxiliary-title test suites no longer leak a synthetic `agent` module across the process.** `test_title_aux_routing.py` and `test_2235_initial_aux_title.py` installed stub `agent` / `agent.auxiliary_client` modules via `sys.modules.setdefault` at import time and never removed them, so any later test importing `agent.context_compressor` in the same process got the stub and failed (breaking `test_issue4685_post_compression_context_metering.py` by run order). Both suites now route the install through a shared `tests/_aux_client_helpers.py` chokepoint scoped with `unittest.mock.patch.dict`, plus four tests covering restore-by-identity, absent-key cleanup, present-but-`None` preservation, and restore-on-raise. Test-only, no production change. Thanks @rodboev. (#6661, #6630)
|
||||
|
||||
- **Internal: clarified the transcript-virtualization boot comment (opt-in default retained).** The comment above the boot-time `_virtualizeTranscript` apply now records that the #4346 Phase B footer-jitter fix resolved the original scroll-up flicker root cause, while virtualization stays opt-in (default OFF) until battle-tested further. Zero behavior change — the apply predicate, settings-load fallback, checkbox, and server default all remain opt-in/OFF and in agreement. Thanks @webtecnica. (#6318, #6155)
|
||||
|
||||
- **Internal: added regression coverage for the empty-`tool_calls` drop on the metadata-restoration mirror path.** `_api_safe_message_positions()` must apply the same `tool_calls: []` drop as the main API sanitizer (#5737) so metadata restoration never treats a `tool_calls: []` row as a distinct message; a new test pins that empty arrays lose the key while populated tool-call chains (and their tool results) survive untouched. Test-only, no production change. Thanks @webtecnica. (#6803, #6796)
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
"""Shared scaffolding for the auxiliary title-generation test suites.
|
||||
|
||||
``test_title_aux_routing.py`` and ``test_2235_initial_aux_title.py`` both need
|
||||
``agent.auxiliary_client`` importable so ``api.streaming`` can be patched
|
||||
against it. Both used to install synthetic modules at import time with
|
||||
``sys.modules.setdefault`` and never remove them, so whichever ran first left
|
||||
the stub in place for the rest of the process and the next file to import
|
||||
``agent.context_compressor`` got the stub instead of the real module (#6630).
|
||||
|
||||
Scope the install to the test that needs it and let ``mock.patch.dict`` own the
|
||||
restore: it snapshots the mapping and puts it back on exit, so a key that was
|
||||
absent is deleted, a key that was present is restored by identity, and a key
|
||||
holding ``None`` stays ``None``. ``tests/test_4413_seed_provider_models.py``
|
||||
already stubs ``sys.modules`` this way, including the ``None`` case.
|
||||
|
||||
One constraint that comes with it: ``patch.dict`` restores by clearing the
|
||||
mapping and replacing it wholesale, so any module first imported inside the
|
||||
scope is evicted on exit and re-imported later as a new object. That is
|
||||
harmless for these suites, which import everything they touch at collection
|
||||
time, but it is the thing to check before reusing this helper around code
|
||||
that imports lazily.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
from unittest import mock
|
||||
|
||||
|
||||
def auxiliary_client_modules():
|
||||
"""Context manager installing stub ``agent`` / ``agent.auxiliary_client``.
|
||||
|
||||
The stubs are fresh objects owned by this helper, so the ``auxiliary_client``
|
||||
attribute is set on our own module and no pre-existing ``agent`` module is
|
||||
mutated. Restoration is therefore entirely ``patch.dict``'s job.
|
||||
"""
|
||||
agent_stub = types.ModuleType("agent")
|
||||
auxiliary_client_stub = types.ModuleType("agent.auxiliary_client")
|
||||
agent_stub.auxiliary_client = auxiliary_client_stub
|
||||
return mock.patch.dict(
|
||||
sys.modules,
|
||||
{"agent": agent_stub, "agent.auxiliary_client": auxiliary_client_stub},
|
||||
)
|
||||
|
||||
|
||||
def patch_tg_config(config_dict):
|
||||
"""Make ``_get_auxiliary_task_config`` return ``config_dict``."""
|
||||
return mock.patch(
|
||||
"agent.auxiliary_client._get_auxiliary_task_config",
|
||||
return_value=config_dict,
|
||||
create=True,
|
||||
)
|
||||
@@ -10,24 +10,29 @@ Covers:
|
||||
3. Refresh path parity — configured aux routing still applies to the
|
||||
adaptive title refresh path.
|
||||
"""
|
||||
import sys
|
||||
import threading
|
||||
import types
|
||||
import unittest
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
# Stub agent.auxiliary_client so it is importable in the test environment
|
||||
# (the real package lives in hermes-agent, which is not installed here).
|
||||
_agent_stub = types.ModuleType('agent')
|
||||
_aux_stub = types.ModuleType('agent.auxiliary_client')
|
||||
sys.modules.setdefault('agent', _agent_stub)
|
||||
sys.modules.setdefault('agent.auxiliary_client', _aux_stub)
|
||||
_agent_stub.auxiliary_client = _aux_stub
|
||||
import pytest
|
||||
|
||||
from tests._aux_client_helpers import auxiliary_client_modules, patch_tg_config
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _install_auxiliary_client_modules():
|
||||
"""Scope the synthetic hermes-agent modules to each test (#6630).
|
||||
|
||||
This file carried the same unscoped sys.modules.setdefault install as
|
||||
test_title_aux_routing.py, so it leaked the stub the same way.
|
||||
"""
|
||||
with auxiliary_client_modules():
|
||||
yield
|
||||
|
||||
|
||||
def _patch_tg_config(config_dict):
|
||||
"""Return a patch context manager that makes _get_auxiliary_task_config return config_dict."""
|
||||
return patch('agent.auxiliary_client._get_auxiliary_task_config', return_value=config_dict, create=True)
|
||||
return patch_tg_config(config_dict)
|
||||
|
||||
|
||||
def _make_provisional_session(user_text, assistant_text='Here is the answer.'):
|
||||
|
||||
@@ -13,18 +13,73 @@ import types
|
||||
import unittest
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
# Stub agent.auxiliary_client so it is importable in the test environment
|
||||
# (the real package lives in hermes-agent, which is not installed here).
|
||||
_agent_stub = types.ModuleType('agent')
|
||||
_aux_stub = types.ModuleType('agent.auxiliary_client')
|
||||
sys.modules.setdefault('agent', _agent_stub)
|
||||
sys.modules.setdefault('agent.auxiliary_client', _aux_stub)
|
||||
_agent_stub.auxiliary_client = _aux_stub
|
||||
import pytest
|
||||
|
||||
from tests._aux_client_helpers import auxiliary_client_modules, patch_tg_config
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _install_auxiliary_client_modules():
|
||||
"""Scope the synthetic hermes-agent modules to each test (#6630)."""
|
||||
with auxiliary_client_modules():
|
||||
yield
|
||||
|
||||
|
||||
def _patch_tg_config(config_dict):
|
||||
"""Return a patch context manager that makes _get_auxiliary_task_config return config_dict."""
|
||||
return patch('agent.auxiliary_client._get_auxiliary_task_config', return_value=config_dict, create=True)
|
||||
return patch_tg_config(config_dict)
|
||||
|
||||
|
||||
class TestAuxiliaryClientModuleRestore(unittest.TestCase):
|
||||
"""Restoration properties the #6630 fix depends on.
|
||||
|
||||
These drive the helper directly. The fixture boundary itself is proved by
|
||||
the cross-file run in the PR description: running this file before
|
||||
tests/test_issue4685_post_compression_context_metering.py used to fail that
|
||||
file's agent.context_compressor import and no longer does.
|
||||
"""
|
||||
|
||||
def test_prior_modules_restored_by_identity(self):
|
||||
prior_agent = types.ModuleType("agent")
|
||||
prior_aux = types.ModuleType("agent.auxiliary_client")
|
||||
prior_agent.auxiliary_client = prior_aux
|
||||
with patch.dict(sys.modules, {"agent": prior_agent, "agent.auxiliary_client": prior_aux}):
|
||||
with auxiliary_client_modules():
|
||||
self.assertIsNot(sys.modules["agent"], prior_agent)
|
||||
self.assertIsNot(sys.modules["agent.auxiliary_client"], prior_aux)
|
||||
self.assertIs(sys.modules["agent"], prior_agent)
|
||||
self.assertIs(sys.modules["agent.auxiliary_client"], prior_aux)
|
||||
self.assertIs(prior_agent.auxiliary_client, prior_aux)
|
||||
|
||||
def test_absent_keys_removed(self):
|
||||
with patch.dict(sys.modules):
|
||||
sys.modules.pop("agent", None)
|
||||
sys.modules.pop("agent.auxiliary_client", None)
|
||||
with auxiliary_client_modules():
|
||||
self.assertIn("agent", sys.modules)
|
||||
self.assertIn("agent.auxiliary_client", sys.modules)
|
||||
self.assertNotIn("agent", sys.modules)
|
||||
self.assertNotIn("agent.auxiliary_client", sys.modules)
|
||||
|
||||
def test_prior_none_value_preserved(self):
|
||||
"""A present-but-None entry is not an absent entry."""
|
||||
with patch.dict(sys.modules, {"agent": None, "agent.auxiliary_client": None}):
|
||||
with auxiliary_client_modules():
|
||||
self.assertIsNotNone(sys.modules["agent"])
|
||||
self.assertIn("agent", sys.modules)
|
||||
self.assertIsNone(sys.modules["agent"])
|
||||
self.assertIsNone(sys.modules["agent.auxiliary_client"])
|
||||
|
||||
def test_restored_after_exception(self):
|
||||
prior_agent = types.ModuleType("agent")
|
||||
prior_aux = types.ModuleType("agent.auxiliary_client")
|
||||
prior_agent.auxiliary_client = prior_aux
|
||||
with patch.dict(sys.modules, {"agent": prior_agent, "agent.auxiliary_client": prior_aux}):
|
||||
with self.assertRaisesRegex(RuntimeError, "boom"):
|
||||
with auxiliary_client_modules():
|
||||
raise RuntimeError("boom")
|
||||
self.assertIs(sys.modules["agent"], prior_agent)
|
||||
self.assertIs(sys.modules["agent.auxiliary_client"], prior_aux)
|
||||
|
||||
|
||||
class TestAuxTitleConfigured(unittest.TestCase):
|
||||
|
||||
Reference in new issue
Block a user