mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(tests): eliminate flaky/broken tests — shadow sys.path inserts, unmocked network in compressor tests, stale-SDK feishu pin guard, quadratic redact regexes
- Remove tests/-shadowing sys.path.insert(dirname/'..') from 11 test files: it prepended the tests/ dir itself to sys.path, so 'import agent' / 'import hermes_cli' resolved to the test packages and collection died with ModuleNotFoundError depending on import order (2 files failed in every full-suite run; 9 more were latent). - Patch call_llm in 5 context-compressor tests that called compress() unmocked: each burned ~50s attempting live LLM traffic through the relay before falling back (572s file — the slowest in the suite, and flaky under the 300s per-file timeout). File now runs in ~5s. - agent/redact.py: fix two catastrophically-backtracking regexes hit by the compressor's redaction pass on large payloads — _STRICT_URL_USERINFO_RE anchors on the mandatory '//' (optional-scheme prefix backtracked O(n^2): ~55s on a 320KB payload, now sub-ms; output-equivalence fuzz-verified on 20k random strings), and the _CFG_DOTTED_RE/_CFG_ANCHORED_RE subs gain an exact linear keyword pre-gate so secret-free text skips the quadratic pattern entirely. - tests/gateway/test_feishu.py: version-guard the extra_ua_tags SDK signature check; the repo pins lark-oapi==1.6.8 but stale local installs (1.5.3) fail the assertion — skip below the pin. - tests/tools/test_managed_browserbase_and_modal.py: stub agent.redact + agent.credential_persistence in the fake agent package (empty __path__ blocks all real agent.* imports added since the fake was written). - tests/gateway/test_startup_restart_race.py: raise wait_for timeouts 2s -> 30s; 2s wall-clock on a loaded 40-worker box flaked in the baseline run (passes instantly when the box is quiet).
This commit is contained in:
parent
1f70ba6bca
commit
28524adb0e
16 changed files with 73 additions and 44 deletions
|
|
@ -140,6 +140,11 @@ _ENV_ASSIGN_RE = re.compile(
|
|||
# The colon-form URL guard (skip when ``://`` present) lives at the call site.
|
||||
_SECRET_CFG_NAMES = r"(?:api[ _.\-]?key|token|secret|passwd|password|credential|auth)"
|
||||
_CFG_VALUE = r"(['\"]?)([^\s&]+?)\2(?=[\s&]|$)"
|
||||
# Linear pre-gate for the _CFG_*_RE subs below: a text with no secret keyword
|
||||
# can never match either pattern, so the (potentially backtrack-heavy) subs
|
||||
# are skipped entirely for such text. See the call site in
|
||||
# redact_sensitive_text().
|
||||
_CFG_SECRET_WORD_RE = re.compile(_SECRET_CFG_NAMES, re.IGNORECASE)
|
||||
|
||||
# Programmatic env lookups (``os.getenv(...)``, ``os.environ[...]``,
|
||||
# ``os.environ.get(...)``, ``process.env.X``, ``$ENV{X}``) reference variable
|
||||
|
|
@ -377,8 +382,17 @@ _STRICT_URL_PARAM_RE = re.compile(
|
|||
# Match userinfo in both absolute (``scheme://user:pass@host``) and
|
||||
# network-path (``//user:pass@host``) references. The authority boundary stops
|
||||
# at path/query/fragment delimiters so an ``@`` elsewhere in a URL is ignored.
|
||||
#
|
||||
# Anchored on the mandatory ``//`` rather than an optional scheme prefix: the
|
||||
# scheme sits outside the match either way (replacement callbacks re-emit
|
||||
# group(1), so ``https:`` stays untouched in the surrounding text), and the
|
||||
# old optional-scheme prefix ``(?:[A-Za-z][A-Za-z0-9+.-]*:)?`` backtracked
|
||||
# catastrophically (O(n²)) on long unbroken alphanumeric runs — a 320KB
|
||||
# synthetic compaction payload spent ~55s inside this pattern per sub() call.
|
||||
# Output-equivalence to the old pattern was fuzz-verified (20k random strings
|
||||
# plus targeted URL forms).
|
||||
_STRICT_URL_USERINFO_RE = re.compile(
|
||||
r"((?:[A-Za-z][A-Za-z0-9+.-]*:)?//)([^/\s?#@]+)@"
|
||||
r"(//)([^/\s?#@]+)@"
|
||||
)
|
||||
|
||||
# HTTP access logs often use a relative request target rather than a full URL:
|
||||
|
|
@ -706,7 +720,15 @@ def redact_sensitive_text(
|
|||
# web-URL query params are intentionally passed through (see note
|
||||
# near the bottom of this function); _DB_CONNSTR_RE still guards
|
||||
# connection-string passwords.
|
||||
if "://" not in text:
|
||||
#
|
||||
# Extra gate: every _CFG_*_RE match requires a secret keyword in
|
||||
# the key, so a text without any secret keyword cannot match —
|
||||
# skipping is exact. This matters because _CFG_DOTTED_RE
|
||||
# backtracks quadratically on long unbroken [A-Za-z0-9_.\-] runs
|
||||
# (e.g. base64/hex blobs in compaction payloads); the linear
|
||||
# keyword scan prevents that pathological path on secret-free
|
||||
# text.
|
||||
if "://" not in text and _CFG_SECRET_WORD_RE.search(text):
|
||||
text = _CFG_DOTTED_RE.sub(_redact_env, text)
|
||||
text = _CFG_ANCHORED_RE.sub(_redact_env, text)
|
||||
|
||||
|
|
|
|||
|
|
@ -174,7 +174,8 @@ class TestCompress:
|
|||
|
||||
def test_too_few_messages_returns_unchanged(self, compressor):
|
||||
msgs = self._make_messages(4) # protect_first=2 + protect_last=2 + 1 = 5 needed
|
||||
result = compressor.compress(msgs)
|
||||
with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
|
||||
result = compressor.compress(msgs)
|
||||
assert result == msgs
|
||||
|
||||
def test_truncation_fallback_no_client(self, compressor):
|
||||
|
|
@ -380,15 +381,19 @@ class TestCompress:
|
|||
def test_compression_increments_count(self, compressor):
|
||||
msgs = self._make_messages(10)
|
||||
# Default config (abort_on_summary_failure=False) — fallback path
|
||||
# increments the count even on summary failure.
|
||||
compressor.compress(msgs)
|
||||
assert compressor.compression_count == 1
|
||||
compressor.compress(msgs)
|
||||
assert compressor.compression_count == 2
|
||||
# increments the count even on summary failure. Patch call_llm so the
|
||||
# summary attempt fails fast instead of attempting live network I/O
|
||||
# (unpatched, each compress() call burned ~50s in connect/retry).
|
||||
with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
|
||||
compressor.compress(msgs)
|
||||
assert compressor.compression_count == 1
|
||||
compressor.compress(msgs)
|
||||
assert compressor.compression_count == 2
|
||||
|
||||
def test_protects_first_and_last(self, compressor):
|
||||
msgs = self._make_messages(10)
|
||||
result = compressor.compress(msgs)
|
||||
with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
|
||||
result = compressor.compress(msgs)
|
||||
# First 2 messages should be preserved (protect_first_n=2)
|
||||
# Last 2 messages should be preserved (protect_last_n=2)
|
||||
assert result[-1]["content"] == msgs[-1]["content"]
|
||||
|
|
@ -607,7 +612,8 @@ class TestGenerateSummaryNoneContent:
|
|||
{"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"}
|
||||
for i in range(10)
|
||||
]
|
||||
result = c.compress(msgs)
|
||||
with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
|
||||
result = c.compress(msgs)
|
||||
assert len(result) < len(msgs)
|
||||
|
||||
|
||||
|
|
@ -2682,7 +2688,8 @@ class TestSummaryTargetRatio:
|
|||
+ [{"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"}
|
||||
for i in range(8)]
|
||||
)
|
||||
result = c.compress(msgs)
|
||||
with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
|
||||
result = c.compress(msgs)
|
||||
# System prompt (msg[0]) survives as head
|
||||
assert result[0]["role"] == "system"
|
||||
assert result[0]["content"].startswith("System prompt")
|
||||
|
|
|
|||
|
|
@ -4,13 +4,10 @@ get_model_context_length.
|
|||
All tests use synthetic inputs — no filesystem or live server required.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import os
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
|
|||
|
|
@ -9,11 +9,8 @@ No filesystem or network I/O required — we use tmp_path to create real
|
|||
CA bundle stand-in files and monkeypatch env vars.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
import pytest
|
||||
|
||||
|
|
|
|||
|
|
@ -7,13 +7,10 @@ Covers:
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ import sys
|
|||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
def _make_cli(env_overrides=None, config_overrides=None, **kwargs):
|
||||
|
|
|
|||
|
|
@ -1,9 +1,7 @@
|
|||
import importlib
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
_cli_mod = None
|
||||
|
|
|
|||
|
|
@ -5,14 +5,11 @@ Verifies that resuming a session shows a compact recap of the previous
|
|||
conversation with correct formatting, truncation, and config behavior.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from io import StringIO
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import cli as cli_mod
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
def _make_cli(config_overrides=None, env_overrides=None, **kwargs):
|
||||
|
|
|
|||
|
|
@ -5,12 +5,10 @@ persistent lines to scrollback on tool.completed, restoring the stacked
|
|||
tool history that was lost when the TUI switched to a single-line spinner.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import importlib
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
# Module-level reference to the cli module (set by _make_cli on first call)
|
||||
_cli_mod = None
|
||||
|
|
|
|||
|
|
@ -176,9 +176,26 @@ class TestFeishuMessageNormalization(unittest.TestCase):
|
|||
class TestFeishuAdapterMessaging(unittest.TestCase):
|
||||
@unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed")
|
||||
def test_websocket_sdk_accepts_channel_ua_tag(self):
|
||||
"""The shipped SDK must support the Channel signaling argument."""
|
||||
"""The shipped SDK must support the Channel signaling argument.
|
||||
|
||||
Guarded on the pinned version: the repo pins lark-oapi==1.6.8 (the
|
||||
first release with ``extra_ua_tags``). Dev machines can carry an
|
||||
older lazy-installed lark-oapi that predates the argument — that is
|
||||
an environment artifact, not a product regression, so skip rather
|
||||
than fail there. Environments installing the pin (the feishu extra)
|
||||
still exercise the real assertion.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
from importlib.metadata import version as _pkg_version
|
||||
|
||||
installed = tuple(int(p) for p in _pkg_version("lark-oapi").split(".")[:3] if p.isdigit())
|
||||
if installed < (1, 6, 8):
|
||||
self.skipTest(
|
||||
f"lark-oapi {_pkg_version('lark-oapi')} predates extra_ua_tags; "
|
||||
"repo pin is 1.6.8 — stale local install"
|
||||
)
|
||||
|
||||
from lark_oapi.ws import Client as FeishuWSClient
|
||||
|
||||
signature = inspect.signature(FeishuWSClient)
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ async def test_startup_aborts_when_restart_requested_before_start(tmp_path, monk
|
|||
runner.request_restart(detached=False, via_service=True)
|
||||
runner._create_adapter = MagicMock()
|
||||
|
||||
result = await asyncio.wait_for(runner.start(), timeout=2)
|
||||
result = await asyncio.wait_for(runner.start(), timeout=30)
|
||||
|
||||
assert result is True
|
||||
runner._create_adapter.assert_not_called()
|
||||
|
|
@ -167,7 +167,7 @@ async def test_startup_aborts_when_restart_begins_during_platform_connect(tmp_pa
|
|||
telegram.disconnect = disconnect_and_release
|
||||
runner._create_adapter = MagicMock(side_effect=[telegram, slack])
|
||||
|
||||
result = await asyncio.wait_for(runner.start(), timeout=2)
|
||||
result = await asyncio.wait_for(runner.start(), timeout=30)
|
||||
|
||||
assert result is True
|
||||
assert telegram.disconnected is True
|
||||
|
|
@ -202,7 +202,7 @@ async def test_startup_abort_waits_for_existing_stop_task(tmp_path):
|
|||
|
||||
result = await asyncio.wait_for(
|
||||
runner._abort_startup_if_shutdown_requested(adapter, Platform.TELEGRAM),
|
||||
timeout=2,
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
assert result is True
|
||||
|
|
@ -227,7 +227,7 @@ async def test_startup_aborts_after_registered_adapter_restart(tmp_path, monkeyp
|
|||
|
||||
runner._update_platform_runtime_status = MagicMock(side_effect=update_platform_runtime_status)
|
||||
|
||||
result = await asyncio.wait_for(runner.start(), timeout=2)
|
||||
result = await asyncio.wait_for(runner.start(), timeout=30)
|
||||
|
||||
assert result is True
|
||||
assert telegram.connected is True
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ fixture because `ssl.create_default_context(cafile=...)` parses the
|
|||
bundle and refuses stubs.
|
||||
"""
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import ssl
|
||||
import subprocess
|
||||
|
|
@ -19,7 +18,6 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
from hermes_cli.auth import _default_verify, _resolve_verify
|
||||
|
||||
|
|
|
|||
|
|
@ -9,12 +9,9 @@ and ``speak_text`` tolerates empty input without touching the provider
|
|||
stack.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
class TestPublicAPI:
|
||||
|
|
|
|||
|
|
@ -23,11 +23,8 @@ These are different and the old code conflated them; the fix keeps them
|
|||
separate.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import os
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -12,10 +12,7 @@ is always within the visible window. These tests exercise that logic in
|
|||
isolation without requiring a real TTY.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import os
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -101,6 +101,17 @@ def _install_fake_tools_package():
|
|||
sys.modules["agent.auxiliary_client"] = types.SimpleNamespace(
|
||||
call_llm=lambda *args, **kwargs: "",
|
||||
)
|
||||
# The fake `agent` package has an empty __path__, so every real
|
||||
# agent.* submodule that production code imports needs an explicit
|
||||
# stand-in here. tools.browser_tool imports redact_cdp_url;
|
||||
# hermes_cli.auth (imported transitively by nous_account /
|
||||
# tool_backend_helpers) imports sanitize_borrowed_credential_payload.
|
||||
sys.modules["agent.redact"] = types.SimpleNamespace(
|
||||
redact_cdp_url=lambda value: str(value),
|
||||
)
|
||||
sys.modules["agent.credential_persistence"] = types.SimpleNamespace(
|
||||
sanitize_borrowed_credential_payload=lambda entry, provider_id=None: entry,
|
||||
)
|
||||
|
||||
# Stubs for the browser-provider plugin layer introduced in PR #25214.
|
||||
# The fake `agent` package has an empty __path__ so real submodules
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue