mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-27 04:00:37 +00:00
harden(docker-smoke): catch !!ERROR/!!Exiting + tighten egg_info test
Two non-blocking observations from the review, both addressed:
1. The bad-pattern grep listed `error_exit` as a literal token, but the
`error_exit()` function at docker_init.bash:5-10 only echoes the
strings `"!! ERROR: "` and `"!! Exiting script (ID: $$)"` — the
function name itself never appears in container logs. So
`grep -E -i "error_exit"` would only fire on stray debug prints of
the name, not on actual failures. The other patterns
(`Failed to set (UID|GID|...)`, `groupmod: cannot`, etc.) DO catch
real error_exit output, so this wasn't a coverage gap — just a dead
token.
Add `!! ERROR` and `!! Exiting script` to the bad-pattern set so the
grep actually matches the function's output. Keep the literal
`error_exit` token as belt-and-suspenders for any debug/echo of the
name.
2. `test_docker_init_excludes_egg_info_during_staging` was a single
`assert "egg-info" in src` check. That passes if any occurrence
appears — including the explanatory comment block above the staging
logic. A maintainer removing the `--exclude='*.egg-info'` from
rsync but keeping the comment would slip past the test.
Tighten to:
- scope to the staging block (between `_stage_src=` and the
`uv pip install` line) so comments outside that window can't
satisfy the assertion;
- require the literal `--exclude='*.egg-info'` rsync flag;
- require `*.egg-info` in the block so the cp-fallback cleanup is
also pinned;
- additionally require `--exclude='build'`, `--exclude='dist'`,
`--exclude='__pycache__'` so all four setuptools-touchable
artifact dirs stay excluded.
Verified:
- tests/test_docker_docs_and_readonly.py — 11/11 pass.
- YAML parses cleanly via `yaml.safe_load`.
- Full suite: 5770 passed, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -195,7 +195,11 @@ jobs:
|
||||
# "errorless", URL paths) are improbable for these specific tokens.
|
||||
echo "::group::Startup log scan"
|
||||
LOGS="$(docker compose -p "$PROJECT" -f "$COMPOSE_FILE" logs --no-color)"
|
||||
BAD_PATTERNS='EROFS|Read-only file system|Traceback|PermissionError|error_exit|groupmod: cannot|usermod: cannot|Failed to set (UID|GID|owner|permissions|ownership)'
|
||||
# `!! ERROR` + `!! Exiting script` are the actual strings emitted by
|
||||
# docker_init.bash's error_exit() helper — the function name itself
|
||||
# never appears in output. The literal token `error_exit` is kept as
|
||||
# a belt-and-suspenders catch for any stray debug/echo of the name.
|
||||
BAD_PATTERNS='EROFS|Read-only file system|Traceback|PermissionError|!! ERROR|!! Exiting script|error_exit|groupmod: cannot|usermod: cannot|Failed to set (UID|GID|owner|permissions|ownership)'
|
||||
if echo "$LOGS" | grep -E -i "$BAD_PATTERNS"; then
|
||||
echo "❌ Startup logs contain known-bad pattern (see above)"
|
||||
exit 1
|
||||
|
||||
@@ -212,12 +212,43 @@ def test_docker_init_excludes_egg_info_during_staging():
|
||||
directories. setuptools takes a different (timestamp-update) code path
|
||||
when one is already present in the source tree, which itself hits the
|
||||
:ro mount through stat/utime calls. Excluding them keeps the build
|
||||
happily on the fresh-build code path."""
|
||||
happily on the fresh-build code path.
|
||||
|
||||
Tight assertions on both the rsync and cp-fallback paths — a loose
|
||||
`"egg-info" in src` check would pass on a stray comment mention, so
|
||||
we require the actual exclusion mechanics to be present.
|
||||
"""
|
||||
src = (REPO / "docker_init.bash").read_text(encoding="utf-8")
|
||||
# Either rsync --exclude= form or the cp-fallback's explicit rm -rf —
|
||||
# accept either provided the egg-info exclusion exists.
|
||||
assert "egg-info" in src, (
|
||||
"docker_init.bash staging must exclude *.egg-info from the copy "
|
||||
"to avoid setuptools' timestamp-update code path."
|
||||
|
||||
# Find the staging block: rsync invocation OR cp-fallback. Both must
|
||||
# actually exclude *.egg-info — a comment mention is not enough.
|
||||
stage_idx = src.index("_stage_src=")
|
||||
install_idx = src.index("uv pip install", stage_idx)
|
||||
stage_block = src[stage_idx:install_idx]
|
||||
|
||||
# Rsync path must carry --exclude='*.egg-info'.
|
||||
assert "--exclude='*.egg-info'" in stage_block, (
|
||||
"docker_init.bash rsync invocation must include "
|
||||
"--exclude='*.egg-info' so setuptools' timestamp-update code path "
|
||||
"doesn't fire (which itself hits the :ro mount through stat/utime)."
|
||||
)
|
||||
|
||||
# cp-fallback path must explicitly rm the egg-info dir after copy
|
||||
# (cp -a has no --exclude flag, so the cleanup happens post-copy).
|
||||
assert "*.egg-info" in stage_block, (
|
||||
"docker_init.bash cp-fallback must remove $_stage_src/*.egg-info "
|
||||
"after copy so the install runs on the fresh-build code path."
|
||||
)
|
||||
|
||||
# Both build and dist must also be excluded — setuptools touches them
|
||||
# under different conditions but the failure mode is identical.
|
||||
assert "--exclude='build'" in stage_block, (
|
||||
"rsync staging must --exclude='build' (setuptools build artifacts)."
|
||||
)
|
||||
assert "--exclude='dist'" in stage_block, (
|
||||
"rsync staging must --exclude='dist' (setuptools build artifacts)."
|
||||
)
|
||||
assert "--exclude='__pycache__'" in stage_block, (
|
||||
"rsync staging must --exclude='__pycache__' to keep the copy minimal."
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user