fix(tests): forward HERMES_TEST_* knobs through the hermetic runner
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).
This commit is contained in:
@@ -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 \
|
||||
|
||||
Reference in New Issue
Block a user