From c07691c96d9b8ef0a3b4395730046889d7e361e1 Mon Sep 17 00:00:00 2001 From: Alassane <79718216+yattdev@users.noreply.github.com> Date: Wed, 19 Aug 2026 06:22:24 -0400 Subject: [PATCH] fix(e2e): restore managed container-run preflights (#2799) Co-authored-by: ayattara Co-authored-by: Carlos Florencio --- Makefile | 2 +- apps/web/e2e/global-setup.test.ts | 55 +++++++++++-- apps/web/e2e/global-setup.ts | 37 ++++++--- apps/web/e2e/scripts/run-e2e.sh | 10 ++- apps/web/e2e/scripts/run-e2e.test.ts | 111 +++++++++++++++++++++++++++ 5 files changed, 197 insertions(+), 18 deletions(-) create mode 100644 apps/web/e2e/scripts/run-e2e.test.ts diff --git a/Makefile b/Makefile index db418ef558..3f84bf4e0e 100644 --- a/Makefile +++ b/Makefile @@ -554,7 +554,7 @@ test-scripts: @node --test scripts/validate-public-docs.test.mjs .PHONY: test-e2e -test-e2e: build-backend build-web-e2e build-e2e-plugin-package +test-e2e: build-backend build-backend-linux-helpers build-web-e2e build-e2e-plugin-package @printf "$(CYAN)Running E2E tests (headless, parallel, managed runner)...$(RESET)\n" @cd $(WEB_DIR) && status=0; for project in routing auth chromium mobile-chrome containers; do \ printf "$(CYAN)-- project: $$project --$(RESET)\n"; \ diff --git a/apps/web/e2e/global-setup.test.ts b/apps/web/e2e/global-setup.test.ts index 6d65e0ef00..fb91d8c4f0 100644 --- a/apps/web/e2e/global-setup.test.ts +++ b/apps/web/e2e/global-setup.test.ts @@ -6,6 +6,7 @@ import globalSetup, { assertBackendArtifactsFresh } from "./global-setup"; let backendDir: string; let binPath: string; +const originalArgv = [...process.argv]; beforeEach(() => { backendDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-backend-fixture-")); @@ -25,6 +26,7 @@ beforeEach(() => { afterEach(() => { vi.restoreAllMocks(); vi.unstubAllEnvs(); + process.argv = [...originalArgv]; fs.rmSync(backendDir, { recursive: true, force: true }); }); @@ -248,7 +250,7 @@ describe("globalSetup", () => { vi.stubEnv("KANDEV_E2E_SKIP_FRESHNESS", "1"); const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(true); - (globalSetup as (config: unknown) => void)({ projects: [] }); + (globalSetup as () => void)(); expect(existsSync).toHaveBeenCalledWith(customBackend); expect(existsSync).not.toHaveBeenCalledWith(path.join(backendDirForTest(), "bin", "kandev")); @@ -261,17 +263,25 @@ describe("globalSetup", () => { .spyOn(fs, "existsSync") .mockImplementation((artifactPath) => artifactPath !== customBackend); - expect(() => (globalSetup as (config: unknown) => void)({ projects: [] })).toThrow( - /KANDEV_E2E_BIN file does not exist/, - ); + expect(() => (globalSetup as () => void)()).toThrow(/KANDEV_E2E_BIN file does not exist/); expect(existsSync).toHaveBeenCalledWith(customBackend); }); - it("checks Linux helper binaries when the containers project is selected", () => { + // Regression: `globalSetup` used to derive "is this a container run" from + // Playwright's `FullConfig.projects`, which always lists every project + // declared in playwright.config.ts regardless of `--project=` selection — + // so an ordinary `--project=chromium` run was indistinguishable from + // `--project=containers` and wrongly demanded Linux-only helper binaries + // that `make build` does not produce. See global-setup.ts's isContainerRun. + it.each([ + ["KANDEV_E2E_CONTAINERS", "1"], + ["KANDEV_E2E_DOCKER", "1"], + ])("checks Linux helper binaries for the %s marker", (marker, value) => { vi.stubEnv("KANDEV_E2E_SKIP_FRESHNESS", "1"); + vi.stubEnv(marker, value); const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(true); - (globalSetup as (config: unknown) => void)({ projects: [{ name: "containers" }] }); + (globalSetup as () => void)(); expect(existsSync).toHaveBeenCalledWith( path.join(backendDirForTest(), "bin", "mock-agent-linux-amd64"), @@ -280,6 +290,39 @@ describe("globalSetup", () => { path.join(backendDirForTest(), "bin", "agentctl-linux-amd64"), ); }); + + it("checks Linux helper binaries for a direct containers project selection", () => { + vi.stubEnv("KANDEV_E2E_SKIP_FRESHNESS", "1"); + vi.stubEnv("KANDEV_E2E_CONTAINERS", ""); + vi.stubEnv("KANDEV_E2E_DOCKER", ""); + process.argv = [...originalArgv, "--project=containers"]; + const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(true); + + (globalSetup as () => void)(); + + expect(existsSync).toHaveBeenCalledWith( + path.join(backendDirForTest(), "bin", "mock-agent-linux-amd64"), + ); + expect(existsSync).toHaveBeenCalledWith( + path.join(backendDirForTest(), "bin", "agentctl-linux-amd64"), + ); + }); + + it("does not require Linux helper binaries for an ordinary run", () => { + vi.stubEnv("KANDEV_E2E_SKIP_FRESHNESS", "1"); + vi.stubEnv("KANDEV_E2E_CONTAINERS", ""); + vi.stubEnv("KANDEV_E2E_DOCKER", ""); + const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(true); + + (globalSetup as () => void)(); + + expect(existsSync).not.toHaveBeenCalledWith( + path.join(backendDirForTest(), "bin", "mock-agent-linux-amd64"), + ); + expect(existsSync).not.toHaveBeenCalledWith( + path.join(backendDirForTest(), "bin", "agentctl-linux-amd64"), + ); + }); }); function backendDirForTest(): string { diff --git a/apps/web/e2e/global-setup.ts b/apps/web/e2e/global-setup.ts index a7ad802b9d..b09a5e674a 100644 --- a/apps/web/e2e/global-setup.ts +++ b/apps/web/e2e/global-setup.ts @@ -1,6 +1,5 @@ import fs from "node:fs"; import path from "node:path"; -import type { FullConfig } from "@playwright/test"; const BACKEND_DIR = path.resolve(__dirname, "../../../apps/backend"); const WEB_DIR = path.resolve(__dirname, ".."); @@ -10,7 +9,7 @@ export type BackendArtifact = { rebuildTarget: string; }; -export default function globalSetup(config: FullConfig) { +export default function globalSetup() { const customKandevBin = process.env.KANDEV_E2E_BIN; const kandevBin = customKandevBin ?? path.join(BACKEND_DIR, "bin", "kandev"); const artifacts: BackendArtifact[] = [ @@ -24,7 +23,7 @@ export default function globalSetup(config: FullConfig) { if (!customKandevBin) { artifacts.unshift({ path: kandevBin, rebuildTarget: "build" }); } - if (isContainerRun(config, process.env)) { + if (isContainerRun(process.env)) { artifacts.push( { path: path.join(BACKEND_DIR, "bin", "mock-agent-linux-amd64"), @@ -53,12 +52,32 @@ export default function globalSetup(config: FullConfig) { assertPseudoCatalogBundled(); } -function isContainerRun(config: FullConfig, env: NodeJS.ProcessEnv): boolean { - return ( - config.projects.some(({ name }) => name === "containers" || name === "docker") || - env.KANDEV_E2E_CONTAINERS === "1" || - env.KANDEV_E2E_DOCKER === "1" - ); +/** + * Playwright's `globalSetup` receives the fully resolved `FullConfig`, whose + * `projects` array always lists every project declared in + * `playwright.config.ts` — it is not filtered down to the project(s) selected + * via `--project=`. `config.projects.some((p) => p.name === "containers")` is + * therefore always true and cannot distinguish a `--project=chromium` run + * from a `--project=containers` one; verified empirically, not just by + * reading the types. + * + * Managed runs and CI pass an explicit environment marker. Direct Playwright + * runs keep the selected project in `process.argv`, so this function reads the + * CLI value as well. This mirrors `fixtures/backend.ts`'s + * `isContainerProjectActive`, whose project-name branch reads + * `testInfo.project.name` — the per-test resolved project — instead of + * `FullConfig`. + */ +export function isContainerRun(env: NodeJS.ProcessEnv, argv = process.argv): boolean { + if (env.KANDEV_E2E_CONTAINERS === "1" || env.KANDEV_E2E_DOCKER === "1") return true; + + const selectedProjects = argv.flatMap((arg, index) => { + if (arg === "--project") return argv[index + 1] ? [argv[index + 1]!] : []; + if (arg.startsWith("--project=")) return [arg.slice("--project=".length)]; + return []; + }); + + return selectedProjects.some((projects) => projects.split(",").includes("containers")); } function backendMakeCommand(backendDir: string, target: string): string { diff --git a/apps/web/e2e/scripts/run-e2e.sh b/apps/web/e2e/scripts/run-e2e.sh index f71ceabd53..6ef5b513dc 100755 --- a/apps/web/e2e/scripts/run-e2e.sh +++ b/apps/web/e2e/scripts/run-e2e.sh @@ -106,6 +106,7 @@ build_fe() { build_backend_host() { log "building backend (host)" local targets=(build) + # The Playwright project is `containers`; `KANDEV_E2E_DOCKER` remains only an env alias. [[ "$PROJECT" == containers ]] && targets+=(build-agentctl-linux build-mock-agent-linux) make -C "$BACKEND_DIR" "${targets[@]}" >/dev/null || die "backend build failed" } @@ -193,6 +194,8 @@ log "mode=$MODE shards=$SHARDS project=$PROJECT strict=$STRICT" STRICT_ENV=() [[ "$STRICT" == 1 ]] && STRICT_ENV=(KANDEV_E2E_WS_ASSERT=1) +CONTAINER_ENV=() +[[ "$PROJECT" == containers ]] && CONTAINER_ENV=(KANDEV_E2E_CONTAINERS=1) # --------------------------------------------------------------------------- # HOST mode @@ -202,13 +205,13 @@ run_host() { [[ "$DO_BUILD" == 1 ]] && { build_backend_host; build_fe; build_plugin_package; } 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[@]}"} ) + ( cd "$WEB_DIR" && env ${STRICT_ENV[@]+"${STRICT_ENV[@]}"} ${CONTAINER_ENV[@]+"${CONTAINER_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)) \ + ( cd "$WEB_DIR" && env ${STRICT_ENV[@]+"${STRICT_ENV[@]}"} ${CONTAINER_ENV[@]+"${CONTAINER_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+=("$!") @@ -234,6 +237,8 @@ run_docker() { local strict_flag=() [[ "$STRICT" == 1 ]] && strict_flag=(-e KANDEV_E2E_WS_ASSERT=1) + local container_flag=() + [[ "$PROJECT" == containers ]] && container_flag=(-e KANDEV_E2E_CONTAINERS=1) local capture_flag=() [[ -n "${CAPTURE_PR_ASSETS:-}" ]] && capture_flag=(-e CAPTURE_PR_ASSETS) 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\"" @@ -244,6 +249,7 @@ run_docker() { docker run --rm --ipc=host \ -v "$REPO_ROOT":/work -w /work/apps/web \ ${strict_flag[@]+"${strict_flag[@]}"} \ + ${container_flag[@]+"${container_flag[@]}"} \ ${capture_flag[@]+"${capture_flag[@]}"} \ -e NODE_OPTIONS=--dns-result-order=ipv4first \ -e PLAYWRIGHT_BROWSERS_PATH=/ms-playwright \ diff --git a/apps/web/e2e/scripts/run-e2e.test.ts b/apps/web/e2e/scripts/run-e2e.test.ts new file mode 100644 index 0000000000..def10deb63 --- /dev/null +++ b/apps/web/e2e/scripts/run-e2e.test.ts @@ -0,0 +1,111 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { afterEach, describe, expect, it } from "vitest"; + +const scriptPath = path.resolve(__dirname, "run-e2e.sh"); +const tempDirs: string[] = []; +const tempFiles: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + for (const file of tempFiles.splice(0)) fs.rmSync(file, { force: true }); +}); + +function runnerEnv(binDir: string, extra: Record = {}): NodeJS.ProcessEnv { + const env = { + ...process.env, + ...extra, + PATH: `${binDir}:${process.env.PATH ?? ""}`, + }; + delete env.KANDEV_E2E_CONTAINERS; + delete env.KANDEV_E2E_DOCKER; + delete env.CAPTURE_PR_ASSETS; + return env; +} + +describe("run-e2e.sh", () => { + it("marks a managed containers run before invoking Playwright", () => { + 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("passes the marker to every host shard", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + const resultFile = path.join(binDir, "marker.txt"); + tempDirs.push(binDir); + tempFiles.push("/tmp/e2e-host-shard-1.log", "/tmp/e2e-host-shard-2.log"); + const pnpmPath = path.join(binDir, "pnpm"); + fs.writeFileSync( + pnpmPath, + '#!/usr/bin/env sh\nprintf \'%s\\n\' "${KANDEV_E2E_CONTAINERS:-}" >> "$KANDEV_RUNNER_RESULT_FILE"\n', + ); + fs.chmodSync(pnpmPath, 0o755); + + const result = spawnSync( + "bash", + [ + scriptPath, + "--host", + "--no-build", + "--shards", + "2", + "--project", + "containers", + "--", + "--help", + ], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_RUNNER_RESULT_FILE: resultFile }), + }, + ); + + expect(result.status).toBe(0); + expect(fs.readFileSync(resultFile, "utf8").trim().split("\n")).toEqual(["1", "1"]); + }); + + it("passes the marker to the outer Docker runner", () => { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-e2e-runner-")); + const resultFile = path.join(binDir, "docker-args.txt"); + tempDirs.push(binDir); + tempFiles.push("/tmp/e2e-docker-shard-1.log"); + const dockerPath = path.join(binDir, "docker"); + fs.writeFileSync( + dockerPath, + '#!/usr/bin/env sh\nif [ "$1" = "info" ]; then exit 1; fi\nif [ "$1" = "run" ]; then printf \'%s\\n\' "$*" >> "$KANDEV_RUNNER_RESULT_FILE"; exit 0; fi\nexit 1\n', + ); + fs.chmodSync(dockerPath, 0o755); + + const result = spawnSync( + "bash", + [scriptPath, "--docker", "--no-build", "--project", "containers", "--", "--help"], + { + encoding: "utf8", + env: runnerEnv(binDir, { KANDEV_RUNNER_RESULT_FILE: resultFile }), + }, + ); + + expect(result.status).toBe(0); + const dockerInvocations = fs.readFileSync(resultFile, "utf8").trim().split("\n"); + expect(dockerInvocations.some((args) => args.includes("-e KANDEV_E2E_CONTAINERS=1"))).toBe( + true, + ); + }); +});