mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
refactor(tests): re-architect tests + fix CI failures (#5946)
* refactor: re-architect tests to mirror the codebase
* Update tests.yml
* fix: add missing tool_error imports after registry refactor
* fix(tests): replace patch.dict with monkeypatch to prevent env var leaks under xdist
patch.dict(os.environ) can leak TERMINAL_ENV across xdist workers,
causing test_code_execution tests to hit the Modal remote path.
* fix(tests): fix update_check and telegram xdist failures
- test_update_check: replace patch("hermes_cli.banner.os.getenv") with
monkeypatch.setenv("HERMES_HOME") — banner.py no longer imports os
directly, it uses get_hermes_home() from hermes_constants.
- test_telegram_conflict/approval_buttons: provide real exception classes
for telegram.error mock (NetworkError, TimedOut, BadRequest) so the
except clause in connect() doesn't fail with "catching classes that do
not inherit from BaseException" when xdist pollutes sys.modules.
* fix(tests): accept unavailable_models kwarg in _prompt_model_selection mock
This commit is contained in:
Siddharth Balyan
GitHub
parent
99ff375f7a
commit
f3006ebef9
110 changed files with 153 additions and 150 deletions
|
|
@ -12,8 +12,6 @@ Covers the bugs discovered while setting up TBLite evaluation:
|
|||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
# Ensure repo root is importable
|
||||
|
|
@ -64,89 +62,72 @@ class TestToolResolution:
|
|||
class TestCwdHandling:
|
||||
"""Verify host paths are sanitized for container backends."""
|
||||
|
||||
def test_home_path_replaced_for_modal(self):
|
||||
def test_home_path_replaced_for_modal(self, monkeypatch):
|
||||
"""TERMINAL_CWD=/home/user/... should be replaced with /root for modal."""
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "modal",
|
||||
"TERMINAL_CWD": "/home/dakota/github/hermes-agent",
|
||||
}):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Expected /root, got {config['cwd']}. "
|
||||
"/home/ paths should be replaced for modal backend."
|
||||
)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "modal")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "/home/dakota/github/hermes-agent")
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Expected /root, got {config['cwd']}. "
|
||||
"/home/ paths should be replaced for modal backend."
|
||||
)
|
||||
|
||||
def test_users_path_replaced_for_docker_by_default(self):
|
||||
def test_users_path_replaced_for_docker_by_default(self, monkeypatch):
|
||||
"""Docker should keep host paths out of the sandbox unless explicitly enabled."""
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "docker",
|
||||
"TERMINAL_CWD": "/Users/someone/projects",
|
||||
}):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Expected /root, got {config['cwd']}. "
|
||||
"Host paths should be discarded for docker backend by default."
|
||||
)
|
||||
assert config["host_cwd"] is None
|
||||
assert config["docker_mount_cwd_to_workspace"] is False
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "/Users/someone/projects")
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Expected /root, got {config['cwd']}. "
|
||||
"Host paths should be discarded for docker backend by default."
|
||||
)
|
||||
assert config["host_cwd"] is None
|
||||
assert config["docker_mount_cwd_to_workspace"] is False
|
||||
|
||||
def test_users_path_maps_to_workspace_for_docker_when_enabled(self):
|
||||
def test_users_path_maps_to_workspace_for_docker_when_enabled(self, monkeypatch):
|
||||
"""Docker should map the host cwd into /workspace only when explicitly enabled."""
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "docker",
|
||||
"TERMINAL_CWD": "/Users/someone/projects",
|
||||
"TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE": "true",
|
||||
}):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/workspace"
|
||||
assert config["host_cwd"] == "/Users/someone/projects"
|
||||
assert config["docker_mount_cwd_to_workspace"] is True
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "/Users/someone/projects")
|
||||
monkeypatch.setenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", "true")
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/workspace"
|
||||
assert config["host_cwd"] == "/Users/someone/projects"
|
||||
assert config["docker_mount_cwd_to_workspace"] is True
|
||||
|
||||
def test_windows_path_replaced_for_modal(self):
|
||||
def test_windows_path_replaced_for_modal(self, monkeypatch):
|
||||
"""TERMINAL_CWD=C:\\Users\\... should be replaced for modal."""
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "modal",
|
||||
"TERMINAL_CWD": "C:\\Users\\someone\\projects",
|
||||
}):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root"
|
||||
monkeypatch.setenv("TERMINAL_ENV", "modal")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "C:\\Users\\someone\\projects")
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root"
|
||||
|
||||
def test_default_cwd_is_root_for_container_backends(self):
|
||||
@pytest.mark.parametrize("backend", ["modal", "docker", "singularity", "daytona"])
|
||||
def test_default_cwd_is_root_for_container_backends(self, backend, monkeypatch):
|
||||
"""Container backends should default to /root, not ~."""
|
||||
for backend in ("modal", "docker", "singularity", "daytona"):
|
||||
with patch.dict(os.environ, {"TERMINAL_ENV": backend}, clear=False):
|
||||
# Remove TERMINAL_CWD so it uses default
|
||||
env = os.environ.copy()
|
||||
env.pop("TERMINAL_CWD", None)
|
||||
env.pop("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", None)
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Backend {backend}: expected /root default, got {config['cwd']}"
|
||||
)
|
||||
monkeypatch.setenv("TERMINAL_ENV", backend)
|
||||
monkeypatch.delenv("TERMINAL_CWD", raising=False)
|
||||
monkeypatch.delenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", raising=False)
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/root", (
|
||||
f"Backend {backend}: expected /root default, got {config['cwd']}"
|
||||
)
|
||||
|
||||
def test_docker_default_cwd_maps_current_directory_when_enabled(self):
|
||||
def test_docker_default_cwd_maps_current_directory_when_enabled(self, monkeypatch):
|
||||
"""Docker should use /workspace when cwd mounting is explicitly enabled."""
|
||||
with patch("tools.terminal_tool.os.getcwd", return_value="/home/user/project"):
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "docker",
|
||||
"TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE": "true",
|
||||
}, clear=False):
|
||||
env = os.environ.copy()
|
||||
env.pop("TERMINAL_CWD", None)
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/workspace"
|
||||
assert config["host_cwd"] == "/home/user/project"
|
||||
monkeypatch.setattr("tools.terminal_tool.os.getcwd", lambda: "/home/user/project")
|
||||
monkeypatch.setenv("TERMINAL_ENV", "docker")
|
||||
monkeypatch.setenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", "true")
|
||||
monkeypatch.delenv("TERMINAL_CWD", raising=False)
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/workspace"
|
||||
assert config["host_cwd"] == "/home/user/project"
|
||||
|
||||
def test_local_backend_uses_getcwd(self):
|
||||
def test_local_backend_uses_getcwd(self, monkeypatch):
|
||||
"""Local backend should use os.getcwd(), not /root."""
|
||||
with patch.dict(os.environ, {"TERMINAL_ENV": "local"}, clear=False):
|
||||
env = os.environ.copy()
|
||||
env.pop("TERMINAL_CWD", None)
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == os.getcwd()
|
||||
monkeypatch.setenv("TERMINAL_ENV", "local")
|
||||
monkeypatch.delenv("TERMINAL_CWD", raising=False)
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == os.getcwd()
|
||||
|
||||
def test_create_environment_passes_docker_host_cwd_and_flag(self, monkeypatch):
|
||||
"""Docker host cwd and mount flag should reach DockerEnvironment."""
|
||||
|
|
@ -173,18 +154,16 @@ class TestCwdHandling:
|
|||
assert captured["host_cwd"] == "/home/user/project"
|
||||
assert captured["auto_mount_cwd"] is True
|
||||
|
||||
def test_ssh_preserves_home_paths(self):
|
||||
def test_ssh_preserves_home_paths(self, monkeypatch):
|
||||
"""SSH backend should NOT replace /home/ paths (they're valid remotely)."""
|
||||
with patch.dict(os.environ, {
|
||||
"TERMINAL_ENV": "ssh",
|
||||
"TERMINAL_CWD": "/home/remote-user/work",
|
||||
"TERMINAL_SSH_HOST": "example.com",
|
||||
"TERMINAL_SSH_USER": "user",
|
||||
}):
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/home/remote-user/work", (
|
||||
"SSH backend should preserve /home/ paths"
|
||||
)
|
||||
monkeypatch.setenv("TERMINAL_ENV", "ssh")
|
||||
monkeypatch.setenv("TERMINAL_CWD", "/home/remote-user/work")
|
||||
monkeypatch.setenv("TERMINAL_SSH_HOST", "example.com")
|
||||
monkeypatch.setenv("TERMINAL_SSH_USER", "user")
|
||||
config = _tt_mod._get_env_config()
|
||||
assert config["cwd"] == "/home/remote-user/work", (
|
||||
"SSH backend should preserve /home/ paths"
|
||||
)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
|
|
@ -194,12 +173,8 @@ class TestCwdHandling:
|
|||
class TestEphemeralDiskCheck:
|
||||
"""Verify ephemeral_disk is only passed when modal supports it."""
|
||||
|
||||
def test_ephemeral_disk_skipped_when_unsupported(self):
|
||||
def test_ephemeral_disk_skipped_when_unsupported(self, monkeypatch):
|
||||
"""If modal.Sandbox.create doesn't have ephemeral_disk param, skip it."""
|
||||
# Mock the modal import and Sandbox.create signature
|
||||
mock_modal = MagicMock()
|
||||
mock_sandbox_create = MagicMock()
|
||||
# Simulate a signature WITHOUT ephemeral_disk
|
||||
import inspect
|
||||
mock_params = {
|
||||
"args": inspect.Parameter("args", inspect.Parameter.VAR_POSITIONAL),
|
||||
|
|
@ -208,26 +183,25 @@ class TestEphemeralDiskCheck:
|
|||
"cpu": inspect.Parameter("cpu", inspect.Parameter.KEYWORD_ONLY),
|
||||
"memory": inspect.Parameter("memory", inspect.Parameter.KEYWORD_ONLY),
|
||||
}
|
||||
mock_sig = inspect.Signature(parameters=list(mock_params.values()))
|
||||
|
||||
with patch.dict(os.environ, {"TERMINAL_ENV": "modal"}):
|
||||
config = _tt_mod._get_env_config()
|
||||
# The config has container_disk default of 51200
|
||||
disk = config.get("container_disk", 51200)
|
||||
assert disk > 0, "disk should default to > 0"
|
||||
monkeypatch.setenv("TERMINAL_ENV", "modal")
|
||||
config = _tt_mod._get_env_config()
|
||||
# The config has container_disk default of 51200
|
||||
disk = config.get("container_disk", 51200)
|
||||
assert disk > 0, "disk should default to > 0"
|
||||
|
||||
# Simulate the version check logic from terminal_tool.py
|
||||
sandbox_kwargs = {}
|
||||
if disk > 0:
|
||||
try:
|
||||
if "ephemeral_disk" in mock_params:
|
||||
sandbox_kwargs["ephemeral_disk"] = disk
|
||||
except Exception:
|
||||
pass
|
||||
# Simulate the version check logic from terminal_tool.py
|
||||
sandbox_kwargs = {}
|
||||
if disk > 0:
|
||||
try:
|
||||
if "ephemeral_disk" in mock_params:
|
||||
sandbox_kwargs["ephemeral_disk"] = disk
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
assert "ephemeral_disk" not in sandbox_kwargs, (
|
||||
"ephemeral_disk should not be set when Sandbox.create doesn't support it"
|
||||
)
|
||||
assert "ephemeral_disk" not in sandbox_kwargs, (
|
||||
"ephemeral_disk should not be set when Sandbox.create doesn't support it"
|
||||
)
|
||||
|
||||
|
||||
# =========================================================================
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue