From 1dd6d4bc9d6c887127672cbcd831da08896ffa5b Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 9 Sep 2026 14:52:07 +0800 Subject: [PATCH] fix(e2e): parse run-e2e.sh options after a leading -- and bound the docker info probe (#3518) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- .agents/skills/e2e/SKILL.md | 2 +- apps/web/e2e/scripts/run-e2e.sh | 61 ++++++- apps/web/e2e/scripts/run-e2e.test.ts | 254 +++++++++++++++++++++++++++ docs/public/testing.md | 2 + 4 files changed, 315 insertions(+), 4 deletions(-) diff --git a/.agents/skills/e2e/SKILL.md b/.agents/skills/e2e/SKILL.md index 73d8161421..bdf90e346f 100644 --- a/.agents/skills/e2e/SKILL.md +++ b/.agents/skills/e2e/SKILL.md @@ -79,7 +79,7 @@ pnpm e2e:run # auto: docker if daemon + CI ima pnpm e2e:run tests/task/my-test.spec.ts # single file (extra args pass through to Playwright) pnpm e2e:run tests/path/spec.ts -- --grep "exact test name" # exact CI failure with a fresh build 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:run --no-build -- --grep "task creation" # runner options before --; Playwright options after pnpm e2e:run --no-build --project mobile-chrome tests/layout/mobile-spa-resilience.spec.ts 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 diff --git a/apps/web/e2e/scripts/run-e2e.sh b/apps/web/e2e/scripts/run-e2e.sh index ab103150c2..ebefac5b4b 100755 --- a/apps/web/e2e/scripts/run-e2e.sh +++ b/apps/web/e2e/scripts/run-e2e.sh @@ -22,8 +22,16 @@ # container. # # Usage: -# run-e2e.sh [run] [options] [-- ] # default subcommand: run -# run-e2e.sh clean # remove build/test artifacts (incl. root-owned) +# Direct: run-e2e.sh [run] [options] [-- ] # default subcommand: run +# run-e2e.sh clean # remove build/test artifacts (incl. root-owned) +# Via pnpm: pnpm e2e:run [options] [-- ] +# pnpm/npm forward a caller-supplied `--` verbatim, so `pnpm e2e:run -- --host` +# reaches this script as `-- --host`. The package entry point drops that leading +# artifact before option parsing; direct script calls preserve `--` as the +# documented Playwright separator. For package calls, write script options +# directly after `e2e:run` (`pnpm e2e:run --host --no-build -- --grep foo`). +# A `--` anywhere else still ends option parsing and forwards everything after it +# straight to Playwright. # # Options: # --docker | --host Force runner (default: auto-detect) @@ -38,6 +46,8 @@ # 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) # KANDEV_E2E_ALLOW_UNSAFE_PARALLELISM=1 bypass local shard/worker guards for experiments +# KANDEV_E2E_DOCKER_PROBE_TIMEOUT (default: 10) seconds to wait for `docker info` +# before treating Docker as unavailable in auto/clean mode set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -48,11 +58,50 @@ 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:-}" +DOCKER_PROBE_TIMEOUT="${KANDEV_E2E_DOCKER_PROBE_TIMEOUT:-10}" 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; } +# Feeds arithmetic in docker_up; validated to reject fractional, suffixed, and +# whitespace-padded values. 0 is accepted — it means "skip the probe, treat +# Docker as unavailable". No leading zeros: bash arithmetic reads them as octal, +# so "08" is a parse error and "010" silently means 8. +[[ "$DOCKER_PROBE_TIMEOUT" =~ ^(0|[1-9][0-9]*)$ ]] \ + || die "KANDEV_E2E_DOCKER_PROBE_TIMEOUT must be a non-negative integer (got '$DOCKER_PROBE_TIMEOUT')" + +# Bounded in pure bash (no `timeout`/`gtimeout` dependency — bare macOS has +# neither). Backgrounds the probe and kills it if it outlives the budget, so a +# hung daemon degrades to host mode with a visible reason instead of hanging +# the whole runner forever. +docker_up() { + command -v docker >/dev/null 2>&1 || return 1 + if (( DOCKER_PROBE_TIMEOUT == 0 )); then + log "docker info did not respond within 0s; treating Docker as unavailable (pass --host to skip this probe)" + return 1 + fi + local had_monitor=0 + case "$-" in + *m*) had_monitor=1 ;; + esac + set -m + docker info >/dev/null 2>&1 & + local pid=$! max_ticks=$(( DOCKER_PROBE_TIMEOUT * 10 )) tick=0 + [[ "$had_monitor" -eq 1 ]] || set +m + while kill -0 "$pid" 2>/dev/null; do + if (( tick >= max_ticks )); then + # SIGKILL, not SIGTERM: a wedged docker client can be blocked in a way + # that doesn't honor SIGTERM, and `wait` below blocks until it exits. + kill -9 -- "-$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + log "docker info did not respond within ${DOCKER_PROBE_TIMEOUT}s; treating Docker as unavailable (pass --host to skip this probe)" + return 1 + fi + sleep 0.1 + tick=$(( tick + 1 )) + done + wait "$pid" +} is_container_project() { [[ "$PROJECT" == containers || "$PROJECT" == kubernetes-compat ]]; } resolve_runtime_image() { @@ -171,6 +220,12 @@ DO_BUILD=1 STRICT=1 PROJECT=chromium PW_ARGS=() +# pnpm/npm forward the caller's `--` verbatim, so `pnpm e2e:run -- --host` +# reaches this script as `-- --host`. Only the package entry point drops that +# artifact. Direct calls keep the documented `-- ` separator. +if [[ "${npm_lifecycle_event:-}" == e2e:run && "${1:-}" == -- ]]; then + shift +fi [[ "${1:-}" == clean ]] && { SUBCMD=clean; shift; } [[ "${1:-}" == run ]] && shift while [[ $# -gt 0 ]]; do diff --git a/apps/web/e2e/scripts/run-e2e.test.ts b/apps/web/e2e/scripts/run-e2e.test.ts index f6487c5e8c..0f1c832079 100644 --- a/apps/web/e2e/scripts/run-e2e.test.ts +++ b/apps/web/e2e/scripts/run-e2e.test.ts @@ -17,6 +17,7 @@ afterEach(() => { function runnerEnv(binDir: string, extra: Record = {}): NodeJS.ProcessEnv { const env = { ...process.env, + npm_lifecycle_event: "e2e:run", KANDEV_E2E_ALLOW_UNSAFE_PARALLELISM: "", ...extra, PATH: `${binDir}:${process.env.PATH ?? ""}`, @@ -27,6 +28,28 @@ function runnerEnv(binDir: string, extra: Record = {}): NodeJS.P return env; } +function fakeExecutable(binDir: string, name: string, contents: string): void { + const executablePath = path.join(binDir, name); + fs.writeFileSync(executablePath, contents); + fs.chmodSync(executablePath, 0o755); +} + +function isRunningProcess(pid: number): boolean { + const result = spawnSync("ps", ["-o", "stat=", "-p", String(pid)], { + encoding: "utf8", + }); + if (result.status === 0) { + const state = result.stdout.trim(); + return state !== "" && !state.startsWith("Z"); + } + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + describe("run-e2e.sh", () => { it("marks a managed containers run before invoking Playwright", () => { const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); @@ -337,6 +360,237 @@ describe("run-e2e.sh", () => { expect(`${result.stdout}\n${result.stderr}`).toContain("Playwright worker limit"); }); + it("parses script options after a leading -- (the natural pnpm invocation)", () => { + // pnpm/npm forward `--` verbatim, so `pnpm e2e:run -- --host` reaches the + // script as `-- --host`. A leading bare `--` must be dropped before parsing. + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nprintf '%s' \"${KANDEV_E2E_CONTAINERS:-}\"\n"); + fs.chmodSync(pnpmPath, 0o755); + + const result = spawnSync( + "bash", + [scriptPath, "--", "--host", "--no-build", "--project", "containers", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir), + }, + ); + + expect(result.status).toBe(0); + expect(result.stdout).toBe("1"); + }); + + it("preserves a leading -- for direct script invocations", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + fakeExecutable(binDir, "docker", "#!/usr/bin/env sh\nexit 1\n"); + fakeExecutable(binDir, "make", "#!/usr/bin/env sh\nexit 0\n"); + fakeExecutable(binDir, "pnpm", "#!/usr/bin/env sh\nprintf '%s' \"$*\"\n"); + + const result = spawnSync("bash", [scriptPath, "--", "clean"], { + encoding: "utf8", + env: runnerEnv(binDir, { npm_lifecycle_event: "" }), + }); + + expect(result.status).toBe(0); + expect(result.stderr).not.toContain("clean done"); + expect(result.stdout).toContain("clean"); + }); + + it("forwards only the tail after a second -- when the first -- is a leading pnpm artifact", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nprintf '%s' \"$*\"\n"); + fs.chmodSync(pnpmPath, 0o755); + + const result = spawnSync( + "bash", + [scriptPath, "--", "--host", "--no-build", "--project", "chromium", "--", "--grep", "foo"], + { + encoding: "utf8", + env: runnerEnv(binDir), + }, + ); + + expect(result.status).toBe(0); + expect(result.stdout).toBe( + "exec playwright test --config e2e/playwright.config.ts --project=chromium --workers=1 --grep foo", + ); + }); + + it("runs the clean subcommand when it follows a leading pnpm --", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const dockerPath = path.join(binDir, "docker"); + fs.writeFileSync( + dockerPath, + '#!/usr/bin/env sh\nif [ "$1" = "info" ]; then exit 1; fi\nexit 0\n', + ); + fs.chmodSync(dockerPath, 0o755); + + const result = spawnSync("bash", [scriptPath, "--", "clean"], { + encoding: "utf8", + env: runnerEnv(binDir), + }); + + expect(result.status).toBe(0); + expect(result.stderr).toContain("clean done"); + }); + + it("bounds the docker info probe and falls back to host mode on a hung daemon", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const dockerPath = path.join(binDir, "docker"); + fs.writeFileSync( + dockerPath, + '#!/usr/bin/env sh\nif [ "$1" = "info" ]; then sleep 30; exit 0; fi\nexit 0\n', + ); + fs.chmodSync(dockerPath, 0o755); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nexit 0\n"); + fs.chmodSync(pnpmPath, 0o755); + + const start = Date.now(); + const result = spawnSync( + "bash", + [scriptPath, "--no-build", "--project", "chromium", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_E2E_DOCKER_PROBE_TIMEOUT: "1" }), + }, + ); + const elapsedMs = Date.now() - start; + + expect(result.status).toBe(0); + expect(result.stderr).toContain( + "docker info did not respond within 1s; treating Docker as unavailable", + ); + expect(result.stderr).toContain("mode=host"); + expect(elapsedMs).toBeLessThan(15_000); + }); + + it("selects Docker mode after a successful bounded probe", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + fakeExecutable( + binDir, + "docker", + '#!/usr/bin/env sh\nif [ "$1" = "info" ] || [ "$1" = "image" ] || [ "$1" = "run" ]; then exit 0; fi\nexit 1\n', + ); + + const result = spawnSync( + "bash", + [scriptPath, "--no-build", "--project", "chromium", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_E2E_DOCKER_PROBE_TIMEOUT: "1" }), + }, + ); + + expect(result.status).toBe(0); + expect(result.stderr).toContain("mode=docker"); + expect(result.stderr).not.toContain("did not respond within"); + }); + + it("bounds the docker info probe even when the hung process ignores SIGTERM", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const childPidFile = path.join(binDir, "child.pid"); + const dockerPath = path.join(binDir, "docker"); + fs.writeFileSync( + dockerPath, + '#!/usr/bin/env sh\nif [ "$1" = "info" ]; then trap "" TERM; sleep 10 & child=$!; printf \'%s\' "$child" > "$KANDEV_RUNNER_CHILD_PID_FILE"; wait "$child"; exit 0; fi\nexit 0\n', + ); + fs.chmodSync(dockerPath, 0o755); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nexit 0\n"); + fs.chmodSync(pnpmPath, 0o755); + + const start = Date.now(); + const result = spawnSync( + "bash", + [scriptPath, "--no-build", "--project", "chromium", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { + KANDEV_E2E_DOCKER_PROBE_TIMEOUT: "1", + KANDEV_RUNNER_CHILD_PID_FILE: childPidFile, + }), + }, + ); + const elapsedMs = Date.now() - start; + + expect(result.status).toBe(0); + expect(result.stderr).toContain( + "docker info did not respond within 1s; treating Docker as unavailable", + ); + expect(result.stderr).toContain("mode=host"); + expect(elapsedMs).toBeLessThan(5_000); + const childPid = Number(fs.readFileSync(childPidFile, "utf8")); + expect(isRunningProcess(childPid)).toBe(false); + }, 20_000); + + it.each([ + ["non-numeric", "abc"], + ["fractional", "1.5"], + ["a natural but unsupported time suffix", "10s"], + ["trailing whitespace, e.g. from a .env file", "10 "], + ["a leading zero, which bash arithmetic reads as octal", "08"], + ])("rejects an invalid KANDEV_E2E_DOCKER_PROBE_TIMEOUT (%s)", (_label, value) => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nexit 0\n"); + fs.chmodSync(pnpmPath, 0o755); + + const result = spawnSync( + "bash", + [scriptPath, "--host", "--no-build", "--project", "chromium", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_E2E_DOCKER_PROBE_TIMEOUT: value }), + }, + ); + + expect(result.status).toBe(1); + expect(result.stderr).toContain( + "KANDEV_E2E_DOCKER_PROBE_TIMEOUT must be a non-negative integer", + ); + expect(result.stderr).toContain(`got '${value}'`); + }); + + it("accepts 0 as a valid KANDEV_E2E_DOCKER_PROBE_TIMEOUT and skips the probe wait in auto mode", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + tempDirs.push(binDir); + const dockerPath = path.join(binDir, "docker"); + fs.writeFileSync(dockerPath, "#!/usr/bin/env sh\nexit 0\n"); + fs.chmodSync(dockerPath, 0o755); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync(pnpmPath, "#!/usr/bin/env sh\nexit 0\n"); + fs.chmodSync(pnpmPath, 0o755); + + const result = spawnSync( + "bash", + [scriptPath, "--no-build", "--project", "chromium", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_E2E_DOCKER_PROBE_TIMEOUT: "0" }), + }, + ); + + expect(result.status).toBe(0); + expect(result.stderr).not.toContain( + "KANDEV_E2E_DOCKER_PROBE_TIMEOUT must be a non-negative integer", + ); + expect(result.stderr).toContain( + "docker info did not respond within 0s; treating Docker as unavailable", + ); + expect(result.stderr).toContain("mode=host"); + }); + it("applies the worker guard to raw Playwright runs", () => { const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-raw-")); tempDirs.push(binDir); diff --git a/docs/public/testing.md b/docs/public/testing.md index db70c810b2..216e85bb6f 100644 --- a/docs/public/testing.md +++ b/docs/public/testing.md @@ -82,6 +82,8 @@ pnpm e2e:run --project chromium tests/path/to/spec.ts The managed runner builds what it needs, chooses host or the CI runtime image, enables strict WebSocket assertions, and supports `--shards N`, `--no-build`, and `--project NAME`. +Set `KANDEV_E2E_DOCKER_PROBE_TIMEOUT` to change the Docker availability limit in automatic mode. The default is 10 seconds. Set it to `0` to skip the probe and use host mode. + Playwright uses one worker per process. Each worker fixture starts a real Go backend serving the built SPA with unique ports, a temporary `HOME`, Kandev home, SQLite database, repositories/worktrees, and agentctl port range. Each test receives a fresh browser context and resets seeded application state. Kandev process boundaries are real; external providers and the agent process are mocked unless a project says otherwise. Projects in `apps/web/e2e/playwright.config.ts` are: