hermes-agent/tests/tools/test_web_providers.py
ethernet 48be2e0e4d
test: use subprocesses for each test file (#29016)
* ci(tests): install ripgrep from prebuilt tarball instead of apt

apt-get update + install of ripgrep takes ~4 min on the GHA Ubuntu
runners (the apt-get update against archive.ubuntu.com is the slow
part; ripgrep itself is small). Switching to the upstream musl
binary tarball cuts the step to a few seconds.

- Pinned to ripgrep 15.1.0 with sha256 verification (same hash as
  published in the releases sha256 sidecar file).
- Drops the `rg` binary into /usr/local/bin so it is on PATH for
  every subsequent step without GITHUB_PATH manipulation.
- Applied to both the test and e2e jobs in tests.yml.

* fix(cli): compile syntax check to tempdir, not source __pycache__

`_validate_critical_files_syntax` runs `py_compile.compile()` on each
critical bootstrap file after a successful `git pull`. The default
`py_compile` writes the resulting `.pyc` next to the source under
`__pycache__/`, which causes two real problems:

1. Parallel test workers walking the same source tree (e.g. running
   the suite under per-file process isolation) can race against each
   other on the `__pycache__` write — manifests as flaky 'directory
   not empty' errors during teardown.
2. In production, the post-pull syntax check leaves a `.pyc` behind
   that the next interpreter run might pick up — fine when the
   interpreter version matches, sketchy if it doesn't.

Fix: write the compiled output to a `tempfile.TemporaryDirectory()`
that's discarded on function exit. We only care about the compile-or-not
signal, not the artifact.

* test(runner): per-file process isolation, drop manual state reset + xdist

Replace fragile manual _reset_module_state test fixtures with robust
per-file subprocess isolation. Each test file runs in a fresh
`python -m pytest <file>` subprocess via ThreadPoolExecutor. No xdist,
no custom pytest plugin, no shared worker state.

Key changes:
  * scripts/run_tests_parallel.py — new runner: discovers test files,
    runs N in parallel via ThreadPoolExecutor, captures stdout per file,
    treats exit code 5 (no tests collected) as pass, kills all children
    on exit. Change from cpu_count to cpu_count*2. The runner is
    I/O-bound (waiting on subprocess.communicate() from pytest children)
    The parent process does almost no CPU work, so 2x oversubscription
    keeps more pipes full. When a file fails, immediately show the last
    30 lines of pytest output (stack traces + FAILED summary) plus a
    ready-to-copy repro command:
      python -m pytest tests/agent/test_auxiliary_client.py
  * scripts/run_tests.sh — delegates to run_tests_parallel.py
  * .github/workflows/tests.yml — test step: python
scripts/run_tests_parallel.py
  * pyproject.toml — drop pytest-xdist, pytest-split; simplify addopts
  * tests/conftest.py — remove ~200 lines of manual state-reset fixtures
  * AGENTS.md — update Testing section for per-file design

* test(runner): speed gateway test antipattern scan up

* fix(test): web search provider plugin test missing xai

* fix(tests): make 14 test files pass under per-file subprocess isolation

Tests that relied on cross-file state pollution from xdist workers
fail when run in isolation (per-file subprocess model). Root causes
and fixes:

Tool registry not populated:
  - test_video_generation_tool_surface_matrix: add discover_builtin_tools()
  - test_web_providers_brave_free/ddgs/searxng/general: autouse fixtures
    registering all 8 bundled web providers, reset after each test
  - test_website_policy: same provider registration pattern
  - test_web_tools_tavily: same pattern across 3 dispatch test classes
  - Also add is_safe_url/check_website_access mocks where SSRF check
    blocks example.com (DNS resolution fails in isolated envs)

Stale check_fn cache:
  - test_kanban_tools: invalidate_check_fn_cache() + _clear_tool_defs_cache()
    in both kanban guidance tests (prior test cached False for kanban_show)
  - test_discord_tool: cache invalidation in setup/teardown
  - test_homeassistant_tool: invalidate_check_fn_cache() before registry queries

Module-level state pollution:
  - test_auxiliary_client: autouse fixture clearing _aux_unhealthy_until cache
  - test_skill_commands: set_session_vars() instead of patch.dict(os.environ)
    (ContextVar takes precedence over os.environ)
  - test_dm_topics: overwrite sys.modules + separate telegram.constants mock
    + force-reimport of gateway.platforms.telegram
  - test_terminal_tool_requirements: removed duplicate class declaration,
    autouse _clear_caches fixture

* change(tests): run_tests.sh explicitly includes env vars

instead of manually dropping some vars, now we just only include some

* fix(tests): 5 more isolation/NixOS fixes

- test_approval_plugin_hooks: isolate HERMES_HOME so real user's
  command_allowlist doesn't short-circuit the approval path
- test_google_chat: skipif when Platform.GOOGLE_CHAT not in enum
  (feature not merged on this branch)
- test_write_deny: test systemd prefix against tmp_path instead of
  /etc/systemd which resolves to /nix/store on NixOS
- test_pty_bridge: use shutil.which('cat') instead of /bin/cat
  (doesn't exist on NixOS)
- profiles.py: rmtree onexc handler chmod's parent dirs too, fixing
  profile deletion when copytree preserved read-only modes from
  nix store

* fix(tests): clear unhealthy cache in autouse fixture for auxiliary_client

* fix(tests): skip send_message when telegram not installed; handle missing worker_id in browser_supervisor

* fix: py3.11 rmtree onexc compat + belt-and-suspenders unhealthy cache clear for expired codex test

* fix: address PR #29016 review feedback

- Remove tracked .pytest-cache/ artifact and add to .gitignore
- Fix stale 'xdist worker' comment in conftest.py
- Deduplicate web provider registration into tests/tools/conftest.py
  shared helper (register_all_web_providers), replacing 8 copy-pasted
  blocks across 6 test files
- Update PR description: remove stale recovered-test-files claim,
  fix worker count to match code (cpu_count*2)

* fix: eliminate race in stale-cache achievements test

The background scan thread could complete and overwrite _SNAPSHOT_CACHE
before evaluate_all() returned the stale data — only 10 fake sessions
made the scan finish instantly. Added scan_delay param to _FakeSessionDB
and set it to 2s in the stale-cache test so the background thread can't
win the race.
2026-05-21 16:40:04 +05:30

345 lines
13 KiB
Python

"""Tests for the web tools provider architecture.
Covers:
- WebSearchProvider / WebExtractProvider ABC enforcement
- Per-capability backend selection (_get_search_backend, _get_extract_backend)
- Backward compatibility (web.backend still works as shared fallback)
- Config keys merge correctly via DEFAULT_CONFIG
"""
from __future__ import annotations
import json
from typing import Any, Dict, List
import pytest
from tests.tools.conftest import register_all_web_providers
# ---------------------------------------------------------------------------
# ABC enforcement
# ---------------------------------------------------------------------------
class TestWebProviderABCs:
"""The unified WebSearchProvider ABC enforces the interface contract.
After PR #25182, all seven providers are subclasses of
:class:`agent.web_search_provider.WebSearchProvider`. The legacy
in-tree ABCs at ``tools.web_providers.base`` (separate
``WebSearchProvider`` + ``WebExtractProvider``) were deleted in the
same PR — providers now advertise capabilities via
``supports_search() / supports_extract() / supports_crawl()`` flags.
"""
def test_cannot_instantiate_abc_directly(self):
from agent.web_search_provider import WebSearchProvider
with pytest.raises(TypeError):
WebSearchProvider() # type: ignore[abstract]
def test_concrete_search_only_provider_works(self):
from agent.web_search_provider import WebSearchProvider
class Dummy(WebSearchProvider):
@property
def name(self) -> str:
return "dummy"
@property
def display_name(self) -> str:
return "Dummy Search"
def is_available(self) -> bool:
return True
def supports_search(self) -> bool:
return True
def search(self, query: str, limit: int = 5) -> Dict[str, Any]:
return {"success": True, "data": {"web": []}}
d = Dummy()
assert d.name == "dummy"
assert d.display_name == "Dummy Search"
assert d.is_available() is True
assert d.supports_search() is True
assert d.supports_extract() is False # default
assert d.supports_crawl() is False # default
assert d.search("test")["success"] is True
def test_concrete_multi_capability_provider_works(self):
from agent.web_search_provider import WebSearchProvider
class Dummy(WebSearchProvider):
@property
def name(self) -> str:
return "dummy"
@property
def display_name(self) -> str:
return "Dummy Multi"
def is_available(self) -> bool:
return True
def supports_search(self) -> bool:
return True
def supports_extract(self) -> bool:
return True
def supports_crawl(self) -> bool:
return True
def search(self, query: str, limit: int = 5) -> Dict[str, Any]:
return {"success": True, "data": {"web": []}}
def extract(self, urls: List[str], **kwargs: Any) -> List[Dict[str, Any]]:
return [{"url": urls[0], "content": "x"}]
def crawl(self, url: str, **kwargs: Any) -> Dict[str, Any]:
return {"results": [{"url": url, "content": "x"}]}
d = Dummy()
assert d.supports_search() is True
assert d.supports_extract() is True
assert d.supports_crawl() is True
assert d.extract(["https://example.com"])[0]["url"] == "https://example.com"
assert d.crawl("https://example.com")["results"][0]["url"] == "https://example.com"
def test_search_only_provider_skips_extract_and_crawl(self):
"""Search-only providers don't have to implement extract() / crawl()."""
from agent.web_search_provider import WebSearchProvider
class SearchOnly(WebSearchProvider):
@property
def name(self) -> str:
return "search-only"
@property
def display_name(self) -> str:
return "Search Only"
def is_available(self) -> bool:
return True
def supports_search(self) -> bool:
return True
def search(self, query: str, limit: int = 5) -> Dict[str, Any]:
return {"success": True, "data": {"web": []}}
# Should instantiate fine — extract/crawl have default
# supports_*() returning False and aren't required to be
# overridden when not advertised.
s = SearchOnly()
assert s.supports_search() is True
assert s.supports_extract() is False
assert s.supports_crawl() is False
# ---------------------------------------------------------------------------
# Per-capability backend selection
# ---------------------------------------------------------------------------
class TestPerCapabilityBackendSelection:
"""_get_search_backend and _get_extract_backend read per-capability config."""
def test_search_backend_overrides_generic(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "firecrawl",
"search_backend": "tavily",
})
monkeypatch.setenv("TAVILY_API_KEY", "test-key")
assert web_tools._get_search_backend() == "tavily"
def test_extract_backend_overrides_generic(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "tavily",
"extract_backend": "exa",
})
monkeypatch.setenv("EXA_API_KEY", "test-key")
assert web_tools._get_extract_backend() == "exa"
def test_falls_back_to_generic_backend_when_search_backend_empty(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "tavily",
"search_backend": "",
})
monkeypatch.setenv("TAVILY_API_KEY", "test-key")
assert web_tools._get_search_backend() == "tavily"
def test_falls_back_to_generic_backend_when_extract_backend_empty(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "parallel",
"extract_backend": "",
})
monkeypatch.setenv("PARALLEL_API_KEY", "test-key")
assert web_tools._get_extract_backend() == "parallel"
def test_search_backend_ignored_when_not_available(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "firecrawl",
"search_backend": "exa", # set but no EXA_API_KEY
})
monkeypatch.delenv("EXA_API_KEY", raising=False)
monkeypatch.setenv("FIRECRAWL_API_KEY", "fc-key")
# Should fall back to firecrawl since exa isn't configured
assert web_tools._get_search_backend() == "firecrawl"
def test_fully_backward_compatible_with_web_backend_only(self, monkeypatch):
from tools import web_tools
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {
"backend": "tavily",
})
monkeypatch.setenv("TAVILY_API_KEY", "test-key")
# No search_backend or extract_backend set — both fall through
assert web_tools._get_search_backend() == "tavily"
assert web_tools._get_extract_backend() == "tavily"
# ---------------------------------------------------------------------------
# Config key presence in DEFAULT_CONFIG
# ---------------------------------------------------------------------------
class TestDefaultConfig:
"""The web section exists in DEFAULT_CONFIG with per-capability keys."""
def test_web_section_in_default_config(self):
from hermes_cli.config import DEFAULT_CONFIG
assert "web" in DEFAULT_CONFIG
web = DEFAULT_CONFIG["web"]
assert "backend" in web
assert "search_backend" in web
assert "extract_backend" in web
# All empty string by default (no override)
assert web["backend"] == ""
assert web["search_backend"] == ""
assert web["extract_backend"] == ""
# ---------------------------------------------------------------------------
# web_search_tool uses _get_search_backend
# ---------------------------------------------------------------------------
class TestWebSearchUsesSearchBackend:
"""web_search_tool dispatches through _get_search_backend not _get_backend."""
def test_search_tool_calls_search_backend(self, monkeypatch):
from tools import web_tools
called_with = []
original_get_search = web_tools._get_search_backend
def tracking_get_search():
result = original_get_search()
called_with.append(("search", result))
return result
monkeypatch.setattr(web_tools, "_get_search_backend", tracking_get_search)
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {"backend": "firecrawl"})
monkeypatch.setenv("FIRECRAWL_API_KEY", "fake")
# The function will fail at Firecrawl client level but we just
# need to verify _get_search_backend was called
try:
web_tools.web_search_tool("test", 1)
except Exception:
pass
assert len(called_with) > 0
assert called_with[0][0] == "search"
class TestUnconfiguredErrorEnvelopeParity:
"""Regression tests for PR #25182: the post-migration dispatcher must
emit the same top-level error envelope as pre-migration main when no
web backend is configured.
Plugin-level error wrapping is correct for in-flight errors (per-page
SDK exceptions, scrape timeouts) but PRE-FLIGHT configuration errors
must surface at the top level so function-calling models that check
``result.get("error")`` detect the failure cleanly.
"""
_register_providers = staticmethod(register_all_web_providers)
@pytest.fixture(autouse=True)
def _populate_web_registry(self):
self._register_providers()
yield
from agent.web_search_registry import _reset_for_tests
_reset_for_tests()
def _clear_web_creds(self, monkeypatch):
for k in (
"BRAVE_SEARCH_API_KEY",
"SEARXNG_URL",
"TAVILY_API_KEY",
"EXA_API_KEY",
"PARALLEL_API_KEY",
"FIRECRAWL_API_KEY",
"FIRECRAWL_API_URL",
"FIRECRAWL_GATEWAY_URL",
"TOOL_GATEWAY_DOMAIN",
):
monkeypatch.delenv(k, raising=False)
def test_unconfigured_search_emits_top_level_error(self, monkeypatch):
"""``web_search_tool`` with no creds returns ``{"error": "Error searching web: ..."}``
— matching main's ``tool_error()`` envelope, not a per-result shape.
"""
import json
from tools import web_tools
self._clear_web_creds(monkeypatch)
# Reset firecrawl client cache so the unconfigured state is re-evaluated
monkeypatch.setattr(web_tools, "_firecrawl_client", None, raising=False)
monkeypatch.setattr(web_tools, "_firecrawl_client_config", None, raising=False)
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
result = json.loads(web_tools.web_search_tool("hello world", limit=3))
assert "error" in result, f"expected top-level 'error' key, got {result}"
# ``Error searching web:`` prefix comes from web_tools' top-level except handler
assert "Error searching web:" in result["error"]
assert "FIRECRAWL_API_KEY" in result["error"]
# No per-result burying
assert "results" not in result
def test_unconfigured_crawl_emits_top_level_error(self, monkeypatch):
"""``web_crawl_tool`` with no creds returns ``{"success": False, "error": "web_crawl requires Firecrawl..."}``
— the dispatcher gates on ``provider.is_available()`` BEFORE
delegating to the plugin so pre-config errors don't get wrapped
into ``results[]``.
"""
import asyncio
import json
from tools import web_tools
self._clear_web_creds(monkeypatch)
monkeypatch.setattr(web_tools, "_firecrawl_client", None, raising=False)
monkeypatch.setattr(web_tools, "_firecrawl_client_config", None, raising=False)
monkeypatch.setattr(web_tools, "_load_web_config", lambda: {})
result = json.loads(asyncio.run(web_tools.web_crawl_tool("https://example.com", use_llm_processing=False)))
assert result.get("success") is False
assert "error" in result, f"expected top-level 'error' key, got {result}"
assert "web_crawl requires Firecrawl" in result["error"]
# Crucially: no per-page burying
assert "results" not in result