From bb445b24ad307ed359d7cd2e8a193480b45e1041 Mon Sep 17 00:00:00 2001 From: ethernet Date: Tue, 23 Jun 2026 08:34:17 -0400 Subject: [PATCH] wip comments --- tests/docker/conftest.py | 42 +++++++++++++-- tests/docker/test_config_migration.py | 17 +++--- tests/docker/test_container_restart.py | 45 +++++++--------- tests/docker/test_gateway_bootstrap_state.py | 13 ++--- tests/docker/test_gateway_run_supervised.py | 24 ++++----- tests/docker/test_home_override_scripts.py | 22 ++++---- tests/docker/test_immutable_install.py | 18 +++---- tests/docker/test_license_file_present.py | 14 ++--- tests/docker/test_log_dir_seed.py | 11 ++-- tests/docker/test_puid_pgid_remap.py | 17 +++--- tests/docker/test_stage2_browser_discovery.py | 52 +++++-------------- tests/docker/test_tini_compat_shim.py | 5 +- tests/docker/test_toplevel_chown.py | 17 +++--- tests/docker/test_user_flag_guard.py | 6 +-- 14 files changed, 134 insertions(+), 169 deletions(-) diff --git a/tests/docker/conftest.py b/tests/docker/conftest.py index 4281a292fae..f779517ef3c 100644 --- a/tests/docker/conftest.py +++ b/tests/docker/conftest.py @@ -8,9 +8,6 @@ Override the image with ``HERMES_TEST_IMAGE`` env var to point at a pre-built image (faster local iteration); otherwise the ``built_image`` fixture builds the repo's Dockerfile once per session. -Docker tests need longer timeouts than the suite default (30s), so every -test under this directory is granted a 180s default via -``pytest.mark.timeout`` applied at collection time. """ from __future__ import annotations @@ -43,11 +40,9 @@ def pytest_collection_modifyitems(config, items): # noqa: D401 - pytest hook skip_docker = pytest.mark.skip( reason="Docker not available or daemon not running", ) - extend_timeout = pytest.mark.timeout(180) for item in items: if "tests/docker/" not in str(item.fspath).replace(os.sep, "/"): continue - item.add_marker(extend_timeout) if not docker_ok: item.add_marker(skip_docker) @@ -137,3 +132,40 @@ def docker_exec_sh( return docker_exec( container, "sh", "-c", command, user=user, timeout=timeout, ) + + +def wait_for_container_ready( + container: str, + *, + deadline_s: float = 30.0, + interval_s: float = 0.25, +) -> None: + """Poll until the container has finished s6 cont-init (stage2 + reconcile). + + The readiness signal is ``profile=default`` appearing in + ``/opt/data/logs/container-boot.log``, which the 02-reconcile-profiles + cont-init script writes on every boot. That log entry fires AFTER + stage2-hook.sh completes, so by the time it appears the full + cont-init chain (UID remap, chown, config seeding, skills sync, + browser discovery, config migration) has run. + + Raises ``TimeoutError`` if the container never becomes ready — much + 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: + r = docker_exec( + container, + "sh", "-c", + "cat /opt/data/logs/container-boot.log 2>/dev/null", + timeout=5, + ) + if r.returncode == 0 and "profile=default" in r.stdout: + return + _time.sleep(interval_s) + raise TimeoutError( + f"container {container} did not finish cont-init within {deadline_s}s" + ) diff --git a/tests/docker/test_config_migration.py b/tests/docker/test_config_migration.py index 469d974c1c9..995b19cca9e 100644 --- a/tests/docker/test_config_migration.py +++ b/tests/docker/test_config_migration.py @@ -1,17 +1,14 @@ """Runtime smoke test for Docker config-schema migration on boot. -Replaces the old text-assertion test that grepped stage2-hook.sh for -the docker_config_migrate.py invocation. This test builds the real -image and verifies the actual runtime behavior: a config.yaml present -in $HERMES_HOME is migrated by docker_config_migrate.py on boot, -running as the hermes user. +Build the real image and verify: a config.yaml present in $HERMES_HOME +is migrated by docker_config_migrate.py on boot, running as the hermes +user. """ 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, wait_for_container_ready def test_config_migration_runs_on_boot( @@ -25,7 +22,7 @@ def test_config_migration_runs_on_boot( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Verify config.yaml exists (should be seeded by stage2 if not present) r = docker_exec_sh( @@ -70,7 +67,7 @@ def test_config_migration_opt_out_env_var_respected( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # config.yaml should still be seeded (seeding is separate from migration) r = docker_exec_sh( @@ -80,4 +77,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_container_restart.py b/tests/docker/test_container_restart.py index ca08c127f8b..ad5a577256a 100644 --- a/tests/docker/test_container_restart.py +++ b/tests/docker/test_container_restart.py @@ -32,13 +32,6 @@ def _docker(*args: str, **kw) -> subprocess.CompletedProcess[str]: ) -def _exec(container: str, *args: str, timeout: int = 30) -> subprocess.CompletedProcess[str]: - return docker_exec(container, *args, timeout=timeout) - - -def _sh(container: str, cmd: str, timeout: int = 30) -> subprocess.CompletedProcess[str]: - return docker_exec_sh(container, cmd, timeout=timeout) - def _wait_for_path( container: str, @@ -61,7 +54,7 @@ def _wait_for_path( """ end = time.monotonic() + deadline_s while time.monotonic() < end: - r = _sh(container, f"test -{kind} {path}", timeout=5) + r = docker_exec_sh(container, f"test -{kind} {path}", timeout=5) if r.returncode == 0: return True time.sleep(interval_s) @@ -86,7 +79,7 @@ def _wait_for_reconcile_log_mention( end = time.monotonic() + deadline_s last = "" while time.monotonic() < end: - r = _sh(container, "cat /opt/data/logs/container-boot.log", timeout=5) + r = docker_exec_sh(container, "cat /opt/data/logs/container-boot.log", timeout=5) if r.returncode == 0: last = r.stdout if f"profile={profile}" in last: @@ -145,16 +138,16 @@ def test_running_gateway_survives_container_restart(restart_container: str) -> N # Create the profile + start its gateway. The Phase 4 hooks # register the s6 service slot during create and the dispatch # path brings it up via s6-svc -u. - r = _exec(container, "hermes", "profile", "create", "coder") + r = docker_exec(container, "hermes", "profile", "create", "coder") assert r.returncode == 0, f"profile create failed: {r.stderr}" - r = _exec(container, "hermes", "-p", "coder", "gateway", "start", timeout=60) + r = docker_exec(container, "hermes", "-p", "coder", "gateway", "start", timeout=60) assert r.returncode == 0, f"gateway start failed: {r.stderr}" # Give the service time to actually come up under supervision. deadline = time.monotonic() + 15.0 while time.monotonic() < deadline: - r = _sh(container, "/command/s6-svstat /run/service/gateway-coder") + r = docker_exec_sh(container, "/command/s6-svstat /run/service/gateway-coder") if r.returncode == 0 and "up " in r.stdout: break time.sleep(0.5) @@ -170,7 +163,7 @@ def test_running_gateway_survives_container_restart(restart_container: str) -> N "p = pathlib.Path('/opt/data/profiles/coder/gateway_state.json'); " "p.write_text(json.dumps({'gateway_state': 'running', 'timestamp': 1}))" ) - _exec(container, "python3", "-c", write_state, timeout=10).check_returncode() + docker_exec(container, "python3", "-c", write_state, timeout=10).check_returncode() # Restart. After this, /run/service/ is empty until cont-init.d # runs the reconciler. We need to wait long enough for the @@ -190,14 +183,14 @@ def test_running_gateway_survives_container_restart(restart_container: str) -> N ), "slot not recreated after restart" # No `down` marker — we asked for auto-start. - r = _sh(container, "test -f /run/service/gateway-coder/down") + r = docker_exec_sh(container, "test -f /run/service/gateway-coder/down") assert r.returncode != 0, "down marker present despite prior_state=running" def test_stopped_gateway_stays_stopped_after_restart(restart_container: str) -> None: container = restart_container - _exec(container, "hermes", "profile", "create", "writer").check_returncode() + docker_exec(container, "hermes", "profile", "create", "writer").check_returncode() # Write 'stopped' directly so we don't have to race against the # gateway's own state writes. @@ -206,7 +199,7 @@ def test_stopped_gateway_stays_stopped_after_restart(restart_container: str) -> "p = pathlib.Path('/opt/data/profiles/writer/gateway_state.json'); " "p.write_text(json.dumps({'gateway_state': 'stopped', 'timestamp': 1}))" ) - _exec(container, "python3", "-c", write_state, timeout=10).check_returncode() + docker_exec(container, "python3", "-c", write_state, timeout=10).check_returncode() _docker("restart", container, timeout=60).check_returncode() log = _wait_for_reconcile_log_mention(container, "writer", deadline_s=30.0) @@ -218,7 +211,7 @@ def test_stopped_gateway_stays_stopped_after_restart(restart_container: str) -> ) # Down marker present. - r = _sh(container, "test -f /run/service/gateway-writer/down") + r = docker_exec_sh(container, "test -f /run/service/gateway-writer/down") assert r.returncode == 0, "down marker missing despite prior_state=stopped" @@ -229,7 +222,7 @@ def test_stale_gateway_pid_cleaned_up_on_restart(restart_container: str) -> None process-mismatch checks.""" container = restart_container - _exec(container, "hermes", "profile", "create", "ghost").check_returncode() + docker_exec(container, "hermes", "profile", "create", "ghost").check_returncode() # Stamp stale runtime files alongside a 'running' state so the # reconciler walks this profile. @@ -240,15 +233,15 @@ def test_stale_gateway_pid_cleaned_up_on_restart(restart_container: str) -> None "(p / 'gateway.pid').write_text(json.dumps({'pid': 99999, 'host': 'old'})); " "(p / 'processes.json').write_text('[]')" ) - _exec(container, "python3", "-c", stamp, timeout=10).check_returncode() + docker_exec(container, "python3", "-c", stamp, timeout=10).check_returncode() _docker("restart", container, timeout=60).check_returncode() _wait_for_reconcile_log_mention(container, "ghost", deadline_s=30.0) # Stale runtime files swept. - r = _sh(container, "test -f /opt/data/profiles/ghost/gateway.pid") + r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/gateway.pid") assert r.returncode != 0, "stale gateway.pid survived restart" - r = _sh(container, "test -f /opt/data/profiles/ghost/processes.json") + r = docker_exec_sh(container, "test -f /opt/data/profiles/ghost/processes.json") assert r.returncode != 0, "stale processes.json survived restart" @@ -271,15 +264,15 @@ def test_live_gateway_autostarts_after_real_restart_without_manual_state_stamp( """ container = restart_container - _exec(container, "hermes", "profile", "create", "live").check_returncode() - r = _exec(container, "hermes", "-p", "live", "gateway", "start", timeout=60) + docker_exec(container, "hermes", "profile", "create", "live").check_returncode() + r = docker_exec(container, "hermes", "-p", "live", "gateway", "start", timeout=60) assert r.returncode == 0, f"gateway start failed: {r.stderr}" # Wait for the gateway to actually come up under supervision AND write # its own gateway_state=running (we do NOT stamp it ourselves). deadline = time.monotonic() + 20.0 while time.monotonic() < deadline: - r = _sh(container, "/command/s6-svstat /run/service/gateway-live") + r = docker_exec_sh(container, "/command/s6-svstat /run/service/gateway-live") if r.returncode == 0 and "up " in r.stdout: break time.sleep(0.5) @@ -290,7 +283,7 @@ def test_live_gateway_autostarts_after_real_restart_without_manual_state_stamp( deadline = time.monotonic() + 15.0 state = "" while time.monotonic() < deadline: - r = _sh( + r = docker_exec_sh( container, "cat /opt/data/profiles/live/gateway_state.json 2>/dev/null", ) @@ -322,7 +315,7 @@ def test_live_gateway_autostarts_after_real_restart_without_manual_state_stamp( assert _wait_for_path( container, "/run/service/gateway-live", kind="d", deadline_s=10.0, ), "slot not recreated after restart" - r = _sh(container, "test -f /run/service/gateway-live/down") + r = docker_exec_sh(container, "test -f /run/service/gateway-live/down") assert r.returncode != 0, ( "down marker present despite a live gateway being restarted — " "the signal-initiated shutdown wrongly persisted 'stopped' (#42675)" diff --git a/tests/docker/test_gateway_bootstrap_state.py b/tests/docker/test_gateway_bootstrap_state.py index 6dedd6014ee..4b25bafbef5 100644 --- a/tests/docker/test_gateway_bootstrap_state.py +++ b/tests/docker/test_gateway_bootstrap_state.py @@ -1,8 +1,6 @@ """Runtime smoke tests for Docker gateway_state.json bootstrap seeding. -Replaces the old text-assertion tests that extracted and ran the seed -block from stage2-hook.sh in a sandbox. These tests build the real image -and verify the actual runtime behavior: +Build the real image and verify the actual runtime behavior: 1. HERMES_GATEWAY_BOOTSTRAP_STATE=running on a fresh volume seeds gateway_state.json with running state @@ -14,9 +12,8 @@ from __future__ import annotations import json import subprocess -import time -from tests.docker.conftest import docker_exec, docker_exec_sh +from tests.docker.conftest import docker_exec, docker_exec_sh, wait_for_container_ready def _start_container( @@ -28,7 +25,7 @@ def _start_container( args.extend(["-e", e]) args.extend([built_image, "sleep", "infinity"]) subprocess.run(args, check=True, capture_output=True, timeout=60) - time.sleep(5) + wait_for_container_ready(name) return name @@ -97,7 +94,7 @@ def test_does_not_clobber_existing_state( # start and write its own state, but the stage2 [ ! -f ] guard runs # during cont-init (before any service starts), so the file must # still be our "stopped" state at this point. - time.sleep(3) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, "cat /opt/data/gateway_state.json", timeout=10, ) @@ -157,4 +154,4 @@ def test_non_running_value_ignored( subprocess.run( ["docker", "rm", "-f", name], capture_output=True, timeout=10, - ) + ) \ 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 91314d5b2f1..acdd01406a9 100644 --- a/tests/docker/test_gateway_run_supervised.py +++ b/tests/docker/test_gateway_run_supervised.py @@ -26,12 +26,8 @@ import time from tests.docker.conftest import docker_exec_sh -def _sh(container: str, command: str, timeout: int = 30): - return docker_exec_sh(container, command, timeout=timeout) - - def _svstat(container: str, slot: str = "gateway-default") -> str: - r = _sh(container, f"/command/s6-svstat /run/service/{slot}") + r = docker_exec_sh(container, f"/command/s6-svstat /run/service/{slot}") return r.stdout if r.returncode == 0 else "" @@ -98,7 +94,7 @@ def test_gateway_run_redirects_to_supervised( # The CMD process (PID under /init that the wrapper exec'd into) # should be sleeping, not the gateway. We grep `ps` for the # `sleep infinity` heartbeat. - r = _sh(container_name, "ps -eo pid,cmd | grep -v grep | grep 'sleep infinity'") + r = docker_exec_sh(container_name, "ps -eo pid,cmd | grep -v grep | grep 'sleep infinity'") assert r.returncode == 0 and "sleep infinity" in r.stdout, ( f"expected `sleep infinity` heartbeat process; got ps:\n{r.stdout}\n" f"stderr: {r.stderr}" @@ -175,7 +171,7 @@ def test_gateway_run_no_supervise_flag_preserves_legacy_behavior( if status == "running": # Gateway running in foreground — the CMD process should be # the gateway itself, NOT a sleep-infinity heartbeat. - r = _sh( + r = docker_exec_sh( container_name, "ps -eo pid,ppid,cmd | grep -v grep | awk '/main-wrapper.sh|rc.init top/ { wrapper_pid=$1 } " "$3==\"sleep\" && $4==\"infinity\" && $2==wrapper_pid { c++ } END { print c+0 }'", @@ -186,7 +182,7 @@ def test_gateway_run_no_supervise_flag_preserves_legacy_behavior( f"--no-supervise: expected NO `sleep infinity` parented to " f"the CMD wrapper (foreground gateway should be the CMD), " f"found {redirected_sleeps}. " - f"ps:\n{_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" + f"ps:\n{docker_exec_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" ) # The gateway-default s6 slot exists (the cont-init.d @@ -271,14 +267,14 @@ def test_supervised_gateway_does_not_recurse( # recursion guard fails, s6 would respawn fresh `gateway run` # processes on every cycle, leaving multiple Python-process # descendants under the gateway-default supervise tree. - r = _sh(container_name, "ps -eo pid,cmd | grep -v grep | grep -E 'python.*hermes.*gateway run' | wc -l") + r = docker_exec_sh(container_name, "ps -eo pid,cmd | grep -v grep | grep -E 'python.*hermes.*gateway run' | wc -l") assert r.returncode == 0 n = int(r.stdout.strip() or 0) assert n <= 1, ( f"expected at most one supervised python `hermes gateway run` " f"process (the legitimately-supervised gateway); found {n}. " f"Recursion guard may have failed. " - f"ps:\n{_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" + f"ps:\n{docker_exec_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" ) # Stronger positive assertion: there should be exactly one @@ -286,7 +282,7 @@ def test_supervised_gateway_does_not_recurse( # CMD process (PID 17 typically). The static `main-hermes` # service has its own `sleep infinity` child; THAT one is fine # and unrelated to our redirect. - r = _sh( + r = docker_exec_sh( container_name, # Find PID of the CMD process (main-wrapper.sh or its sh # parent), then count `sleep infinity` children. @@ -298,7 +294,7 @@ def test_supervised_gateway_does_not_recurse( assert redirected == 1, ( f"expected exactly one `sleep infinity` parented to the CMD " f"wrapper (the redirect heartbeat); found {redirected}. " - f"ps:\n{_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" + f"ps:\n{docker_exec_sh(container_name, 'ps -eo pid,ppid,cmd').stdout}" ) @@ -377,14 +373,14 @@ def test_supervised_gateway_stdout_reaches_docker_logs( "This means the `1` action directive in _render_log_run isn't " "forwarding stdout to /init. " f"docker logs (last 2000 chars):\n{combined[-2000:]}\n" - f"file contents:\n{_sh(container_name, 'cat /opt/data/logs/gateways/default/current').stdout}" + f"file contents:\n{docker_exec_sh(container_name, 'cat /opt/data/logs/gateways/default/current').stdout}" ) # Cross-check: the same banner must also be in the rotated log # file (we kept the file destination, just added stdout). The # file version has s6-log's ISO 8601 timestamp prefix; the # docker logs version is raw. - file_contents = _sh( + file_contents = docker_exec_sh( container_name, "cat /opt/data/logs/gateways/default/current", ).stdout assert "⚕" in file_contents or "Hermes Gateway Starting" in file_contents, ( diff --git a/tests/docker/test_home_override_scripts.py b/tests/docker/test_home_override_scripts.py index ca9989b6617..d800fac99e3 100644 --- a/tests/docker/test_home_override_scripts.py +++ b/tests/docker/test_home_override_scripts.py @@ -1,8 +1,6 @@ """Runtime smoke tests for Docker HOME overrides and script behavior. -Replaces the old text-assertion tests that grepped main-wrapper.sh, -dashboard/run, and stage2-hook.sh for string patterns. These tests -build the real image and verify the actual runtime behavior: +Build the real image and verify the actual runtime behavior: 1. main-wrapper preserves the Docker ``-w`` working directory 2. dashboard service resets HOME to /opt/data before privilege drop @@ -12,9 +10,8 @@ build the real image and verify the actual runtime behavior: 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, wait_for_container_ready def test_main_wrapper_preserves_docker_workdir( @@ -68,7 +65,7 @@ def test_dashboard_service_resets_home( check=True, capture_output=True, timeout=60, ) # Give s6 + dashboard service time to start. - time.sleep(5) + wait_for_container_ready(container_name) # Check if the dashboard process is running and inspect its HOME. r = docker_exec_sh( @@ -98,9 +95,8 @@ def test_dashboard_does_not_auto_insecure_from_host( """The dashboard MUST NOT auto-add ``--insecure`` based on HERMES_DASHBOARD_HOST. The auth gate is the authority now. - Regression: the old host-derived ``--insecure`` case-statement - disabled the OAuth auth gate on every non-loopback bind, exposing - every wildcard-subdomain agent dashboard publicly (early 2026). + The auth gate is the authority on whether non-loopback binds are + safe; ``--insecure`` must never be auto-derived from the bind host. We start the container with a non-loopback bind host and verify the dashboard process does NOT receive ``--insecure`` in its @@ -115,7 +111,7 @@ def test_dashboard_does_not_auto_insecure_from_host( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Check the dashboard process command line for --insecure. r = docker_exec_sh( @@ -149,7 +145,7 @@ def test_stage2_repairs_profiles_and_cron_ownership( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Create root-owned files in profiles/ and cron/ to simulate # docker exec (root) writes. @@ -183,7 +179,7 @@ def test_stage2_repairs_profiles_and_cron_ownership( check=True, capture_output=True, timeout=60, ) # Wait for stage2 to complete. - time.sleep(5) + wait_for_container_ready(container_name) # Verify files are now owned by hermes. r = docker_exec_sh( @@ -195,4 +191,4 @@ def test_stage2_repairs_profiles_and_cron_ownership( assert "hermes" in r.stdout, ( f"expected hermes-owned files after restart, got: {r.stdout!r} — " f"stage2 hook did not repair profiles/ and cron/ ownership" - ) + ) \ No newline at end of file diff --git a/tests/docker/test_immutable_install.py b/tests/docker/test_immutable_install.py index 5ffcf04ac5e..011cacee2ad 100644 --- a/tests/docker/test_immutable_install.py +++ b/tests/docker/test_immutable_install.py @@ -1,7 +1,6 @@ """Runtime smoke tests for Docker immutable install tree and install-method stamp. -Replaces the old text-assertion tests that grepped stage2-hook.sh for -string patterns. These tests build the real image and verify at runtime: +Build the real image and verify at runtime: 1. /opt/hermes is not writable by the hermes user (immutable install tree) 2. PYTHONDONTWRITEBYTECODE and HERMES_DISABLE_LAZY_INSTALLS are set @@ -12,9 +11,8 @@ string patterns. These tests build the real image and verify at runtime: 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, wait_for_container_ready def test_install_tree_not_writable_by_hermes( @@ -31,7 +29,7 @@ def test_install_tree_not_writable_by_hermes( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(3) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, @@ -68,7 +66,7 @@ def test_hermes_disable_lazy_installs_and_dont_write_bytecode( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(3) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, @@ -93,7 +91,7 @@ def test_install_method_stamp_is_code_scoped( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(3) + wait_for_container_ready(container_name) # Code-scoped stamp must exist and say "docker" r = docker_exec_sh( @@ -131,7 +129,7 @@ def test_stale_docker_stamp_in_home_is_healed_on_boot( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(3) + wait_for_container_ready(container_name) # Write a stale 'docker' stamp as root docker_exec( @@ -148,7 +146,7 @@ def test_stale_docker_stamp_in_home_is_healed_on_boot( ["docker", "restart", container_name], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # The stale stamp must be gone r = docker_exec_sh( @@ -160,4 +158,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_license_file_present.py b/tests/docker/test_license_file_present.py index f88d6b6aa15..fcb69b99315 100644 --- a/tests/docker/test_license_file_present.py +++ b/tests/docker/test_license_file_present.py @@ -1,10 +1,8 @@ """Runtime smoke test for Docker image license-file presence. -Replaces the old text-assertion test that grepped .dockerignore for -the LICENSE filename. This test builds the real image and verifies the -LICENSE file is actually present inside the container, which is the -behavioral outcome the old test was trying to guard (PEP 639 -license-files metadata must resolve inside the Docker image). +Build the real image and verify the LICENSE file is present inside the +container (PEP 639 license-files metadata must resolve inside the +Docker image). """ from __future__ import annotations @@ -15,9 +13,7 @@ def test_docker_image_contains_license_file(built_image: 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. The old test checked .dockerignore - text; this test verifies the actual runtime outcome: the file exists - at /opt/hermes/LICENSE inside the image. + build context must not exclude it. """ r = subprocess.run( ["docker", "run", "--rm", "--entrypoint", "test", @@ -27,4 +23,4 @@ def test_docker_image_contains_license_file(built_image: str) -> None: assert r.returncode == 0, ( f"LICENSE file not found at /opt/hermes/LICENSE inside the Docker " f"image: {r.stderr[-500:]}" - ) + ) \ 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 acbb5139956..e827e057375 100644 --- a/tests/docker/test_log_dir_seed.py +++ b/tests/docker/test_log_dir_seed.py @@ -1,8 +1,6 @@ """Runtime smoke test for Docker $HERMES_HOME/logs/gateways seeding. -Replaces the old text-assertion test that grepped stage2-hook.sh for -the mkdir -p seed block. This test builds the real image and verifies -the actual runtime outcome: logs/ and logs/gateways/ exist and are +Build the real image and verify logs/ and logs/gateways/ exist and are owned by the hermes user after container boot. Regression guard for #45258: if the first gateway log service runs in @@ -13,9 +11,8 @@ s6-log crash-loops on mkdir: Permission denied. from __future__ import annotations import subprocess -import time -from tests.docker.conftest import docker_exec_sh +from tests.docker.conftest import docker_exec_sh, wait_for_container_ready def test_logs_gateways_seeded_and_hermes_owned( @@ -27,7 +24,7 @@ def test_logs_gateways_seeded_and_hermes_owned( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Both directories must exist r = docker_exec_sh( @@ -54,4 +51,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_puid_pgid_remap.py b/tests/docker/test_puid_pgid_remap.py index 6598cb4304c..ab25a89b553 100644 --- a/tests/docker/test_puid_pgid_remap.py +++ b/tests/docker/test_puid_pgid_remap.py @@ -1,8 +1,6 @@ """Runtime smoke tests for Docker PUID/PGID and UID/GID remap. -Replaces the old text-assertion tests that extracted shell blocks from -stage2-hook.sh and ran them in sandboxes. These tests build the real -image and verify the actual runtime behavior: +Build the real image and verify the actual runtime behavior: 1. PUID/PGID env vars remap the hermes user UID/GID at boot 2. HERMES_UID/HERMES_GID take precedence over PUID/PGID aliases @@ -13,9 +11,8 @@ image and verify the actual runtime behavior: from __future__ import annotations import subprocess -import time -from tests.docker.conftest import docker_exec_sh +from tests.docker.conftest import docker_exec_sh, wait_for_container_ready def test_puid_pgid_remaps_hermes_user( @@ -29,7 +26,7 @@ def test_puid_pgid_remaps_hermes_user( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, @@ -63,7 +60,7 @@ def test_hermes_uid_gid_take_precedence_over_aliases( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) r = docker_exec_sh(container_name, "id -u hermes", timeout=10) assert r.stdout.strip() == "2000", ( @@ -87,7 +84,7 @@ def test_nas_low_uid_accepted( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) r = docker_exec_sh(container_name, "id -u hermes", timeout=10) assert r.stdout.strip() == "99", ( @@ -111,7 +108,7 @@ def test_remap_enables_data_volume_writes( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, @@ -120,4 +117,4 @@ def test_remap_enables_data_volume_writes( ) assert "WRITE_OK" in r.stdout, ( f"hermes user cannot write to /opt/data after remap: {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 68b8ca27193..9e490607003 100644 --- a/tests/docker/test_stage2_browser_discovery.py +++ b/tests/docker/test_stage2_browser_discovery.py @@ -1,42 +1,15 @@ """Runtime smoke tests for Docker stage2 browser executable discovery. -Replaces the old text-assertion tests that grepped stage2-hook.sh for -string patterns. These tests build the real image and verify the -chromium binary is actually discovered at boot — i.e. -``AGENT_BROWSER_EXECUTABLE_PATH`` is set, points to a real executable, -and is a browser binary (not a shared library picked up by a broad -``find | grep``). +Build the real image and verify the chromium binary is actually +discovered at boot: ``AGENT_BROWSER_EXECUTABLE_PATH`` is set, points to +a real executable, and is a browser binary (not a shared library picked +up by a broad ``find | grep``). """ from __future__ import annotations import subprocess -import time -from tests.docker.conftest import docker_exec_sh - - -def _wait_for_stage2(container: str, deadline_s: float = 30.0) -> None: - """Wait for stage2-hook to complete by polling for its completion log.""" - end = time.monotonic() + deadline_s - while time.monotonic() < end: - r = docker_exec_sh( - container, - "grep -q 'stage2.*Setup complete' /proc/1/fd/2 2>/dev/null " - "|| grep -rq 'stage2.*Setup complete' /run/s6 2>/dev/null " - "|| true", - timeout=5, - ) - # stage2 logs to stderr which s6 captures; check the container's - # s6 log surface instead. The simplest reliable signal is that - # the env var exists. - r = docker_exec_sh( - container, - "test -f /run/s6/container_environment/AGENT_BROWSER_EXECUTABLE_PATH", - timeout=5, - ) - if r.returncode == 0: - return - time.sleep(0.5) +from tests.docker.conftest import docker_exec_sh, wait_for_container_ready def test_stage2_discovers_chromium_binary( @@ -45,10 +18,10 @@ def test_stage2_discovers_chromium_binary( """The stage2 hook must discover the Playwright chromium binary and export AGENT_BROWSER_EXECUTABLE_PATH so the browser tool can find it. - Regression: the old ``find | grep -Ei 'chrome|chromium'`` picked up - shared libraries (libGLESv2.so etc.) that inherit the executable bit - from Playwright's tarball. The fix uses filename matching; this test - verifies the discovered binary is a real browser, not a .so. + The discovery uses filename matching, not a broad ``find | grep``: + shared libraries (libGLESv2.so etc.) inherit the executable bit from + 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, @@ -56,7 +29,7 @@ def test_stage2_discovers_chromium_binary( check=True, capture_output=True, timeout=60, ) # Give s6 + stage2-hook time to run. - time.sleep(5) + wait_for_container_ready(container_name) # AGENT_BROWSER_EXECUTABLE_PATH must be set via s6 container_environment. r = docker_exec_sh( @@ -81,7 +54,6 @@ def test_stage2_discovers_chromium_binary( ) # Must be a browser binary by basename — NOT a shared library. - # This is the runtime equivalent of the old "filename-matched" test. accepted_names = ( "chrome", "chromium", "chrome-headless-shell", "headless_shell", "chromium-browser", @@ -110,7 +82,7 @@ def test_stage2_browser_path_accessible_to_hermes_user( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) r = docker_exec_sh( container_name, @@ -120,4 +92,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_tini_compat_shim.py b/tests/docker/test_tini_compat_shim.py index df054a3a028..26e0e4d13c4 100644 --- a/tests/docker/test_tini_compat_shim.py +++ b/tests/docker/test_tini_compat_shim.py @@ -1,7 +1,6 @@ """Runtime smoke test for the Docker tini compatibility shim (#34192). -Replaces the old text-assertion tests that grepped the Dockerfile for -string patterns. These tests build the real image and verify: +Build the real image and verify: 1. /usr/bin/tini exists and is a symlink to /init (the compat shim for orchestration templates that still reference /usr/bin/tini) @@ -52,4 +51,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}" - ) + ) \ No newline at end of file diff --git a/tests/docker/test_toplevel_chown.py b/tests/docker/test_toplevel_chown.py index 2f256f61343..4d833325d14 100644 --- a/tests/docker/test_toplevel_chown.py +++ b/tests/docker/test_toplevel_chown.py @@ -1,8 +1,6 @@ """Runtime smoke tests for Docker top-level state-file ownership repair. -Replaces the old text-assertion tests that extracted the chown for-loop -from stage2-hook.sh and ran it in a sandbox. These tests build the real -image and verify the actual runtime behavior: +Build the real image and verify the actual runtime behavior: 1. Root-owned top-level state files (auth.json, state.db, gateway.lock, gateway_state.json) are chowned to hermes on boot @@ -12,9 +10,8 @@ image and verify the actual runtime behavior: 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, wait_for_container_ready # The files the stage2 hook should repair (mirrors the allowlist in @@ -31,7 +28,7 @@ def test_root_owned_state_files_repaired_on_boot( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Create root-owned state files to simulate docker exec (root) writes for f in ALLOWLISTED_FILES: @@ -54,7 +51,7 @@ def test_root_owned_state_files_repaired_on_boot( ["docker", "restart", container_name], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Verify files are now hermes-owned r = docker_exec_sh( @@ -79,7 +76,7 @@ def test_non_allowlisted_host_file_not_touched( built_image, "sleep", "infinity"], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # Create a non-allowlisted file as root docker_exec( @@ -97,7 +94,7 @@ def test_non_allowlisted_host_file_not_touched( ["docker", "restart", container_name], check=True, capture_output=True, timeout=60, ) - time.sleep(5) + wait_for_container_ready(container_name) # The file must STILL be root-owned (not touched by stage2) r = docker_exec_sh( @@ -108,4 +105,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_user_flag_guard.py b/tests/docker/test_user_flag_guard.py index 87971177398..4aa7eba6c7b 100644 --- a/tests/docker/test_user_flag_guard.py +++ b/tests/docker/test_user_flag_guard.py @@ -1,8 +1,6 @@ """Runtime smoke tests for Docker --user flag guard. -Replaces the old text-assertion tests that extracted the guard block -from stage2-hook.sh and main-wrapper.sh and ran it in a sandbox. These -tests build the real image and verify the actual runtime behavior: +Build the real image and verify the actual runtime behavior: 1. docker run --user is rejected with actionable guidance 2. Root start (default) works fine @@ -65,4 +63,4 @@ def test_user_pinned_to_hermes_uid_works( assert r.returncode == 0, ( f"--user 10000:10000 (hermes UID) was rejected: {r.stderr[-500:]}" ) - assert "OK" in r.stdout + assert "OK" in r.stdout \ No newline at end of file