Post-merge review of #214 (by @aptracebloc) surfaced real defects in the install-journey test harness. All verified against the code on develop.
Critical
guard() in scripts/tests/e2e-journey.sh captures local rc=$? inside the then branch of if ! timeout ..., so rc is the negated status (always 0). Every assertion runs under guard, so set -e never trips and the job prints E2E JOURNEY PASS even when steps fail or hang. Reproduced empirically (exit 3 and a 124 hang both return 0). Leg 2 asserts nothing today.
High
scripts/tests/path-persist.sh assert_cell uses bash -c/zsh -c for the "nonlogin" cell. Non-interactive shells read no rc files (verified: only bash -ic reads .bashrc), so the "customer opens a new terminal" case is never reproduced.
- Container runs as root →
install.sh writes to /usr/local/bin (already on default PATH), so the ~/.local/bin rc-persistence path (the cli#61 class) is never exercised. Cells pass trivially on both pre-fix and fixed installers — the advertised red/green proof cannot happen.
Medium
- No
timeout-minutes on the path-persist and e2e-journey jobs in .github/workflows/installer-tests.yaml (every other job has one).
pull_request trigger uses default types, so adding the e2e label does not start a run until the next push. Needs types: [..., labeled].
Low (docs drift)
Plan
- Fix
guard() (one line) + add a negative-control assertion so the class cannot regress silently.
- Rework path-persist to use interactive shells and a non-root / non-default-prefix cell so it can distinguish installers.
- Add
timeout-minutes to both jobs; add labeled to the PR trigger types.
Follow-up to #214.
Post-merge review of #214 (by @aptracebloc) surfaced real defects in the install-journey test harness. All verified against the code on develop.
Critical
guard()inscripts/tests/e2e-journey.shcaptureslocal rc=$?inside thethenbranch ofif ! timeout ..., sorcis the negated status (always 0). Every assertion runs underguard, soset -enever trips and the job printsE2E JOURNEY PASSeven when steps fail or hang. Reproduced empirically (exit 3 and a 124 hang both return 0). Leg 2 asserts nothing today.High
scripts/tests/path-persist.shassert_cellusesbash -c/zsh -cfor the "nonlogin" cell. Non-interactive shells read no rc files (verified: onlybash -icreads.bashrc), so the "customer opens a new terminal" case is never reproduced.install.shwrites to/usr/local/bin(already on default PATH), so the~/.local/binrc-persistence path (the cli#61 class) is never exercised. Cells pass trivially on both pre-fix and fixed installers — the advertised red/green proof cannot happen.Medium
timeout-minuteson thepath-persistande2e-journeyjobs in.github/workflows/installer-tests.yaml(every other job has one).pull_requesttrigger uses default types, so adding thee2elabel does not start a run until the next push. Needstypes: [..., labeled].Low (docs drift)
TRACEBLOC_CLI_REF, "nightly" is actually weekly + every push).Plan
guard()(one line) + add a negative-control assertion so the class cannot regress silently.timeout-minutesto both jobs; addlabeledto the PR trigger types.Follow-up to #214.