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:
Teknium 2026-07-29 12:18:06 -07:00
parent bcb352eeab
commit 3dd8059a05
No known key found for this signature in database
16 changed files with 73 additions and 44 deletions

View file

@ -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)

View file

@ -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")

View file

@ -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)

View file

@ -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

View file

@ -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)

View file

@ -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):

View file

@ -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

View file

@ -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):

View file

@ -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

View file

@ -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)

View file

@ -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

View file

@ -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

View file

@ -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:

View file

@ -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__), ".."))

View 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__), ".."))
# ---------------------------------------------------------------------------

View 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