diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index da8c7e25e39..699fb57f12d 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -126,7 +126,7 @@ jobs: NOUS_API_KEY: "" run: | source .venv/bin/activate - python -m pytest tests/docker/ -v --tb=short + python scripts/run_tests_parallel.py tests/docker/ --file-timeout 300 -- -v --tb=short - name: Log in to Docker Hub if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'release' diff --git a/tests/docker/conftest.py b/tests/docker/conftest.py index f779517ef3c..cdda7e8faac 100644 --- a/tests/docker/conftest.py +++ b/tests/docker/conftest.py @@ -14,6 +14,7 @@ from __future__ import annotations import os import shutil import subprocess +import time from collections.abc import Iterator import pytest @@ -153,10 +154,8 @@ def wait_for_container_ready( better than a fixed ``time.sleep()`` that either wastes time on fast machines or flakes on slow ones. """ - import time as _time - - end = _time.monotonic() + deadline_s - while _time.monotonic() < end: + end = time.monotonic() + deadline_s + while time.monotonic() < end: r = docker_exec( container, "sh", "-c", @@ -165,7 +164,120 @@ def wait_for_container_ready( ) if r.returncode == 0 and "profile=default" in r.stdout: return - _time.sleep(interval_s) + time.sleep(interval_s) raise TimeoutError( f"container {container} did not finish cont-init within {deadline_s}s" ) + + +def start_container( + image: str, + name: str, + *env: str, + cmd: str = "sleep infinity", + timeout: int = 60, +) -> str: + """Start a detached container and wait for cont-init to finish. + + Args: + image: Docker image to run. + name: Container name (cleanup is the caller's responsibility — + typically handled by the ``container_name`` fixture). + env: Env vars as ``KEY=VALUE`` strings, each passed via ``-e``. + cmd: Container CMD (default ``sleep infinity``). + timeout: ``docker run`` subprocess timeout. + + Returns the container name. Raises on ``docker run`` failure or if + the container never finishes cont-init within 30s. + """ + args = ["docker", "run", "-d", "--name", name] + for e in env: + args.extend(["-e", e]) + args.extend([image, *cmd.split()]) + subprocess.run(args, check=True, capture_output=True, timeout=timeout) + wait_for_container_ready(name) + return name + + +def restart_container(container: str, timeout: int = 60) -> None: + """Restart a container and wait for cont-init to finish. + + Equivalent to ``docker restart `` followed by + :func:`wait_for_container_ready`. + """ + subprocess.run( + ["docker", "restart", container], + check=True, capture_output=True, timeout=timeout, + ) + wait_for_container_ready(container) + + +def poll_container( + container: str, + probe: str, + *, + deadline_s: float = 30.0, + interval_s: float = 0.5, + user: str = "hermes", +) -> tuple[bool, str]: + """Repeatedly run ``probe`` inside the container until it exits 0 or + ``deadline_s`` elapses. + + Returns ``(success, last_stdout)``. Useful for waiting on a process + to appear, a port to open, a file to contain a string, etc. + """ + end = time.monotonic() + deadline_s + last = "" + while time.monotonic() < end: + r = docker_exec_sh(container, probe, user=user, timeout=10) + last = r.stdout + if r.returncode == 0: + return True, last + time.sleep(interval_s) + return False, last + + +def wait_for_path( + container: str, + path: str, + *, + kind: str = "f", + deadline_s: float = 30.0, + interval_s: float = 0.25, +) -> bool: + """Poll ``test - `` inside the container until success or timeout. + + ``kind`` is the ``test`` flag: ``'f'`` for file, ``'d'`` for directory, + ``'e'`` for existence. Returns ``True`` on success, ``False`` on timeout. + """ + return poll_container( + container, f"test -{kind} {path}", + deadline_s=deadline_s, interval_s=interval_s, + )[0] + + +def wait_for_log( + container: str, + log_path: str, + needle: str, + *, + deadline_s: float = 30.0, + interval_s: float = 0.25, +) -> str: + """Poll until a log file inside the container contains ``needle``. + + Returns the matching log content on success, or the last observed + contents on timeout (so the caller can render a meaningful diagnostic). + """ + end = time.monotonic() + deadline_s + last = "" + while time.monotonic() < end: + r = docker_exec_sh( + container, f"cat {log_path} 2>/dev/null", timeout=5, + ) + if r.returncode == 0: + last = r.stdout + if needle in last: + return last + time.sleep(interval_s) + return last diff --git a/tests/docker/test_config_migration.py b/tests/docker/test_config_migration.py index 995b19cca9e..4640919e0a3 100644 --- a/tests/docker/test_config_migration.py +++ b/tests/docker/test_config_migration.py @@ -6,9 +6,7 @@ user. """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import docker_exec, docker_exec_sh, start_container def test_config_migration_runs_on_boot( @@ -17,12 +15,7 @@ def test_config_migration_runs_on_boot( """A config.yaml in $HERMES_HOME must be migrated on boot by docker_config_migrate.py, running as the hermes user.""" # Start container - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Verify config.yaml exists (should be seeded by stage2 if not present) r = docker_exec_sh( @@ -61,13 +54,9 @@ def test_config_migration_opt_out_env_var_respected( built_image: str, container_name: str, ) -> None: """HERMES_SKIP_CONFIG_MIGRATION=1 must skip the migration.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "HERMES_SKIP_CONFIG_MIGRATION=1", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, + start_container( + built_image, container_name, "HERMES_SKIP_CONFIG_MIGRATION=1", ) - wait_for_container_ready(container_name) # config.yaml should still be seeded (seeding is separate from migration) r = docker_exec_sh( @@ -77,4 +66,4 @@ def test_config_migration_opt_out_env_var_respected( ) assert "EXISTS" in r.stdout, ( f"config.yaml should be seeded even with migration skipped: {r.stdout}" - ) \ No newline at end of file + ) diff --git a/tests/docker/test_dashboard.py b/tests/docker/test_dashboard.py index 800414f58ee..a69b982ab6a 100644 --- a/tests/docker/test_dashboard.py +++ b/tests/docker/test_dashboard.py @@ -16,36 +16,14 @@ import json import subprocess import time -from tests.docker.conftest import docker_exec, docker_exec_sh - - -def _poll(container: str, probe: str, *, deadline_s: float = 30.0, - interval_s: float = 0.5) -> tuple[bool, str]: - """Repeatedly run ``probe`` inside the container until it exits 0 or - ``deadline_s`` elapses. Returns (success, last stdout).""" - end = time.monotonic() + deadline_s - last = "" - while time.monotonic() < end: - r = docker_exec_sh(container, probe, timeout=10) - last = r.stdout - if r.returncode == 0: - return True, last - time.sleep(interval_s) - return False, last +from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, poll_container def test_dashboard_not_running_by_default( built_image: str, container_name: str, ) -> None: """Without HERMES_DASHBOARD, no dashboard process should be running.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "60"], - check=True, capture_output=True, timeout=30, - ) - # Give the entrypoint enough time to finish bootstrap; if a dashboard - # were going to start it'd be visible by now. - time.sleep(5) + start_container(built_image, container_name, cmd="sleep 60") r = docker_exec(container_name, "pgrep", "-f", "hermes dashboard") # pgrep exits non-zero when no match found assert r.returncode != 0, ( @@ -64,12 +42,7 @@ 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. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "60"], - check=True, capture_output=True, timeout=30, - ) - time.sleep(5) + 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( @@ -135,7 +108,7 @@ def test_dashboard_opt_in_starts( # Poll for the dashboard subprocess to appear — the entrypoint # backgrounds it and bootstrap (skills sync etc.) can take a few # seconds before the python process actually launches. - ok, _ = _poll( + ok, _ = poll_container( container_name, "pgrep -f 'hermes dashboard'", deadline_s=30.0, ) assert ok, "Dashboard should be running with HERMES_DASHBOARD=1" @@ -160,7 +133,7 @@ def test_dashboard_port_override( # to the port yet — uvicorn takes another second or two to come up. # The image doesn't ship ss/netstat, so probe /proc/net/tcp directly: # port 9120 = 0x23A0, state 0A = LISTEN. - ok, stdout = _poll( + ok, stdout = poll_container( container_name, "grep -E ' 0+:23A0 .* 0A ' /proc/net/tcp /proc/net/tcp6 " "2>/dev/null", @@ -193,7 +166,7 @@ def test_dashboard_restarts_after_crash( check=True, capture_output=True, timeout=30, ) # Wait for the first dashboard to come up. - ok, _ = _poll( + ok, _ = poll_container( container_name, "pgrep -f 'hermes dashboard'", deadline_s=30.0, ) assert ok, "Dashboard never started initially" @@ -409,7 +382,7 @@ def test_dashboard_insecure_env_var_no_longer_bypasses_gate( # Fail-closed: the dashboard process must NOT successfully serve. Probe # for a few seconds; /api/status should never become reachable because # start_server raised SystemExit before binding. - ok, _ = _poll( + ok, _ = poll_container( container_name, "curl -fsS -m 2 http://127.0.0.1:9119/api/status >/dev/null 2>&1", deadline_s=12.0, diff --git a/tests/docker/test_docker_exec_privilege_drop.py b/tests/docker/test_docker_exec_privilege_drop.py index 745848938a3..21b49a096ef 100644 --- a/tests/docker/test_docker_exec_privilege_drop.py +++ b/tests/docker/test_docker_exec_privilege_drop.py @@ -287,4 +287,4 @@ def test_e2e_login_then_supervised_gateway_can_read_auth( "Files written by `docker exec` are unreadable to the hermes user " f"(supervised gateway UID): {unreadable}. The shim failed to drop " "privileges before the write." - ) + ) \ No newline at end of file diff --git a/tests/docker/test_gateway_run_supervised.py b/tests/docker/test_gateway_run_supervised.py index acdd01406a9..fac00c36d91 100644 --- a/tests/docker/test_gateway_run_supervised.py +++ b/tests/docker/test_gateway_run_supervised.py @@ -387,5 +387,4 @@ def test_supervised_gateway_stdout_reaches_docker_logs( "Banner also missing from rotated log file — the file " "destination may have been dropped by the new s6-log script. " f"File contents:\n{file_contents}" - ) - + ) \ No newline at end of file diff --git a/tests/docker/test_home_override_scripts.py b/tests/docker/test_home_override_scripts.py index d800fac99e3..bd850f4b5e2 100644 --- a/tests/docker/test_home_override_scripts.py +++ b/tests/docker/test_home_override_scripts.py @@ -11,7 +11,7 @@ from __future__ import annotations import subprocess -from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, restart_container def test_main_wrapper_preserves_docker_workdir( @@ -57,15 +57,7 @@ def test_dashboard_service_resets_home( we bind to 127.0.0.1 where the auth gate doesn't engage, and check the process env. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "HERMES_DASHBOARD=1", - "-e", "HERMES_DASHBOARD_HOST=127.0.0.1", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - # Give s6 + dashboard service time to start. - wait_for_container_ready(container_name) + start_container(built_image, container_name, "HERMES_DASHBOARD=1", "HERMES_DASHBOARD_HOST=127.0.0.1") # Check if the dashboard process is running and inspect its HOME. r = docker_exec_sh( @@ -104,14 +96,7 @@ def test_dashboard_does_not_auto_insecure_from_host( gate correctly blocks an unauthenticated non-loopback bind), that's also acceptable — the point is no auto-insecure. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "HERMES_DASHBOARD=1", - "-e", "HERMES_DASHBOARD_HOST=0.0.0.0", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name, "HERMES_DASHBOARD=1", "HERMES_DASHBOARD_HOST=0.0.0.0") # Check the dashboard process command line for --insecure. r = docker_exec_sh( @@ -140,12 +125,7 @@ def test_stage2_repairs_profiles_and_cron_ownership( We simulate a root-owned file in each, then restart the container and verify ownership is repaired. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Create root-owned files in profiles/ and cron/ to simulate # docker exec (root) writes. @@ -174,12 +154,7 @@ def test_stage2_repairs_profiles_and_cron_ownership( ) # Restart — stage2 hook runs again and repairs ownership. - subprocess.run( - ["docker", "restart", container_name], - check=True, capture_output=True, timeout=60, - ) - # Wait for stage2 to complete. - wait_for_container_ready(container_name) + restart_container(container_name) # Verify files are now owned by hermes. r = docker_exec_sh( diff --git a/tests/docker/test_immutable_install.py b/tests/docker/test_immutable_install.py index 011cacee2ad..99d2a1d9739 100644 --- a/tests/docker/test_immutable_install.py +++ b/tests/docker/test_immutable_install.py @@ -10,9 +10,12 @@ Build the real image and verify at runtime: """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import ( + docker_exec, + docker_exec_sh, + restart_container, + start_container, +) def test_install_tree_not_writable_by_hermes( @@ -24,12 +27,7 @@ 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. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) r = docker_exec_sh( container_name, @@ -61,12 +59,7 @@ def test_hermes_disable_lazy_installs_and_dont_write_bytecode( """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.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) r = docker_exec_sh( container_name, @@ -86,12 +79,7 @@ def test_install_method_stamp_is_code_scoped( ) -> None: """The 'docker' install-method stamp must be baked at /opt/hermes/.install_method (code-scoped), NOT in $HERMES_HOME.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Code-scoped stamp must exist and say "docker" r = docker_exec_sh( @@ -124,12 +112,7 @@ def test_stale_docker_stamp_in_home_is_healed_on_boot( """A stale 'docker' stamp left in $HERMES_HOME by an older image must be removed on boot so shared homes self-heal.""" # Start container, write a stale stamp - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Write a stale 'docker' stamp as root docker_exec( @@ -142,11 +125,7 @@ def test_stale_docker_stamp_in_home_is_healed_on_boot( assert r.stdout.strip() == "docker" # Restart - stage2 should heal it - subprocess.run( - ["docker", "restart", container_name], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + restart_container(container_name) # The stale stamp must be gone r = docker_exec_sh( @@ -158,4 +137,4 @@ def test_stale_docker_stamp_in_home_is_healed_on_boot( assert "HEALED" in r.stdout or r.stdout.strip() != "docker", ( f"stale 'docker' stamp in $HERMES_HOME was not healed on boot: " f"{r.stdout}" - ) \ No newline at end of file + ) diff --git a/tests/docker/test_log_dir_seed.py b/tests/docker/test_log_dir_seed.py index e827e057375..2893da2814d 100644 --- a/tests/docker/test_log_dir_seed.py +++ b/tests/docker/test_log_dir_seed.py @@ -10,21 +10,14 @@ s6-log crash-loops on mkdir: Permission denied. """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import docker_exec_sh, start_container def test_logs_gateways_seeded_and_hermes_owned( built_image: str, container_name: str, ) -> None: """logs/ and logs/gateways/ must exist and be owned by hermes after boot.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Both directories must exist r = docker_exec_sh( @@ -51,4 +44,4 @@ def test_logs_gateways_seeded_and_hermes_owned( ) assert "gateways=hermes" in r.stdout, ( f"logs/gateways/ not owned by hermes: {r.stdout}" - ) \ No newline at end of file + ) diff --git a/tests/docker/test_profile_gateway.py b/tests/docker/test_profile_gateway.py index 5bfc1c46c87..9ddd6d1a7de 100644 --- a/tests/docker/test_profile_gateway.py +++ b/tests/docker/test_profile_gateway.py @@ -69,12 +69,7 @@ def _svstat_wants_up(container: str) -> bool: def test_profile_create_then_gateway_start( built_image: str, container_name: str, ) -> None: - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "120"], - check=True, capture_output=True, timeout=30, - ) - time.sleep(3) + start_container(built_image, container_name, cmd="sleep 120") r = _sh(container_name, f"hermes profile create {PROFILE}") assert r.returncode == 0, f"profile create failed: {r.stderr}" @@ -114,12 +109,7 @@ def test_profile_delete_stops_gateway( ) -> None: """Deleting a profile should stop its gateway and remove the s6 service slot.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "120"], - check=True, capture_output=True, timeout=30, - ) - time.sleep(3) + start_container(built_image, container_name, cmd="sleep 120") _sh(container_name, f"hermes profile create {PROFILE}") _sh(container_name, f"hermes -p {PROFILE} gateway start", timeout=60) @@ -135,4 +125,4 @@ def test_profile_delete_stops_gateway( time.sleep(2) # Service slot should be gone. r = _sh(container_name, f"test -d /run/service/gateway-{PROFILE}") - assert r.returncode != 0, "s6 service slot still present after profile delete" + assert r.returncode != 0, "s6 service slot still present after profile delete" \ No newline at end of file diff --git a/tests/docker/test_puid_pgid_remap.py b/tests/docker/test_puid_pgid_remap.py index ab25a89b553..a6fafae8236 100644 --- a/tests/docker/test_puid_pgid_remap.py +++ b/tests/docker/test_puid_pgid_remap.py @@ -10,23 +10,14 @@ Build the real image and verify the actual runtime behavior: """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import docker_exec_sh, start_container def test_puid_pgid_remaps_hermes_user( built_image: str, container_name: str, ) -> None: """PUID=1000 PGID=1000 must remap the hermes user to UID 1000.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "PUID=1000", - "-e", "PGID=1000", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name, "PUID=1000", "PGID=1000") r = docker_exec_sh( container_name, @@ -51,16 +42,7 @@ def test_hermes_uid_gid_take_precedence_over_aliases( built_image: str, container_name: str, ) -> None: """HERMES_UID/HERMES_GID must win over PUID/PGID when both are set.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "HERMES_UID=2000", - "-e", "HERMES_GID=2001", - "-e", "PUID=1000", - "-e", "PGID=1000", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name, "HERMES_UID=2000", "HERMES_GID=2001", "PUID=1000", "PGID=1000") r = docker_exec_sh(container_name, "id -u hermes", timeout=10) assert r.stdout.strip() == "2000", ( @@ -77,14 +59,7 @@ def test_nas_low_uid_accepted( built_image: str, container_name: str, ) -> None: """NAS-style low UIDs (99:100, common on Unraid) must be accepted.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "PUID=99", - "-e", "PGID=100", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name, "PUID=99", "PGID=100") r = docker_exec_sh(container_name, "id -u hermes", timeout=10) assert r.stdout.strip() == "99", ( @@ -101,14 +76,7 @@ def test_remap_enables_data_volume_writes( built_image: str, container_name: str, ) -> None: """After remap, the hermes user must be able to write to /opt/data.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - "-e", "PUID=1000", - "-e", "PGID=1000", - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name, "PUID=1000", "PGID=1000") r = docker_exec_sh( container_name, diff --git a/tests/docker/test_s6_profile_gateway_integration.py b/tests/docker/test_s6_profile_gateway_integration.py index 22b41ca5ace..fc64e44054c 100644 --- a/tests/docker/test_s6_profile_gateway_integration.py +++ b/tests/docker/test_s6_profile_gateway_integration.py @@ -22,7 +22,7 @@ from __future__ import annotations import subprocess import time -from tests.docker.conftest import docker_exec +from tests.docker.conftest import docker_exec, start_container _REGISTER_SCRIPT = """ @@ -45,49 +45,39 @@ print("UNREGISTERED") """ -def _exec(container: str, *args: str, timeout: int = 30) -> subprocess.CompletedProcess: - return docker_exec(container, *args, timeout=timeout) - - def test_s6_register_creates_service_dir_in_live_container( built_image: str, container_name: str, ) -> None: """S6ServiceManager.register_profile_gateway must create ``/run/service/gateway-/`` and trigger s6-svscan rescan against the real s6 supervision tree.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "120"], - check=True, capture_output=True, timeout=30, - ) - # Give the supervision tree a moment to come up. - time.sleep(3) + start_container(built_image, container_name, cmd="sleep 120") - r = _exec(container_name, "python3", "-c", _REGISTER_SCRIPT, timeout=30) + r = docker_exec(container_name, "python3", "-c", _REGISTER_SCRIPT, timeout=30) assert "REGISTERED" in r.stdout, ( f"register failed: stderr={r.stderr!r} stdout={r.stdout!r}" ) # Service directory exists with the expected structure. - r = _exec(container_name, "test", "-d", "/run/service/gateway-phase3test") + r = docker_exec(container_name, "test", "-d", "/run/service/gateway-phase3test") assert r.returncode == 0, "service directory not created" - r = _exec(container_name, "test", "-f", "/run/service/gateway-phase3test/run") + r = docker_exec(container_name, "test", "-f", "/run/service/gateway-phase3test/run") assert r.returncode == 0, "run script not created" - r = _exec(container_name, "test", "-f", + r = docker_exec(container_name, "test", "-f", "/run/service/gateway-phase3test/log/run") assert r.returncode == 0, "log/run script not created" # s6-svscan picked it up — s6-svstat works against the dir. # `docker exec` doesn't put /command/ on PATH (only the supervision # tree does), so call s6-svstat by absolute path. - r = _exec(container_name, "/command/s6-svstat", + r = docker_exec(container_name, "/command/s6-svstat", "/run/service/gateway-phase3test") assert r.returncode == 0, f"s6-svstat failed: {r.stderr or r.stdout}" # list_profile_gateways picks it up. - r = _exec(container_name, "python3", "-c", ( + r = docker_exec(container_name, "python3", "-c", ( "from hermes_cli.service_manager import S6ServiceManager;" "print(S6ServiceManager().list_profile_gateways())" )) @@ -108,22 +98,22 @@ def test_s6_unregister_removes_service_dir_in_live_container( time.sleep(3) # First register so we have something to unregister. - r = _exec(container_name, "python3", "-c", _REGISTER_SCRIPT, timeout=30) + r = docker_exec(container_name, "python3", "-c", _REGISTER_SCRIPT, timeout=30) assert "REGISTERED" in r.stdout # Then unregister. - r = _exec(container_name, "python3", "-c", _UNREGISTER_SCRIPT, timeout=30) + r = docker_exec(container_name, "python3", "-c", _UNREGISTER_SCRIPT, timeout=30) assert "UNREGISTERED" in r.stdout, ( f"unregister failed: stderr={r.stderr!r} stdout={r.stdout!r}" ) # Directory is gone. - r = _exec(container_name, "test", "-d", "/run/service/gateway-phase3test") + r = docker_exec(container_name, "test", "-d", "/run/service/gateway-phase3test") assert r.returncode != 0, "service directory still exists after unregister" # list_profile_gateways no longer includes it. - r = _exec(container_name, "python3", "-c", ( + r = docker_exec(container_name, "python3", "-c", ( "from hermes_cli.service_manager import S6ServiceManager;" "print(S6ServiceManager().list_profile_gateways())" )) - assert "phase3test" not in r.stdout + assert "phase3test" not in r.stdout \ No newline at end of file diff --git a/tests/docker/test_stage2_browser_discovery.py b/tests/docker/test_stage2_browser_discovery.py index 9e490607003..9c4beaeb9bd 100644 --- a/tests/docker/test_stage2_browser_discovery.py +++ b/tests/docker/test_stage2_browser_discovery.py @@ -7,9 +7,7 @@ up by a broad ``find | grep``). """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import docker_exec_sh, start_container def test_stage2_discovers_chromium_binary( @@ -23,13 +21,7 @@ def test_stage2_discovers_chromium_binary( Playwright's tarball but must not be picked up. This test verifies the discovered binary is a real browser, not a .so. """ - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - # Give s6 + stage2-hook time to run. - wait_for_container_ready(container_name) + start_container(built_image, container_name) # AGENT_BROWSER_EXECUTABLE_PATH must be set via s6 container_environment. r = docker_exec_sh( @@ -77,12 +69,7 @@ def test_stage2_browser_path_accessible_to_hermes_user( """The discovered browser binary must be accessible to the unprivileged hermes user (UID 10000), since that's who runs agent-browser subprocesses.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) r = docker_exec_sh( container_name, @@ -92,4 +79,4 @@ def test_stage2_browser_path_accessible_to_hermes_user( ) assert r.returncode == 0, ( f"browser binary not readable+executable by hermes user: {r.stderr}" - ) \ No newline at end of file + ) diff --git a/tests/docker/test_toplevel_chown.py b/tests/docker/test_toplevel_chown.py index 4d833325d14..da3778d9cce 100644 --- a/tests/docker/test_toplevel_chown.py +++ b/tests/docker/test_toplevel_chown.py @@ -9,9 +9,12 @@ Build the real image and verify the actual runtime behavior: """ from __future__ import annotations -import subprocess - -from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_container_ready +from tests.docker.conftest import ( + docker_exec, + docker_exec_sh, + restart_container, + start_container, +) # The files the stage2 hook should repair (mirrors the allowlist in @@ -23,12 +26,7 @@ def test_root_owned_state_files_repaired_on_boot( built_image: str, container_name: str, ) -> None: """Root-owned top-level state files must be chowned to hermes on boot.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Create root-owned state files to simulate docker exec (root) writes for f in ALLOWLISTED_FILES: @@ -47,11 +45,7 @@ def test_root_owned_state_files_repaired_on_boot( assert line == "root", f"expected root-owned, got: {line}" # Restart - stage2 should repair ownership - subprocess.run( - ["docker", "restart", container_name], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + restart_container(container_name) # Verify files are now hermes-owned r = docker_exec_sh( @@ -71,12 +65,7 @@ def test_non_allowlisted_host_file_not_touched( """A non-allowlisted host-owned file must NOT be chowned, even if root-owned. Regression guard for #19788 / #19795: a bind-mounted $HERMES_HOME may contain host-owned files Hermes does not manage.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, - built_image, "sleep", "infinity"], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + start_container(built_image, container_name) # Create a non-allowlisted file as root docker_exec( @@ -90,11 +79,7 @@ def test_non_allowlisted_host_file_not_touched( ) # Restart - subprocess.run( - ["docker", "restart", container_name], - check=True, capture_output=True, timeout=60, - ) - wait_for_container_ready(container_name) + restart_container(container_name) # The file must STILL be root-owned (not touched by stage2) r = docker_exec_sh( @@ -105,4 +90,4 @@ def test_non_allowlisted_host_file_not_touched( assert r.stdout.strip() == "root", ( f"non-allowlisted host file was chowned by stage2 (should be " f"preserved): {r.stdout.strip()}" - ) \ No newline at end of file + ) diff --git a/tests/docker/test_zombie_reaping.py b/tests/docker/test_zombie_reaping.py index ff31be8c0d2..dc14d3c93ca 100644 --- a/tests/docker/test_zombie_reaping.py +++ b/tests/docker/test_zombie_reaping.py @@ -12,22 +12,16 @@ docstring. """ from __future__ import annotations -import subprocess import time -from tests.docker.conftest import docker_exec, docker_exec_sh +from tests.docker.conftest import docker_exec, docker_exec_sh, start_container, start_container def test_orphan_zombies_reaped( built_image: str, container_name: str, ) -> None: """Spawn an orphan child that exits immediately. PID 1 must reap it.""" - subprocess.run( - ["docker", "run", "-d", "--name", container_name, built_image, - "sleep", "60"], - check=True, capture_output=True, timeout=30, - ) - time.sleep(2) + start_container(built_image, container_name, cmd="sleep 60") # `( ( sleep 0.1 & ) & ); sleep 1` creates a grandchild detached from # the original docker exec session — it becomes an orphan reparented @@ -42,4 +36,4 @@ def test_orphan_zombies_reaped( line for line in r.stdout.split("\n") if line.strip().startswith("Z") ] - assert not zombies, f"Zombies not reaped by PID 1: {zombies}" + assert not zombies, f"Zombies not reaped by PID 1: {zombies}" \ No newline at end of file