From d3ff238e94ca6fb9e708a147441a77fc3af61ba2 Mon Sep 17 00:00:00 2001 From: Hweinstock Date: Wed, 23 Sep 2026 18:52:33 +0000 Subject: [PATCH] fix(ci): re-enable Windows e2e tests --- .github/workflows/e2e-test.yml | 9 ++-- e2eTest/helpers/run.test.ts | 88 +++++++++++++++++++++++++++++++ e2eTest/helpers/run.ts | 81 +++++++++++++++++++++++++++- e2eTest/project/templates.test.ts | 50 +++++++++++++----- 4 files changed, 208 insertions(+), 20 deletions(-) create mode 100644 e2eTest/helpers/run.test.ts diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 874bcbca0..5bb32afcd 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -43,7 +43,7 @@ jobs: is-authorized: ${{ steps.check.outputs.is_authorized }} steps: - name: Fetch secrets from Secrets Manager - uses: aws/agentcore-devx-devtools/.github/actions/fetch-secrets@31aa3b031a86664e29861d68956e44b07cf21a74 + uses: aws/agentcore-devx-devtools/.github/actions/fetch-secrets@75989f65f7f193deaf83c237c36572d1a8f800b2 with: role-arn: ${{ secrets.WORKFLOW_SECRETS_READER_ROLE_ARN }} repo: AUTHORIZED_USERS @@ -83,9 +83,8 @@ jobs: # https://docs.aws.amazon.com/codebuild/latest/userguide/sample-github-action-runners-update-labels.html - name: Linux runner: '["codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}", "e2e-linux"]' - # TODO: re-enable these once secret fetching (https://github.com/aws/agentcore-devx-devtools/blob/main/.github/actions/fetch-secrets/action.yml) is windows compatible. - # - name: Windows - # runner: '["codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}", "image:windows-1.0", "e2e-windows"]' + - name: Windows + runner: '["codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}", "image:windows-1.0", "e2e-windows"]' # CodeBuild does not support macOS. # https://docs.aws.amazon.com/codebuild/latest/userguide/action-runner-questions.html#action-runner-platform @@ -103,7 +102,7 @@ jobs: - uses: astral-sh/setup-uv@v6 - run: bun install --frozen-lockfile - name: Fetch E2E role from Secrets Manager - uses: aws/agentcore-devx-devtools/.github/actions/fetch-secrets@31aa3b031a86664e29861d68956e44b07cf21a74 + uses: aws/agentcore-devx-devtools/.github/actions/fetch-secrets@75989f65f7f193deaf83c237c36572d1a8f800b2 with: role-arn: ${{ secrets.WORKFLOW_SECRETS_READER_ROLE_ARN }} repo: E2E_AWS_ROLE_ARN diff --git a/e2eTest/helpers/run.test.ts b/e2eTest/helpers/run.test.ts new file mode 100644 index 000000000..22305126b --- /dev/null +++ b/e2eTest/helpers/run.test.ts @@ -0,0 +1,88 @@ +import { afterAll, expect, test } from "vitest"; +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { CliRunner } from "./run"; + +const scriptsDir = await mkdtemp(join(tmpdir(), "agentcore-e2e-runner-")); + +afterAll(() => rm(scriptsDir, { recursive: true, force: true })); + +test("stop terminates shell-spawned descendants", async () => { + const parentScript = join(scriptsDir, "process-tree.cjs"); + const pidFile = join(scriptsDir, "descendant.pid"); + await writeFile( + parentScript, + [ + "const { spawn } = require('node:child_process');", + "const { writeFileSync } = require('node:fs');", + "const child = spawn(process.execPath, ['-e', 'process.on(\"SIGTERM\", () => {}); setInterval(() => {}, 1000)'], { stdio: 'ignore' });", + "writeFileSync(process.argv[2], String(child.pid));", + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join("\n"), + ); + + const cli = new CliRunner("node"); + const parent = cli.start([parentScript, pidFile], scriptsDir); + let descendantPid: number | undefined; + + try { + descendantPid = await reportedPid(pidFile); + await cli.stop(parent); + expect(await processStopped(descendantPid)).toBe(true); + } finally { + await cli.stop(parent).catch(() => {}); + if (descendantPid !== undefined && processRunning(descendantPid)) forceStop(descendantPid); + } +}, 20_000); + +/** Given a PID file path, resolves once a spawned process reports its descendant. */ +async function reportedPid(path: string): Promise { + for (let attempt = 0; attempt < 500; attempt++) { + try { + return Number(readFileSync(path, "utf8")); + } catch { + await delay(10); + } + } + throw new Error("process did not report a child PID"); +} + +/** Given a process ID, returns whether the process stops within one second. */ +async function processStopped(pid: number): Promise { + for (let attempt = 0; attempt < 100; attempt++) { + if (!processRunning(pid)) return true; + await delay(10); + } + return false; +} + +/** Given a process ID, returns whether it is still executable. */ +function processRunning(pid: number): boolean { + try { + process.kill(pid, 0); + if (process.platform === "linux") { + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + return stat.slice(stat.lastIndexOf(") ") + 2, stat.lastIndexOf(") ") + 3) !== "Z"; + } + return true; + } catch { + return false; + } +} + +/** Given a process ID, forcibly terminates it on the current platform. */ +function forceStop(pid: number): void { + if (process.platform === "win32") { + execFileSync("taskkill", ["/pid", String(pid), "/T", "/F"], { + stdio: "ignore", + timeout: 5000, + }); + return; + } + process.kill(pid, "SIGKILL"); +} diff --git a/e2eTest/helpers/run.ts b/e2eTest/helpers/run.ts index 641a13042..1af322bc8 100644 --- a/e2eTest/helpers/run.ts +++ b/e2eTest/helpers/run.ts @@ -1,6 +1,10 @@ -import { spawn } from "node:child_process"; +import { type ChildProcess, execFile, spawn } from "node:child_process"; import z from "zod"; +const KILL_GRACE_MS = 2000; +const STOP_TIMEOUT_MS = 10_000; +const TASKKILL_TIMEOUT_MS = 5000; + /** Given a CLI process, captures its standard output, error output, and exit code. */ export type RunResult = { stdout: string; @@ -25,7 +29,7 @@ function quoteShellArg(value: string): string { /** Minimal abstraction to handle the running of CLI commands **/ export class CliRunner { - private readonly command = requireEnv("AGENTCORE_CLI_PATH"); + constructor(private readonly command = requireEnv("AGENTCORE_CLI_PATH")) {} /** Given arguments and a working directory, runs the CLI and captures its result. */ run(args: string[], cwd: string): Promise { @@ -45,11 +49,84 @@ export class CliRunner { const command = [this.command, ...args.map(quoteShellArg)].join(" "); return spawn(command, { cwd, + detached: process.platform !== "win32", env: { ...process.env, AGENTCORE_TELEMETRY_DISABLED: "1", FORCE_COLOR: "0" }, shell: true, stdio: ["ignore", "pipe", "pipe"], }); } + + /** Given a running CLI process, terminates it and its descendants. */ + async stop(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + if (processTreeAlive(child)) killProcessTree(child, "SIGKILL"); + return; + } + + const closed = waitForClose(child); + killProcessTree(child, "SIGTERM"); + const killTimer = setTimeout(() => killProcessTree(child, "SIGKILL"), KILL_GRACE_MS); + killTimer.unref(); + + try { + await closed; + } finally { + clearTimeout(killTimer); + if (processTreeAlive(child)) killProcessTree(child, "SIGKILL"); + } + } +} + +/** Given a child process and signal, terminates its process tree on the current platform. */ +function killProcessTree(child: ChildProcess, signal: NodeJS.Signals): void { + if (!child.pid) return; + try { + if (process.platform === "win32") { + const taskkill = execFile( + "taskkill", + ["/pid", String(child.pid), "/T", "/F"], + { timeout: TASKKILL_TIMEOUT_MS, windowsHide: true }, + (error) => { + if (error && child.exitCode === null && child.signalCode === null) child.kill(signal); + }, + ); + taskkill.unref(); + } else { + process.kill(-child.pid, signal); + } + } catch { + if (child.exitCode === null && child.signalCode === null) child.kill(signal); + } +} + +/** Given a child process, resolves on close or rejects after a bounded wait. */ +function waitForClose(child: ChildProcess): Promise { + return new Promise((resolve, reject) => { + const onClose = () => { + clearTimeout(timeout); + resolve(); + }; + const timeout = setTimeout(() => { + child.removeListener("close", onClose); + reject( + new Error( + `CLI process ${child.pid ?? "unknown"} did not exit within ${STOP_TIMEOUT_MS}ms.`, + ), + ); + }, STOP_TIMEOUT_MS); + child.once("close", onClose); + }); +} + +/** Given a child process, returns whether its POSIX process group still exists. */ +function processTreeAlive(child: ChildProcess): boolean { + if (process.platform === "win32" || !child.pid) return false; + try { + process.kill(-child.pid, 0); + return true; + } catch { + return false; + } } /** Given a Zod schema and CLI result, returns typed output or throws a diagnostic error. */ diff --git a/e2eTest/project/templates.test.ts b/e2eTest/project/templates.test.ts index d67e75170..ff23a5e34 100644 --- a/e2eTest/project/templates.test.ts +++ b/e2eTest/project/templates.test.ts @@ -1,5 +1,5 @@ import { afterAll, beforeAll, describe, expect, test } from "vitest"; -import { mkdtemp } from "node:fs/promises"; +import { mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import z from "zod"; @@ -24,6 +24,13 @@ export const TIMEOUT_MS = { PROJECT_INVOKE: 3 * 60 * 1000, }; +const CONTAINER_RUNTIME: RuntimeTemplateTestCase = { + name: "py_strands_container", + template: "agent-python-strands-container", + protocol: "HTTP", + payload: { prompt: "Reply with a short greeting." }, +}; + const RUNTIME_TEMPLATES: RuntimeTemplateTestCase[] = [ { name: "agent_python_minimal", @@ -37,12 +44,7 @@ const RUNTIME_TEMPLATES: RuntimeTemplateTestCase[] = [ protocol: "HTTP", payload: { prompt: "Reply with a short greeting." }, }, - { - name: "py_strands_container", - template: "agent-python-strands-container", - protocol: "HTTP", - payload: { prompt: "Reply with a short greeting." }, - }, + CONTAINER_RUNTIME, { name: "agent_python_langchain", template: "agent-python-langchain", @@ -117,9 +119,18 @@ const RUNTIME_TEMPLATES: RuntimeTemplateTestCase[] = [ }, ] as const; +const LOCAL_RUNTIME_TEMPLATES = RUNTIME_TEMPLATES.filter( + (runtime) => process.platform !== "win32" || runtime !== CONTAINER_RUNTIME, +); + const ProjectCreatedSchema = z.object({ project: z.object({ path: z.string() }), }); +const ProjectSpecSchema = z + .object({ + runtimes: z.array(z.object({ name: z.string() }).loose()), + }) + .loose(); const OperationSchema = z.object({ operation: z.string() }); const RuntimeInvokeResponseSchema = z.object({ statusCode: z.number().int(), @@ -205,6 +216,7 @@ describe( const runtimePorts = new Map(); let dev: ReturnType | undefined; let pendingOutput = ""; + let originalProjectSpec: string | undefined; /** Given dev-process output, records the ports announced by running runtimes. */ const captureDevOutput = (chunk: Buffer) => { @@ -219,7 +231,15 @@ describe( } }; - beforeAll(() => { + beforeAll(async () => { + if (process.platform === "win32") { + const specPath = join(projectDir, "agentcore", "agentcore.json"); + originalProjectSpec = await readFile(specPath, "utf8"); + const spec = ProjectSpecSchema.parse(JSON.parse(originalProjectSpec)); + spec.runtimes = spec.runtimes.filter(({ name }) => name !== CONTAINER_RUNTIME.name); + await writeFile(specPath, `${JSON.stringify(spec, null, 2)}\n`); + } + dev = cli.start(["project", "dev", "--mode", "headless"], projectDir); dev.stdout?.on("data", captureDevOutput); dev.stderr?.on("data", captureDevOutput); @@ -228,12 +248,16 @@ describe( }, TIMEOUT_MS.PROJECT_DEV); afterAll(async () => { - if (!dev || dev.exitCode !== null) return; - dev.kill("SIGTERM"); - await new Promise((resolve) => dev?.once("close", resolve)); - }); + try { + if (dev) await cli.stop(dev); + } finally { + if (originalProjectSpec !== undefined) { + await writeFile(join(projectDir, "agentcore", "agentcore.json"), originalProjectSpec); + } + } + }, TIMEOUT_MS.PROJECT_DEV); - test.each(RUNTIME_TEMPLATES)( + test.each(LOCAL_RUNTIME_TEMPLATES)( "$name runs locally", { concurrent: true, timeout: TIMEOUT_MS.PROJECT_INVOKE }, async (runtime) => {