Files
ersatztv/scripts/e2e-ui.sh
T
timothy 0f1951340e fix(445,533): harden e2e-ui lifecycle + retire stale #533 decision record [decisions-edit]
Addresses the adversarial review round. Codex returned BLOCKED on the harness
lifecycle contract; the second (cold) reviewer independently flagged the same
trap-ordering defect, which is what made it credible.

**Trap installed AFTER boot -> signal mid-boot orphans the server.** The window
between `e2e-local.sh` returning and `trap ... EXIT INT TERM` had no handler, so
a Ctrl-C/TERM there (or the PID-parse bail-out) left dotnet holding the port —
violating the script's own "always kills the server" contract. The trap is now
installed BEFORE boot. When the PID is not yet known, cleanup falls back to
reaping whatever LISTENS on our port; attribution is sound because the
pre-flight proved that port free moments earlier.

**Signals were not re-raised.** A TERM landing beside a passing Playwright run
could exit 0, reporting success for a cancelled run. INT/TERM now clean up and
re-raise, so the wrapper dies BY the signal.

Verified, not assumed: SIGTERM mid-run -> wrapper exits 143 (128+15) and leaves
zero listeners and zero stray ErsatzTV processes.

`e2e-local.sh` backgrounded a MULTI-command subshell, so `$!` is the subshell —
not dotnet — wherever bash does not collapse it. Measured both ways:

  bash 3.2.57 (stock macOS /bin/bash), 2-command subshell : $! = SUBSHELL
  bash 5.3.15 (homebrew)                                   : $! = leaf
  with `exec` (both versions)                              : $! = leaf

Consequence on stock-macOS bash: every PID-based kill/liveness check targeted
the wrong process, the escalation silently no-opped, and the server leaked. Fixed
at the source with `exec`, which benefits all consumers (the CI step's trap and
the e2e-functional.sh pairing), not just e2e-ui.sh.

My first attempt to test this was WRONG and would have cleared the finding: a
single-command subshell is collapsed on both versions. Only the 2-command form
reproduces it.

`testing.e2e-local-fresh-config-dir` was still `status: active` asserting the bug
'wait for either line' option today", "widening the probe ... is tracked as #533".
`READY_LINE` existed ONLY in that record; nowhere in code.

Amended rather than superseded: the operative rule (use a fresh config dir) is
unchanged and still correct — only its RATIONALE moved from "the probe hangs" to
"state bleed". Heading deliberately left alone: the validator matches records by
`## HEADING`, so renaming fails CI as an "unlogged removal"; an explicit note now
tells the reader the heading is historical.

- Boot-failure diagnostics were lost: `OUT="$(...)"` aborts under `set -e` before
  the print. Now `if ! OUT=$(...)` so the output is shown.
- Playwright failures surfaced no server-side evidence (nothing uploads the
  traces in web/e2e/.output). The server log tail is now printed on failure.
- `lsof -ti :PORT` also matched outbound/TIME_WAIT sockets -> false positives.
  Now `-sTCP:LISTEN`. Documented honestly that the CI image ships no lsof, so
  the pre-flight is a local-developer guard only.
- Predictable `/tmp/etv-pw-probe.$$` -> `mktemp` (symlink-redirect on a shared host).
- Suggested escape-hatch port was 8411, which is scripts/security-scan.sh's
  default; 8409/8410 are prod/ersatztv-test on jazz. Now suggests 8419 and names
  the conflicts.
- boot-gate.spec.ts comment overclaimed: after logout the browser holds NO
  cookie, so that assertion cannot prove stamp rotation (the curl harness does,
  by replaying the same cookie). Comment corrected to what it actually proves.

Deliberately NOT changed: `ci-image-pin`'s regex accepts any hex length rather
than the exact 7 chars ci-image.yml publishes (pre-existing guard weakness, not
introduced here — filed as a follow-up rather than widened in this PR).

Also trimmed the two records I had bloated: the active decisions corpus went over
its 5600-line budget as a result of this PR (baseline on main was under), so the
overflow was mine to pay down, not to pass on.

- SIGTERM mid-run: exit 143, no orphan listener/process
- curl harness unaffected by the `exec` change: 45/45 PASS
- UI-E2E 2x clean; typecheck + lint clean; decisions validator OK, no size warning

Refs #445 #533
2026-07-25 14:04:52 +02:00

165 lines
8.1 KiB
Bash
Executable File

#!/usr/bin/env bash
# scripts/e2e-ui.sh — boot a FRESH ErsatzTV instance and run the UI-interactive Playwright flows
# (ersatztv#445). The curl counterpart is scripts/e2e-functional.sh; see docs/e2e-local.md for both.
#
# Usage:
# scripts/e2e-ui.sh [CONFIG_DIR] # extra args after CONFIG_DIR are passed to `playwright test`
#
# CONFIG_DIR defaults to a fresh `mktemp -d`. A FRESH dir is REQUIRED, not merely preferred: the first
# spec asserts the Setup (first-run admin claim) gate, which only appears while the server has no
# local admin. Point this at a reused config dir and that spec fails by design.
#
# Unlike e2e-local.sh, this script OWNS the instance lifecycle: it boots the server, runs the specs,
# and always kills the server on the way out (success, failure, or interrupt). Exit status is
# Playwright's, so this is safe to use directly as a CI gate.
#
# Assumes `dotnet build` + `cd web && npm ci && npm run build` have already run — same contract as
# e2e-local.sh, which this delegates the boot to.
set -euo pipefail
REPO_ROOT="$(git rev-parse --show-toplevel)"
PORT="${ETV_UI_PORT:-8409}"
CONFIG_DIR="${1:-$(mktemp -d)}"
shift || true
# The Chromium build Playwright drives is baked into the CI toolchain image at this path
# (docker/ci/Dockerfile); locally it lands in the default per-user cache. Only export the shared
# path when it actually exists, so a local run keeps using the user cache.
if [ -z "${PLAYWRIGHT_BROWSERS_PATH:-}" ] && [ -d /ms-playwright ]; then
export PLAYWRIGHT_BROWSERS_PATH=/ms-playwright
fi
if [ ! -d "$REPO_ROOT/web/node_modules/@playwright/test" ]; then
echo "error: web/node_modules/@playwright/test is missing — run 'cd web && npm ci' first" >&2
exit 1
fi
# Version-drift guard: actually LAUNCH the browser and close it. Playwright pins a browser REVISION
# per package version, so bumping web/package.json's @playwright/test pin (e.g. by Renovate) without
# rebuilding the CI toolchain image leaves no usable browser. Failing here with an actionable message
# beats Playwright's bare "Executable doesn't exist at ..." mid-suite.
#
# Probing by LAUNCH rather than by path is deliberate: the CI image bakes only
# `chromium-headless-shell` (267M vs 656M for full chromium), and `chromium.executablePath()` reports
# the FULL chromium path, which does not exist there — a path check would fail on a perfectly good
# image. Launching exercises exactly what the specs do.
PW_VERSION="$(cd "$REPO_ROOT/web" && node -p "require('@playwright/test/package.json').version")"
# mktemp, not a predictable /tmp/...$$ path: on a shared host a pre-created symlink at a guessable
# name would redirect this write.
PW_PROBE_ERR="$(mktemp)"
if ! (cd "$REPO_ROOT/web" && node -e "require('@playwright/test').chromium.launch().then(b=>b.close())") 2>"$PW_PROBE_ERR"; then
echo "error: could not launch headless chromium for @playwright/test ${PW_VERSION}:" >&2
sed 's/^/ /' "$PW_PROBE_ERR" >&2 || true
rm -f "$PW_PROBE_ERR"
echo " In CI this means the browser baked into the toolchain image no longer matches" >&2
echo " web/package.json's @playwright/test pin (${PW_VERSION}): bump PLAYWRIGHT_VERSION in" >&2
echo " docker/ci/Dockerfile to match, let ci-image.yml publish the new :<sha>, then update" >&2
echo " all five container pins in docker-build.yml (docs/ci-cd.md -> 'CI toolchain image')." >&2
echo " Locally: cd web && npx playwright install chromium" >&2
exit 1
fi
rm -f "$PW_PROBE_ERR"
# Pre-flight: fail fast and clearly if the port is already taken. Otherwise the symptom is
# e2e-local.sh's 120s readiness timeout, which reads like a broken build rather than "something else
# is already listening" — a real hazard when several sessions share this machine.
#
# `-sTCP:LISTEN` matters: a bare `lsof -ti :PORT` also matches outbound/ESTABLISHED/TIME_WAIT
# sockets touching that port number, which would abort on a false positive.
#
# This is a LOCAL-developer guard. The CI toolchain image ships no `lsof`, so the check silently
# no-ops there — acceptable, because each CI job gets a fresh container where nothing else is bound.
if command -v lsof >/dev/null 2>&1 && [ -n "$(lsof -ti :"$PORT" -sTCP:LISTEN 2>/dev/null || true)" ]; then
echo "error: port $PORT already has a listener: PID(s) $(lsof -ti :"$PORT" -sTCP:LISTEN | tr '\n' ' ')" >&2
echo " Another instance (possibly another session's) is listening. Stop it, or re-run with" >&2
echo " a free port, e.g. ETV_UI_PORT=8419 scripts/e2e-ui.sh" >&2
echo " (8409/8410 are prod + ersatztv-test on jazz; 8411 is scripts/security-scan.sh.)" >&2
echo " Do NOT blanket-kill 'dotnet ErsatzTV.dll' — that can reap another session's server." >&2
exit 1
fi
# --- Server lifecycle -------------------------------------------------------------------------
#
# The trap is installed BEFORE the server is booted, not after. A signal (or any early exit)
# arriving while `e2e-local.sh` is still starting the app would otherwise leave a live server bound
# to the port with no handler to reap it — precisely the "stray process" failure this script exists
# to prevent.
#
# `wait "$PID"` cannot be used to confirm the kill: e2e-local.sh launches the app inside its own
# subshell and then exits, so the process is NOT a child of THIS shell — `wait` fails instantly
# ("not a child of this shell") and returns before the port is released. Poll `kill -0` instead,
# then escalate to SIGKILL.
SERVER_PID=""
cleanup() {
if [ -n "$SERVER_PID" ]; then
kill "$SERVER_PID" 2>/dev/null || true
for _ in $(seq 1 50); do # up to ~5s for a graceful shutdown
if ! kill -0 "$SERVER_PID" 2>/dev/null; then
SERVER_PID=""
return 0
fi
sleep 0.1
done
echo "warning: server $SERVER_PID did not exit after SIGTERM; sending SIGKILL" >&2
kill -9 "$SERVER_PID" 2>/dev/null || true
SERVER_PID=""
return 0
fi
# PID unknown (killed mid-boot, or e2e-local.sh's output could not be parsed). Fall back to
# whatever is listening on our port. Attribution is sound: the pre-flight above proved the port
# was FREE immediately before we booted, so a listener now can only be the server we started.
if command -v lsof >/dev/null 2>&1; then
local stray
stray="$(lsof -ti :"$PORT" -sTCP:LISTEN 2>/dev/null || true)"
if [ -n "$stray" ]; then
echo "warning: reaping server on port $PORT with unknown PID ($stray)" >&2
kill $stray 2>/dev/null || true
fi
fi
}
# Re-raise INT/TERM after cleaning up, so the wrapper dies BY the signal rather than exiting 0.
# Without this a TERM landing next to a passing Playwright run reports success for a cancelled run.
on_signal() {
cleanup
trap - EXIT "$1"
kill -"$1" $$
}
trap cleanup EXIT
trap 'on_signal INT' INT
trap 'on_signal TERM' TERM
echo "Booting instance for UI-E2E (config: $CONFIG_DIR, port: $PORT)..."
# `if ! OUT=$(...)` rather than a bare assignment: under `set -e` a failing assignment aborts the
# script before the diagnostics below can be printed, losing the log path.
if ! OUT="$(ETV_UI_PORT="$PORT" "$REPO_ROOT/scripts/e2e-local.sh" "$CONFIG_DIR")"; then
echo "error: e2e-local.sh failed to boot the instance. Its output:" >&2
printf '%s\n' "$OUT" >&2
exit 1
fi
printf '%s\n' "$OUT"
SERVER_PID="$(printf '%s\n' "$OUT" | awk -F= '/^PID=/{print $2}')"
SERVER_LOG="$(printf '%s\n' "$OUT" | awk -F= '/^LOG=/{print $2}')"
if [ -z "$SERVER_PID" ]; then
echo "error: could not determine the server PID from e2e-local.sh output" >&2
exit 1 # the EXIT trap's port-based fallback reaps the server we just started
fi
echo "Running Playwright UI-E2E specs against http://localhost:${PORT} ..."
cd "$REPO_ROOT/web"
if ! ETV_BASE_URL="http://localhost:${PORT}" npx --no-install playwright test "$@"; then
status=$?
# No workflow in this repo uploads artifacts, so traces/screenshots in web/e2e/.output never leave
# the runner. The server log is the only server-side evidence a CI failure would otherwise lack.
if [ -n "${SERVER_LOG:-}" ] && [ -f "$SERVER_LOG" ]; then
echo "--- server log tail ($SERVER_LOG) ---" >&2
tail -n 40 "$SERVER_LOG" >&2 || true
echo "--- end server log ---" >&2
fi
exit "$status"
fi