From ca120413fc092d0aaa4bc5d6140953dad21763ae Mon Sep 17 00:00:00 2001 From: kshitij Date: Thu, 6 Aug 2026 04:41:35 +0530 Subject: [PATCH] fix(tests): forward HERMES_TEST_* knobs through the hermetic runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/run_tests.sh runs the suite under `env -i` with an explicit allowlist. The runner's own documented environment knobs were never on that list, so all of them were silent no-ops for anyone invoking the canonical wrapper: * HERMES_TEST_WORKERS / PATHS / FILE_TIMEOUT / FILE_RETRIES / SLICE are read by run_tests_parallel.py at argparse-default time — inside the stripped environment. * HERMES_TEST_IMAGE is read by tests/docker/conftest.py to skip its session-scoped docker build. The HERMES_TEST_IMAGE strip is the expensive one, and it's been biting CI since docker.yml switched from bare pytest to run_tests.sh (f0cb04921): the workflow sets HERMES_TEST_IMAGE to the image the build step just loaded, the wrapper drops it, and every per-file pytest subprocess falls back to building hermes-agent-harness:latest itself. The job log timing shows it plainly — the first 8 files dispatched (the LPT-heaviest) all report 248-297s, which is them waiting out the concurrent initial `docker build` (~4 min on a cold local builder); every file dispatched after that rides the layer cache and finishes in 4-38s (e.g. test_dump_build_sha.py, a single `docker run --entrypoint cat`, reported 256.6s). ~4 min of pure waste per docker job, on both arches — and the tests exercised a locally-rebuilt image WITHOUT the HERMES_GIT_SHA build-arg the workflow bakes in, not the artifact being shipped. Fix: forward the six knobs the same way the Windows location vars are forwarded (66c4c9c0b) — an explicit compute-before-drop allowlist, each var only when set, so POSIX runs without them are byte-for-byte unchanged and the 'no credential can leak' property stays auditable. Verified empirically via a probe test through the wrapper: before: HERMES_TEST_IMAGE=None inside the subprocess after: HERMES_TEST_IMAGE='sentinel-image', HERMES_TEST_FILE_TIMEOUT forwarded, HERMES_TEST_WORKERS=3 yields '(3 workers)' in the summary, and an unrelated SOME_SECRET stays stripped. bash -n clean; shellcheck: no new findings (SC2046 on the pre-existing compileall line predates this change). --- scripts/run_tests.sh | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index 0db80ffb13..4445d6d542 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -123,6 +123,32 @@ for _win_var in USERPROFILE HOMEDRIVE HOMEPATH LOCALAPPDATA APPDATA SYSTEMROOT T fi done +# ── Test-runner knobs (computed before we drop env) ──────────────────────── +# The runner's own documented environment knobs must survive the hermetic +# `env -i` below, or they are silent no-ops for anyone invoking this script: +# +# * HERMES_TEST_WORKERS / PATHS / FILE_TIMEOUT / FILE_RETRIES / SLICE are +# read by run_tests_parallel.py at argparse-default time — inside the +# stripped environment. +# * HERMES_TEST_IMAGE is read by tests/docker/conftest.py to skip its +# session-scoped `docker build`. CI's docker.yml sets it to the image +# the build step just loaded; stripping it made every per-file pytest +# subprocess rebuild the 5GB image from a cold builder cache instead +# (~4 min per worker per run, and the rebuilt image lacked the +# HERMES_GIT_SHA build-arg the workflow bakes in). +# +# These are test-infrastructure knobs, not credentials — same class as the +# HERMES_RUN_SLOW_PET_TESTS / HERMES_E2E_BROWSER opt-ins already forwarded. +# Keep this an explicit allowlist (no HERMES_TEST_* glob) so the "no +# credential can leak" property stays auditable at a glance. +TEST_ENV=() +for _test_var in HERMES_TEST_IMAGE HERMES_TEST_WORKERS HERMES_TEST_PATHS \ + HERMES_TEST_FILE_TIMEOUT HERMES_TEST_FILE_RETRIES HERMES_TEST_SLICE; do + if [ -n "${!_test_var:-}" ]; then + TEST_ENV+=("$_test_var=${!_test_var}") + fi +done + # ── Run in hermetic env ────────────────────────────────────────────────────── # env -i: start with empty environment, opt-in only what we need. # No credential var can leak — you'd have to explicitly add it here. @@ -144,6 +170,7 @@ exec env -i \ PATH="$PATH" \ HOME="$HOME" \ ${WIN_ENV[@]+"${WIN_ENV[@]}"} \ + ${TEST_ENV[@]+"${TEST_ENV[@]}"} \ TZ=UTC \ LANG=C.UTF-8 \ LC_ALL=C.UTF-8 \