From 11f5be3f84ea27a10061267263eb72c4e2d6538a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 30 May 2026 19:16:15 +0100 Subject: [PATCH] test(e2e): add managed runner script + skill/flake-triage docs (#1170) --- .agents/skills/e2e/SKILL.md | 38 ++++- apps/web/e2e/README.md | 27 +++- apps/web/e2e/scripts/run-e2e.sh | 245 ++++++++++++++++++++++++++++++++ apps/web/package.json | 3 + 4 files changed, 304 insertions(+), 9 deletions(-) create mode 100755 apps/web/e2e/scripts/run-e2e.sh diff --git a/.agents/skills/e2e/SKILL.md b/.agents/skills/e2e/SKILL.md index 5136a51b91..0ca72f8607 100644 --- a/.agents/skills/e2e/SKILL.md +++ b/.agents/skills/e2e/SKILL.md @@ -46,19 +46,37 @@ Each worker gets an isolated backend, frontend, database, and mock agent — no **Always run headless** (`make test-e2e`). Never use `--headed`, `e2e:headed`, or `test-e2e-headed` — headed mode requires a display and will fail in agent environments. +### Preferred: `pnpm e2e:run` (managed runner — builds, runs, tears down) + +`e2e/scripts/run-e2e.sh` handles the build, the run, and cleanup in one command. Use it instead of stitching the steps together. It auto-selects docker vs host, runs N shards concurrently, enforces strict WS accounting by default (matching CI), and never leaves root-owned artifacts behind. + ```bash -make test-e2e # all tests, headless +cd apps/web +pnpm e2e:run # auto: docker if daemon + CI image available, else host; builds first +pnpm e2e:run tests/task/my-test.spec.ts # single file (extra args pass through to Playwright) +pnpm e2e:run --shards 3 # 3 shards concurrently on this machine (isolated) +pnpm e2e:run --no-build -- --grep "task creation" # skip rebuild; forward flags after -- +pnpm e2e:docker # force the docker CI image (full isolation from a host dev instance) +pnpm e2e:clean # remove build/test artifacts, incl. root-owned ones from prior docker runs +``` + +The runner solves the sharp edges hand-rolling would hit: in docker it builds the CGO backend on the **host** and runs it in the runtime image (forward-compatible when the host glibc ≤ the image's — the usual case; it smoke-tests this and only falls back to the build image if the host is newer), builds the FE standalone on the host, pre-creates the standalone symlinks as relative links so in-container `global-setup` doesn't recreate them as root, and keeps Playwright output container-local. See `apps/web/e2e/README.md` → "the managed runner". + +### Raw commands (when you need fine control) + +```bash +make test-e2e # all tests, headless (host) cd apps && pnpm --filter @kandev/web e2e -- tests/task/my-test.spec.ts # single file cd apps && pnpm --filter @kandev/web e2e -- --grep "task creation" # by name ``` -**CRITICAL: E2E tests run against the production build** (`.next/standalone/`), not dev mode. After any frontend code change, you **must** rebuild before running tests: +**CRITICAL: E2E tests run against the production build** (`.next/standalone/`), not dev mode. After any frontend code change, you **must** rebuild before running tests (`pnpm e2e:run` does this for you): ```bash make build-web # ~30s, required after every frontend change ``` -Without this, tests run against stale code and failures are misleading. `make build-backend` is also required after Go changes. `make test-e2e` handles both automatically. +Without this, tests run against stale code and failures are misleading. `make build-backend` is also required after Go changes. `make test-e2e` and `pnpm e2e:run` handle both automatically. ## Writing a test @@ -253,6 +271,20 @@ cd apps/web && npx playwright test --config e2e/playwright.config.ts --shard=2/1 E2E tests run against the **production build** (`next build`), not dev mode. Always rebuild with `make build-web` (or `pnpm --filter @kandev/web build`) after code changes before running E2E tests locally. +### Flake triage: intrinsic race vs. contention + +A test that flakes under parallel/sharded load is one of two things — decide which **before** touching it: + +1. **Re-run it in a fresh, isolated container** (or at minimum a single fresh worker), `--retries=0`, a few reps: + ```bash + pnpm e2e:docker --no-build -- --repeat-each=4 --workers=1 --retries=0 tests/path.spec.ts:LINE + # or raw: pnpm exec playwright test --config e2e/playwright.config.ts --project=chromium --repeat-each=4 --workers=1 --retries=0 tests/path.spec.ts:LINE + ``` + - **Flakes alone (fails some reps, fast):** intrinsic race — fix it (condition-correct wait, fix the actual race; not a timeout bump). E.g. a `waitForRequest` that times out the full window means the request *never fired* (a click swallowed during hydration) — retry the action with `await expect(async () => { ... }).toPass()`, don't extend the timeout. + - **Passes clean AND fast alone (well under timeout):** contention, not a defect. The wait is correct; the test just starved for CPU/IO under load. No code/test fix applies. +2. **Signature of contention, not a code path:** two identical-config full runs giving *different* hard-fail counts (e.g. 0 vs 3). Same code + same config + different outcome ⇒ host oversubscription, not a bug. CI's isolated runners don't reproduce it; reduce local concurrency (2–3 shards, not 5+) for a clean signal. +3. **Caveat — don't flake-hunt with `--repeat-each` across many heavy specs in one long-lived worker.** It exhausts per-worker resources (agentctl port range, memory) over a long run and manufactures *false* failures unrelated to the test. Use **one fresh container per spec** instead. + ## Selector guidelines - **Prefer `data-testid` selectors** over text-based locators. Text content can change when UI is updated (e.g., hiding a badge), breaking tests that match by text. Use `getByTestId()` or `locator("[data-testid='...']")` for stable targeting. diff --git a/apps/web/e2e/README.md b/apps/web/e2e/README.md index 8035098ff7..49250fc1c0 100644 --- a/apps/web/e2e/README.md +++ b/apps/web/e2e/README.md @@ -2,12 +2,6 @@ Playwright-based end-to-end tests. Each Playwright worker spawns its own real Go backend (no mocks of internal services) on isolated ports and a real Next.js standalone frontend, then drives a real Chromium against them. -> **Source changes require a rebuild.** The harness spawns the production -> standalone server (`.next/standalone/web/server.js`), not `next dev`. After -> any frontend source edit you must run `pnpm --filter @kandev/web build` -> before `pnpm e2e`, otherwise the spec runs against stale bytecode and -> seemingly-correct code looks broken. - ## Project layout | Folder | What's in it | @@ -79,6 +73,27 @@ This project used to be named `docker`. It was renamed to `containers` once SSH Common flags: `--shard=1/4`, `-g "fragment of test name"`, `--repeat-each=3` (flake hunting). +### `pnpm e2e:run` — the managed runner (build + run + teardown) + +`e2e/scripts/run-e2e.sh` (aliased as `pnpm e2e:run`) handles the build, the run, and cleanup so you don't have to assemble the steps by hand. It **auto-selects docker vs host**, runs **N shards concurrently**, enforces strict WS accounting by default (`KANDEV_E2E_WS_ASSERT=1`, matching CI), and never leaves root-owned artifacts behind. + +```bash +pnpm e2e:run # auto: docker if the daemon + CI image are available, else host +pnpm e2e:run --shards 3 # 3 shards concurrently (isolated containers, or host procs with distinct ports) +pnpm e2e:run tests/chat/foo.spec.ts # extra args pass straight through to Playwright +pnpm e2e:run --host --no-build -- --grep "x" # force host, skip rebuild, forward flags after -- +pnpm e2e:docker # force the docker CI image (full isolation from a host dev instance) +pnpm e2e:clean # remove build/test artifacts, incl. root-owned ones from prior docker runs +``` + +Why a script instead of raw `docker run`: in docker mode it builds the CGO/`fts5` backend on the **host** and runs it in the runtime image — a host glibc that's the same or older than the image's (the usual case; the image tracks recent Ubuntu) is forward-compatible, so no build image is needed. The runner smoke-tests the binary in the image first and only falls back to the build image (`KANDEV_CI_BUILD_IMAGE`, default `…/kandev-ci:build-latest`) if your host glibc is _newer_. It also builds the FE standalone on the host, pre-creates the standalone symlinks as **relative** links (so in-container `global-setup` doesn't recreate them as root), points Playwright output at a container-local dir, and cleans up. Run `clean` if a previous bare `docker run` left root-owned files you can't delete. + +> **Office is always enabled in e2e (and dev); only prod gates it off.** `profiles.yaml` sets `KANDEV_FEATURES_OFFICE` to `"true"` in the `e2e` and `dev` profiles and `"false"` in `prod`, and the fixture selects the e2e profile via `KANDEV_E2E_MOCK=true`. So `tests/office/*` always have office routes registered — no manual env setup. +> +> This used to break when e2e was launched from a shell that had inherited `KANDEV_FEATURES_OFFICE=false` (e.g. from a host kandev backend running the prod profile): `profiles.ApplyProfile` only sets vars that are **unset** (so launchers/shells win — see `docs/decisions/0007-runtime-feature-flags.md`), and the fixture spreads `process.env` into the spawned backend, so the stale prod value won and 404'd every office spec. Fixed at the source: `sanitizeInheritedEnv` in `e2e/fixtures/backend.ts` strips all inherited `KANDEV_FEATURES_*` before spawn, so the e2e profile — not whatever the host exported — decides feature flags. No `unset` needed. + +> **Host oversubscription:** running ≥5 heavy shards concurrently on one machine (each = Go backend + Next standalone + Chromium + mock agent) starves CPU/IO and induces timing flakes that CI's isolated runners never see. Use 2–3 concurrent shards locally for a clean signal; see "flake triage" in the `/e2e` skill. + ## Backend isolation per worker Every Playwright worker gets: diff --git a/apps/web/e2e/scripts/run-e2e.sh b/apps/web/e2e/scripts/run-e2e.sh new file mode 100755 index 0000000000..ca28aabec2 --- /dev/null +++ b/apps/web/e2e/scripts/run-e2e.sh @@ -0,0 +1,245 @@ +#!/usr/bin/env bash +# +# Unified E2E runner. Builds what's needed, runs Playwright, and tears down +# cleanly — on the host or inside the kandev-ci docker image (the same one CI +# uses). Handles the sharp edges this would otherwise hit by hand: +# +# • Auto-selects docker vs host (docker if the daemon is up AND the runtime +# image is available; override with --docker / --host). +# • Docker mode builds the backend on the HOST and runs it in the runtime +# image — a host glibc that's the same or older than the image's (the usual +# case) is forward-compatible, so no build image is needed. It smoke-tests +# the binary in the image first and only falls back to the BUILD image +# (KANDEV_CI_BUILD_IMAGE) if the host glibc is newer. FE standalone builds +# on the host (pure JS at runtime). +# • Runs N shards concurrently (--shards N): N isolated containers in docker +# mode, or N host processes with distinct E2E_PORT_OFFSET + output dirs. +# • Never leaves root-owned junk in the repo: pre-creates the standalone +# symlinks host-side (so in-container global-setup doesn't create them as +# root) and points Playwright output at a container-local dir. `clean` +# removes any pre-existing root-owned artifacts via a throwaway container. +# +# Usage: +# run-e2e.sh [run] [options] [-- ] # default subcommand: run +# run-e2e.sh clean # remove build/test artifacts (incl. root-owned) +# +# Options: +# --docker | --host Force runner (default: auto-detect) +# --shards N Run the chromium project as N concurrent shards +# --no-build Skip the build step (reuse existing artifacts) +# --no-strict Don't set KANDEV_E2E_WS_ASSERT=1 (it's on by default here, matching CI) +# --project NAME Playwright project (default: chromium) +# Anything after `--` (or any unrecognized arg) is passed straight to Playwright, +# e.g. tests/chat/foo.spec.ts, --grep "name", --repeat-each=3, --workers=1. +# +# Env overrides: +# KANDEV_CI_BUILD_IMAGE (default: ghcr.io/kdlbs/kandev-ci:build-latest) +# KANDEV_CI_RUNTIME_IMAGE (default: kandev-ci:runtime-local, falls back to ghcr…:runtime-latest) +set -uo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../.." && pwd)" +WEB_DIR="$REPO_ROOT/apps/web" +BACKEND_DIR="$REPO_ROOT/apps/backend" +BUILD_IMAGE="${KANDEV_CI_BUILD_IMAGE:-ghcr.io/kdlbs/kandev-ci:build-latest}" +RUNTIME_IMAGE="${KANDEV_CI_RUNTIME_IMAGE:-}" + +log() { printf '\033[36m[e2e]\033[0m %s\n' "$*" >&2; } +die() { printf '\033[31m[e2e] %s\033[0m\n' "$*" >&2; exit 1; } + +docker_up() { command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1; } + +resolve_runtime_image() { + if [[ -n "$RUNTIME_IMAGE" ]]; then echo "$RUNTIME_IMAGE"; return; fi + if docker image inspect kandev-ci:runtime-local >/dev/null 2>&1; then + echo kandev-ci:runtime-local + else + echo ghcr.io/kdlbs/kandev-ci:runtime-latest + fi +} + +# --- clean: remove build/test artifacts, including root-owned ones from prior +# docker runs (host user can't rm root-owned files, so shell out to a container). +clean_artifacts() { + log "cleaning e2e/test-results, blob-report, standalone symlinks, and /tmp shard logs" + rm -rf "$WEB_DIR"/e2e/test-results* "$WEB_DIR"/e2e/blob-report 2>/dev/null + rm -f "$WEB_DIR"/.next/standalone/web/.next/static "$WEB_DIR"/.next/standalone/web/public 2>/dev/null + rm -f /tmp/e2e-host-shard-*.log /tmp/e2e-docker-shard-*.log 2>/dev/null + if docker_up; then + docker run --rm -v "$WEB_DIR":/web alpine sh -c \ + 'rm -rf /web/e2e/test-results* /web/e2e/blob-report; rm -f /web/.next/standalone/web/.next/static /web/.next/standalone/web/public' \ + 2>/dev/null || true + fi +} + +# Pre-create the standalone symlinks host-side so in-container global-setup +# finds them and skips creating them (which would make them root-owned). +# Use RELATIVE symlinks so they resolve both on the host and at the +# container's /work mount — an absolute host path is broken in-container, which +# makes global-setup's existsSync() return false and then EEXIST on recreate. +# Use python3 to compute the relative path portably (ln -r is GNU-only). +prelink_standalone() { + command -v python3 >/dev/null 2>&1 || die "python3 is required for prelink_standalone" + local sa="$WEB_DIR/.next/standalone/web" + [[ -d "$sa" ]] || return 0 + mkdir -p "$sa/.next" + local rel_static rel_public + rel_static=$(python3 -c "import os,sys; print(os.path.relpath(sys.argv[1],sys.argv[2]))" "$WEB_DIR/.next/static" "$sa/.next") + rel_public=$(python3 -c "import os,sys; print(os.path.relpath(sys.argv[1],sys.argv[2]))" "$WEB_DIR/public" "$sa") + [[ -e "$sa/.next/static" ]] || ln -sn "$rel_static" "$sa/.next/static" 2>/dev/null || true + [[ -e "$sa/public" ]] || ln -sn "$rel_public" "$sa/public" 2>/dev/null || true +} + +build_fe() { + log "building FE standalone (NEXT_PUBLIC_KANDEV_E2E_MOCK=true)" + ( cd "$REPO_ROOT/apps" && NEXT_PUBLIC_KANDEV_E2E_MOCK=true pnpm --filter @kandev/web build ) \ + || die "FE build failed" +} + +build_backend_host() { + log "building backend (host)" + make -C "$BACKEND_DIR" build >/dev/null || die "backend build failed" +} + +backend_runs_in() { # $1=image — does the host-built binary run there? (glibc check, ~2s) + # Mount to a distinct path, not /bin — overlaying /bin would shadow the + # container's system utilities (/bin/sh etc.). + docker run --rm -v "$REPO_ROOT/apps/backend/bin":/test-bin:ro "$1" \ + /test-bin/kandev --version >/dev/null 2>&1 +} + +build_backend_in_build_image() { + log "building backend in $BUILD_IMAGE (glibc-matched to runtime image)" + docker image inspect "$BUILD_IMAGE" >/dev/null 2>&1 \ + || die "host-built backend is incompatible with the runtime image's glibc and $BUILD_IMAGE isn't available. Pull it (docker pull $BUILD_IMAGE) or run with --host." + local uid_gid; uid_gid="$(id -u):$(id -g)" + docker run --rm -v "$REPO_ROOT":/work -w /work/apps/backend \ + -v kandev-gocache:/root/.cache/go-build -v kandev-gomod:/go/pkg/mod \ + "$BUILD_IMAGE" \ + bash -lc "git config --global --add safe.directory /work 2>/dev/null; make build VERSION=dev-docker && chown -R $uid_gid /work/apps/backend/bin" \ + >/dev/null || die "in-container backend build failed" +} + +# Build a backend binary that runs in the runtime image. Prefer the host build +# (fast, no 5GB build image): a binary linked against the host's glibc runs in +# the container as long as the host's glibc is the same or OLDER than the +# image's (the usual case — the runtime image tracks recent Ubuntu). Only when +# the host is NEWER do we fall back to the build image. +build_backend_for_docker() { # $1=runtime image + build_backend_host + if backend_runs_in "$1"; then + log "host-built backend runs in $1 (glibc OK)" + else + log "host-built backend won't run in $1 (host glibc newer than image?) — using $BUILD_IMAGE" + build_backend_in_build_image + fi +} + +# --- arg parsing +SUBCMD=run +MODE=auto +SHARDS=1 +DO_BUILD=1 +STRICT=1 +PROJECT=chromium +PW_ARGS=() +[[ "${1:-}" == clean ]] && { SUBCMD=clean; shift; } +[[ "${1:-}" == run ]] && shift +while [[ $# -gt 0 ]]; do + case "$1" in + --docker) MODE=docker ;; + --host) MODE=host ;; + --shards) [[ "${2:-}" =~ ^[1-9][0-9]*$ ]] || die "--shards must be a positive integer (got '${2:-}')"; SHARDS="$2"; shift ;; + --no-build) DO_BUILD=0 ;; + --no-strict) STRICT=0 ;; + --project) [[ "${2:-}" =~ ^[a-zA-Z0-9_-]+$ ]] || die "invalid --project name: '${2:-}'"; PROJECT="$2"; shift ;; + --) shift; PW_ARGS+=("$@"); break ;; + *) PW_ARGS+=("$1") ;; + esac + shift +done + +if [[ "$SUBCMD" == clean ]]; then clean_artifacts; log "clean done"; exit 0; fi + +# Resolve mode +if [[ "$MODE" == auto ]]; then + if docker_up && { [[ -n "$RUNTIME_IMAGE" ]] || docker image inspect kandev-ci:runtime-local >/dev/null 2>&1 || docker image inspect ghcr.io/kdlbs/kandev-ci:runtime-latest >/dev/null 2>&1; }; then + MODE=docker + else + MODE=host + fi +fi +log "mode=$MODE shards=$SHARDS project=$PROJECT strict=$STRICT" + +STRICT_ENV=() +[[ "$STRICT" == 1 ]] && STRICT_ENV=(KANDEV_E2E_WS_ASSERT=1) + +# --------------------------------------------------------------------------- +# HOST mode +# --------------------------------------------------------------------------- +run_host() { + [[ "$DO_BUILD" == 1 ]] && { build_backend_host; build_fe; } + prelink_standalone + local base_args=(playwright test --config e2e/playwright.config.ts --project="$PROJECT") + if [[ "$SHARDS" -le 1 ]]; then + ( cd "$WEB_DIR" && env ${STRICT_ENV[@]+"${STRICT_ENV[@]}"} pnpm exec "${base_args[@]}" ${PW_ARGS[@]+"${PW_ARGS[@]}"} ) + return $? + fi + log "running $SHARDS host shards (distinct E2E_PORT_OFFSET + output dirs)" + local pids=() rc=0 i + for ((i=1; i<=SHARDS; i++)); do + ( cd "$WEB_DIR" && env ${STRICT_ENV[@]+"${STRICT_ENV[@]}"} E2E_PORT_OFFSET=$((i-1)) \ + pnpm exec "${base_args[@]}" --shard="$i/$SHARDS" --output="e2e/test-results-shard-$i" ${PW_ARGS[@]+"${PW_ARGS[@]}"} \ + > "/tmp/e2e-host-shard-$i.log" 2>&1 ) & + pids+=("$!") + done + for p in "${pids[@]}"; do wait "$p" || rc=1; done + for ((i=1; i<=SHARDS; i++)); do + printf ' shard %s: %s\n' "$i" "$(grep -Eo '[0-9]+ (passed|failed|flaky)' "/tmp/e2e-host-shard-$i.log" | paste -sd' ')" >&2 + done + log "host shard logs: /tmp/e2e-host-shard-*.log" + return $rc +} + +# --------------------------------------------------------------------------- +# DOCKER mode (one container per shard; output stays container-local so the +# mounted repo never gets root-owned junk) +# --------------------------------------------------------------------------- +run_docker() { + local img; img="$(resolve_runtime_image)" + log "runtime image: $img" + clean_artifacts + [[ "$DO_BUILD" == 1 ]] && { build_backend_for_docker "$img"; build_fe; } + prelink_standalone + + local strict_flag=() + [[ "$STRICT" == 1 ]] && strict_flag=(-e KANDEV_E2E_WS_ASSERT=1) + local pw="git config --global --add safe.directory /work 2>/dev/null; cd /work/apps/web && pnpm exec playwright test --config e2e/playwright.config.ts --project=\"$PROJECT\"" + + run_one() { # $1=shard index (or 0 for unsharded) + local i="$1" shardflag="" out="/tmp/pw-out" + [[ "$i" != 0 ]] && shardflag="--shard=$i/$SHARDS" + docker run --rm --ipc=host \ + -v "$REPO_ROOT":/work -w /work/apps/web \ + ${strict_flag[@]+"${strict_flag[@]}"} \ + -e NODE_OPTIONS=--dns-result-order=ipv4first \ + -e PLAYWRIGHT_BROWSERS_PATH=/ms-playwright \ + "$img" \ + bash -lc "$pw $shardflag --output=$out --reporter=list \"\$@\"" e2e-runner ${PW_ARGS[@]+"${PW_ARGS[@]}"} + } + + if [[ "$SHARDS" -le 1 ]]; then run_one 0; return $?; fi + log "running $SHARDS isolated containers" + local pids=() rc=0 i + for ((i=1; i<=SHARDS; i++)); do + run_one "$i" > "/tmp/e2e-docker-shard-$i.log" 2>&1 & + pids+=("$!") + done + for p in "${pids[@]}"; do wait "$p" || rc=1; done + for ((i=1; i<=SHARDS; i++)); do + printf ' shard %s: %s\n' "$i" "$(grep -Eo '[0-9]+ (passed|failed|flaky)' "/tmp/e2e-docker-shard-$i.log" | paste -sd' ')" >&2 + done + log "docker shard logs: /tmp/e2e-docker-shard-*.log" + return $rc +} + +if [[ "$MODE" == docker ]]; then run_docker; else run_host; fi diff --git a/apps/web/package.json b/apps/web/package.json index 7de93f0753..9aaa0c233f 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -13,6 +13,9 @@ "typecheck": "tsc --noEmit", "test": "vitest run", "e2e": "playwright test --config e2e/playwright.config.ts", + "e2e:run": "e2e/scripts/run-e2e.sh", + "e2e:docker": "e2e/scripts/run-e2e.sh --docker", + "e2e:clean": "e2e/scripts/run-e2e.sh clean", "e2e:ui": "playwright test --config e2e/playwright.config.ts --ui", "e2e:headed": "playwright test --config e2e/playwright.config.ts --headed", "e2e:capture": "cross-env CAPTURE_PR_ASSETS=true playwright test --config e2e/playwright.config.ts && tsx e2e/scripts/convert-assets.ts",