mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
perf(docker-tests): share containers across read-only tests
Add a module-scoped `shared_container` fixture to tests/docker/conftest.py that boots one `sleep infinity` container per test module and tears it down at module exit. Convert read-only tests that previously used `docker run --rm --entrypoint sh/cat/test/su` (bypassing s6 to check static image properties) or `docker run -d` + `docker exec` (starting identical containers per test) to use `docker exec` on the shared container instead. Converted files: test_immutable_install_permissions.py — 2 throwaway runs → 2 execs test_license_file_present.py — 1 throwaway run → 1 exec test_tini_compat_shim.py — 1 throwaway run → 1 exec test_tui_prebuilt_bundle.py — 2 throwaway runs → 2 execs test_dump_build_sha.py — 2 throwaway runs → 2 execs test_immutable_install.py — 3 detached runs → 1 shared + 1 isolated test_dashboard.py — 2 detached runs → 0 (use shared) Local profiling shows docker run calls in these 7 files dropped from ~25 to 7 (the 7 are shared_container boots per module + the one test that needs a restart). Each eliminated `docker run` was paying 1-9s of s6 cont-init startup; the replacement `docker exec` calls average 0.10s — an ~50x speedup per operation. Tests that mutate state (restarts, config changes, gateway starts) still use their own containers via `container_name` + `start_container`.
This commit is contained in:
parent
3ec1e82629
commit
3e89edf830
8 changed files with 99 additions and 102 deletions
|
|
@ -88,6 +88,42 @@ def container_name(request) -> Iterator[str]:
|
|||
)
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def shared_container(built_image: str, request) -> Iterator[str]:
|
||||
"""A long-lived container shared across all tests in a module.
|
||||
|
||||
Starts one ``sleep infinity`` container, waits for s6 cont-init to
|
||||
finish, yields the container name, and tears it down at module exit.
|
||||
Tests that only need to *read* static image state (env vars, file
|
||||
existence, immutable-permission checks, etc.) can share this instead
|
||||
of each paying the full ``docker run`` + cont-init startup cost.
|
||||
|
||||
Tests that *mutate* container state (config changes, gateway starts,
|
||||
restarts, etc.) should still use ``container_name`` + ``start_container``
|
||||
for isolation.
|
||||
"""
|
||||
safe = request.module.__name__.replace(".", "-")
|
||||
name = f"hermes-shared-{safe}"
|
||||
# Clean up any leftover from a prior run.
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", name],
|
||||
capture_output=True, timeout=10,
|
||||
)
|
||||
r = subprocess.run(
|
||||
["docker", "run", "-d", "--name", name, built_image, "sleep", "infinity"],
|
||||
capture_output=True, text=True, timeout=60,
|
||||
)
|
||||
assert r.returncode == 0, f"docker run failed: {r.stderr}"
|
||||
try:
|
||||
wait_for_container_ready(name)
|
||||
yield name
|
||||
finally:
|
||||
subprocess.run(
|
||||
["docker", "rm", "-f", name],
|
||||
capture_output=True, timeout=10,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# docker_exec — default to the unprivileged hermes user
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -19,11 +19,10 @@ from tests.docker.conftest import docker_exec, docker_exec_sh, start_container,
|
|||
|
||||
|
||||
def test_dashboard_not_running_by_default(
|
||||
built_image: str, container_name: str,
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""Without HERMES_DASHBOARD, no dashboard process should be running."""
|
||||
start_container(built_image, container_name, cmd="sleep 60")
|
||||
r = docker_exec(container_name, "pgrep", "-f", "hermes dashboard")
|
||||
r = docker_exec(shared_container, "pgrep", "-f", "hermes dashboard")
|
||||
# pgrep exits non-zero when no match found
|
||||
assert r.returncode != 0, (
|
||||
"Dashboard should not be running without HERMES_DASHBOARD"
|
||||
|
|
@ -31,7 +30,7 @@ def test_dashboard_not_running_by_default(
|
|||
|
||||
|
||||
def test_dashboard_slot_reports_down_when_disabled(
|
||||
built_image: str, container_name: str,
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""Without HERMES_DASHBOARD, s6-svstat should report the dashboard
|
||||
slot as DOWN (not up-with-sleep-infinity, which would
|
||||
|
|
@ -41,11 +40,10 @@ def test_dashboard_slot_reports_down_when_disabled(
|
|||
writes a `down` marker file in the live service-dir when
|
||||
HERMES_DASHBOARD is unset, so the slot reflects reality.
|
||||
"""
|
||||
start_container(built_image, container_name, cmd="sleep 60")
|
||||
# /command/ isn't on PATH for docker-exec sessions, so call by
|
||||
# absolute path.
|
||||
r = docker_exec(
|
||||
container_name, "/command/s6-svstat", "/run/service/dashboard",
|
||||
shared_container, "/command/s6-svstat", "/run/service/dashboard",
|
||||
)
|
||||
assert r.returncode == 0, f"s6-svstat failed: {r.stderr!r} / {r.stdout!r}"
|
||||
assert "down" in r.stdout, (
|
||||
|
|
|
|||
|
|
@ -25,24 +25,21 @@ from __future__ import annotations
|
|||
import re
|
||||
import subprocess
|
||||
|
||||
from tests.docker.conftest import docker_exec_sh, wait_for_container_ready
|
||||
|
||||
|
||||
_VERSION_LINE = re.compile(r"^version:\s+(?P<rest>.+)$", re.MULTILINE)
|
||||
_SHA_BRACKET = re.compile(r"\[(?P<sha>[^\]]+)\]\s*$")
|
||||
|
||||
|
||||
def _run_dump(image: str) -> str:
|
||||
"""Return the stdout of ``docker run <image> dump``.
|
||||
def _run_dump(container: str) -> str:
|
||||
"""Return the stdout of ``hermes dump`` inside the running container.
|
||||
|
||||
Relies on Docker's anonymous VOLUME for ``/opt/data`` (declared by the
|
||||
Dockerfile) so the container's hermes user (UID 10000) can bootstrap
|
||||
its config. Anonymous volumes are auto-cleaned by ``--rm``, so unlike
|
||||
a host bind-mount we don't have to chown anything to UID 10000 (which
|
||||
would break cleanup on non-root hosts).
|
||||
The container is already booted with ``sleep infinity`` by the
|
||||
``shared_container`` fixture, so we just ``docker exec`` the command
|
||||
instead of paying the full ``docker run`` startup cost each time.
|
||||
"""
|
||||
r = subprocess.run(
|
||||
["docker", "run", "--rm", image, "dump"],
|
||||
capture_output=True, text=True, timeout=120,
|
||||
)
|
||||
r = docker_exec_sh(container, "hermes dump", timeout=60)
|
||||
assert r.returncode == 0, (
|
||||
f"hermes dump exited {r.returncode}: "
|
||||
f"stderr={r.stderr[-1000:]!r}\nstdout={r.stdout[-1000:]!r}"
|
||||
|
|
@ -50,29 +47,25 @@ def _run_dump(image: str) -> str:
|
|||
return r.stdout
|
||||
|
||||
|
||||
def _read_baked_sha_from_image(image: str) -> str | None:
|
||||
def _read_baked_sha_from_container(container: str) -> str | None:
|
||||
"""Return the ``/opt/hermes/.hermes_build_sha`` content, or None if absent."""
|
||||
r = subprocess.run(
|
||||
[
|
||||
"docker", "run", "--rm", "--entrypoint", "cat", image,
|
||||
"/opt/hermes/.hermes_build_sha",
|
||||
],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
r = docker_exec_sh(
|
||||
container, "cat /opt/hermes/.hermes_build_sha 2>/dev/null", timeout=10,
|
||||
)
|
||||
if r.returncode != 0:
|
||||
if r.returncode != 0 or not r.stdout.strip():
|
||||
return None
|
||||
return r.stdout.strip() or None
|
||||
|
||||
|
||||
def test_dump_reports_baked_sha_when_present(built_image: str) -> None:
|
||||
def test_dump_reports_baked_sha_when_present(shared_container: str) -> None:
|
||||
"""When the image was built with ``HERMES_GIT_SHA``, dump must surface it.
|
||||
|
||||
Together with the smoke-test action (which exercises ``--help``), this
|
||||
closes the regression loop for the missing-sha bug: any future change
|
||||
that breaks the baked-file -> dump pipeline will fail CI here.
|
||||
"""
|
||||
baked = _read_baked_sha_from_image(built_image)
|
||||
stdout = _run_dump(built_image)
|
||||
baked = _read_baked_sha_from_container(shared_container)
|
||||
stdout = _run_dump(shared_container)
|
||||
|
||||
match = _VERSION_LINE.search(stdout)
|
||||
assert match, f"no `version:` line in dump output:\n{stdout[:2000]}"
|
||||
|
|
|
|||
|
|
@ -7,6 +7,10 @@ Build the real image and verify at runtime:
|
|||
3. /opt/hermes/.install_method contains "docker" (code-scoped stamp)
|
||||
4. $HERMES_HOME/.install_method is NOT stamped as "docker" by stage2
|
||||
5. A stale "docker" stamp in $HERMES_HOME is healed (removed) on boot
|
||||
|
||||
Tests 1–4 are read-only checks against the default container state and
|
||||
share the module-scoped ``shared_container`` fixture. Test 5 mutates
|
||||
state and triggers a restart, so it uses its own container.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
|
|
@ -19,7 +23,7 @@ from tests.docker.conftest import (
|
|||
|
||||
|
||||
def test_install_tree_not_writable_by_hermes(
|
||||
built_image: str, container_name: str,
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""The hermes user must not be able to modify /opt/hermes.
|
||||
|
||||
|
|
@ -27,10 +31,8 @@ def test_install_tree_not_writable_by_hermes(
|
|||
root-owned and non-writable so an agent session cannot self-modify
|
||||
the installation and brick the gateway.
|
||||
"""
|
||||
start_container(built_image, container_name)
|
||||
|
||||
r = docker_exec_sh(
|
||||
container_name,
|
||||
shared_container,
|
||||
# Try to create a file under /opt/hermes as the hermes user
|
||||
"touch /opt/hermes/test_write 2>&1 && "
|
||||
"echo WRITE_SUCCEEDED || echo WRITE_FAILED",
|
||||
|
|
@ -43,7 +45,7 @@ def test_install_tree_not_writable_by_hermes(
|
|||
|
||||
# Also check a key subdirectory
|
||||
r = docker_exec_sh(
|
||||
container_name,
|
||||
shared_container,
|
||||
"touch /opt/hermes/.venv/test_write 2>&1 && "
|
||||
"echo WRITE_SUCCEEDED || echo WRITE_FAILED",
|
||||
timeout=10,
|
||||
|
|
@ -54,15 +56,13 @@ def test_install_tree_not_writable_by_hermes(
|
|||
|
||||
|
||||
def test_hermes_disable_lazy_installs_and_dont_write_bytecode(
|
||||
built_image: str, container_name: str,
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""The container must set PYTHONDONTWRITEBYTECODE and
|
||||
HERMES_DISABLE_LAZY_INSTALLS=1 so no .pyc files are written to the
|
||||
immutable install tree and no lazy installs attempt to modify it."""
|
||||
start_container(built_image, container_name)
|
||||
|
||||
r = docker_exec_sh(
|
||||
container_name,
|
||||
shared_container,
|
||||
'test "$PYTHONDONTWRITEBYTECODE" = "1" && '
|
||||
'test "$HERMES_DISABLE_LAZY_INSTALLS" = "1" && '
|
||||
'echo ENV_OK || echo ENV_MISSING',
|
||||
|
|
@ -75,15 +75,13 @@ def test_hermes_disable_lazy_installs_and_dont_write_bytecode(
|
|||
|
||||
|
||||
def test_install_method_stamp_is_code_scoped(
|
||||
built_image: str, container_name: str,
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""The 'docker' install-method stamp must be baked at
|
||||
/opt/hermes/.install_method (code-scoped), NOT in $HERMES_HOME."""
|
||||
start_container(built_image, container_name)
|
||||
|
||||
# Code-scoped stamp must exist and say "docker"
|
||||
r = docker_exec_sh(
|
||||
container_name,
|
||||
shared_container,
|
||||
"cat /opt/hermes/.install_method",
|
||||
timeout=10,
|
||||
)
|
||||
|
|
@ -96,7 +94,7 @@ def test_install_method_stamp_is_code_scoped(
|
|||
|
||||
# $HERMES_HOME must NOT have a 'docker' stamp
|
||||
r = docker_exec_sh(
|
||||
container_name,
|
||||
shared_container,
|
||||
"cat /opt/data/.install_method 2>/dev/null || echo NONE",
|
||||
timeout=10,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,27 +1,25 @@
|
|||
"""Docker smoke tests for immutable install permissions."""
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import textwrap
|
||||
|
||||
from tests.docker.conftest import docker_exec_sh
|
||||
|
||||
def test_container_sets_hosted_write_policy_env(built_image: str) -> None:
|
||||
|
||||
def test_container_sets_hosted_write_policy_env(shared_container: str) -> None:
|
||||
script = (
|
||||
'test "$HERMES_HOME" = "/opt/data" && '
|
||||
'test "$HERMES_WRITE_SAFE_ROOT" = "/opt/data" && '
|
||||
'test "$HERMES_DISABLE_LAZY_INSTALLS" = "1" && '
|
||||
'test "$PYTHONDONTWRITEBYTECODE" = "1"'
|
||||
)
|
||||
result = subprocess.run(
|
||||
["docker", "run", "--rm", "--entrypoint", "sh", built_image, "-c", script],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=60,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr[-2000:]
|
||||
r = docker_exec_sh(shared_container, script, timeout=30)
|
||||
assert r.returncode == 0, r.stderr[-2000:]
|
||||
|
||||
|
||||
def test_hermes_user_cannot_modify_install_but_can_write_data(built_image: str) -> None:
|
||||
def test_hermes_user_cannot_modify_install_but_can_write_data(
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
script = textwrap.dedent(
|
||||
r"""
|
||||
set -eu
|
||||
|
|
@ -46,22 +44,6 @@ def test_hermes_user_cannot_modify_install_but_can_write_data(built_image: str)
|
|||
PY
|
||||
"""
|
||||
).strip()
|
||||
result = subprocess.run(
|
||||
[
|
||||
"docker",
|
||||
"run",
|
||||
"--rm",
|
||||
"--entrypoint",
|
||||
"su",
|
||||
built_image,
|
||||
"hermes",
|
||||
"-s",
|
||||
"/bin/sh",
|
||||
"-c",
|
||||
script,
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=120,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr[-2000:]
|
||||
# Run as hermes user via docker_exec_sh's default user context.
|
||||
r = docker_exec_sh(shared_container, script, timeout=60)
|
||||
assert r.returncode == 0, r.stderr[-2000:]
|
||||
|
|
|
|||
|
|
@ -6,21 +6,17 @@ Docker image).
|
|||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
from tests.docker.conftest import docker_exec
|
||||
|
||||
|
||||
def test_docker_image_contains_license_file(built_image: str) -> None:
|
||||
def test_docker_image_contains_license_file(shared_container: str) -> None:
|
||||
"""The LICENSE file must be present inside the built Docker image.
|
||||
|
||||
PEP 639 license-files metadata references LICENSE, and the Docker
|
||||
build context must not exclude it.
|
||||
"""
|
||||
r = subprocess.run(
|
||||
["docker", "run", "--rm", "--entrypoint", "test",
|
||||
built_image, "-f", "/opt/hermes/LICENSE"],
|
||||
capture_output=True, text=True, timeout=60,
|
||||
)
|
||||
r = docker_exec(shared_container, "test", "-f", "/opt/hermes/LICENSE")
|
||||
assert r.returncode == 0, (
|
||||
f"LICENSE file not found at /opt/hermes/LICENSE inside the Docker "
|
||||
f"image: {r.stderr[-500:]}"
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ from __future__ import annotations
|
|||
import subprocess
|
||||
|
||||
|
||||
def test_tini_compat_symlink_exists(built_image: str) -> None:
|
||||
def test_tini_compat_symlink_exists(shared_container: str) -> None:
|
||||
"""/usr/bin/tini must exist as a symlink to /init.
|
||||
|
||||
Regression for #34192: orchestration templates (e.g. Hostinger's
|
||||
|
|
@ -20,11 +20,10 @@ def test_tini_compat_symlink_exists(built_image: str) -> None:
|
|||
PID-1 reaper without behavior change.
|
||||
"""
|
||||
r = subprocess.run(
|
||||
["docker", "run", "--rm", "--entrypoint", "sh",
|
||||
built_image, "-c",
|
||||
["docker", "exec", shared_container, "sh", "-c",
|
||||
'test -L /usr/bin/tini && '
|
||||
'test "$(readlink -f /usr/bin/tini)" = "/init"'],
|
||||
capture_output=True, text=True, timeout=60,
|
||||
capture_output=True, text=True, timeout=30,
|
||||
)
|
||||
assert r.returncode == 0, (
|
||||
f"/usr/bin/tini is not a symlink to /init: {r.stderr[-500:]}"
|
||||
|
|
@ -51,4 +50,4 @@ def test_entrypoint_is_init_not_tini(built_image: str) -> None:
|
|||
# /usr/bin/tini should NOT be in the entrypoint.
|
||||
assert "tini" not in entrypoint.lower(), (
|
||||
f"ENTRYPOINT references tini instead of /init: {entrypoint!r}"
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -18,33 +18,26 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import shlex
|
||||
import subprocess
|
||||
|
||||
from tests.docker.conftest import docker_exec_sh
|
||||
|
||||
|
||||
def _exec_py(image: str, py: str) -> str:
|
||||
"""Run a Python snippet inside the image as the hermes user, return stdout."""
|
||||
def _exec_py(container: str, py: str) -> str:
|
||||
"""Run a Python snippet inside the container as the hermes user, return stdout."""
|
||||
inner = (
|
||||
"source /opt/hermes/.venv/bin/activate && "
|
||||
". /opt/hermes/.venv/bin/activate && "
|
||||
"cd /opt/hermes && "
|
||||
f"python3 -c {shlex.quote(py)}"
|
||||
)
|
||||
# Drop to the hermes user (UID 10000) so we exercise the same path the
|
||||
# dashboard PTY child runs as — not root.
|
||||
cmd = [
|
||||
"docker", "run", "--rm", "--entrypoint", "su", image,
|
||||
"hermes", "-s", "/bin/bash", "-c", inner,
|
||||
]
|
||||
r = subprocess.run(cmd, capture_output=True, text=True, timeout=120)
|
||||
r = docker_exec_sh(container, inner, timeout=60)
|
||||
assert r.returncode == 0, f"in-container python failed:\n{r.stderr[-2000:]}"
|
||||
return r.stdout.strip()
|
||||
|
||||
|
||||
def test_hermes_tui_dir_env_is_set(built_image: str) -> None:
|
||||
def test_hermes_tui_dir_env_is_set(shared_container: str) -> None:
|
||||
"""HERMES_TUI_DIR must point at the prebuilt bundle dir in the image."""
|
||||
r = subprocess.run(
|
||||
["docker", "run", "--rm", "--entrypoint", "sh", built_image,
|
||||
"-c", 'printf "%s" "$HERMES_TUI_DIR"'],
|
||||
capture_output=True, text=True, timeout=60,
|
||||
r = docker_exec_sh(
|
||||
shared_container, 'printf "%s" "$HERMES_TUI_DIR"', timeout=30,
|
||||
)
|
||||
assert r.returncode == 0, r.stderr[-2000:]
|
||||
assert r.stdout.strip() == "/opt/hermes/ui-tui", (
|
||||
|
|
@ -52,7 +45,9 @@ def test_hermes_tui_dir_env_is_set(built_image: str) -> None:
|
|||
)
|
||||
|
||||
|
||||
def test_prebuilt_bundle_present_and_no_runtime_install(built_image: str) -> None:
|
||||
def test_prebuilt_bundle_present_and_no_runtime_install(
|
||||
shared_container: str,
|
||||
) -> None:
|
||||
"""The launcher must (a) find the prebuilt bundle and (b) NOT want an
|
||||
npm install — i.e. it takes the same path as a nix/packaged release."""
|
||||
py = (
|
||||
|
|
@ -69,7 +64,7 @@ def test_prebuilt_bundle_present_and_no_runtime_install(built_image: str) -> Non
|
|||
"}\n"
|
||||
"print(json.dumps(out))\n"
|
||||
)
|
||||
out = json.loads(_exec_py(built_image, py))
|
||||
out = json.loads(_exec_py(shared_container, py))
|
||||
assert out["dist_entry_exists"], "prebuilt ui-tui/dist/entry.js missing from image"
|
||||
# With HERMES_TUI_DIR set, _make_tui_argv returns the prebuilt path BEFORE
|
||||
# ever reaching the install check — so the resolved argv is what matters.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue