Skip to content

test(e2e): scaffold tests/e2e harness + first smoke case - #20

Merged
moonming merged 1 commit into
mainfrom
feat/e2e-harness
Apr 20, 2026
Merged

test(e2e): scaffold tests/e2e harness + first smoke case#20
moonming merged 1 commit into
mainfrom
feat/e2e-harness

Conversation

@moonming

Copy link
Copy Markdown
Collaborator

Summary

Stands up the TypeScript E2E test harness from plan §12. Each test
spawns a fresh aisix binary against a unique etcd prefix
(/aisix-e2e-<uuid>), waits for /health and /admin/v1/health,
and tears down the binary + etcd prefix on exit.

The first smoke case proves the round-trip:

  1. POST a Model + ApiKey via Admin API.
  2. Wait the spec-mandated 500ms snapshot propagation window.
  3. GET /v1/models with the API key — sees the new model.
  4. POST /v1/chat/completions — forwards to the mock upstream on the
    expected /v1/chat/completions path.

Files

  • tests/e2e/src/harness/app.ts — spawn + wait + cleanup
  • tests/e2e/src/harness/etcd.ts — JSON gateway client (probe + range delete)
  • tests/e2e/src/harness/admin.ts — typed Admin API client
  • tests/e2e/src/harness/proxy.ts — typed Proxy client
  • tests/e2e/src/harness/upstream-openai.ts — node http mock with stream support
  • tests/e2e/src/harness/http.ts — undici wrapper that ignores HTTP_PROXY
  • tests/e2e/src/cases/smoke.test.ts — first canary

CI

The existing e2e job already wired the binary artifact + etcd service

  • pnpm install path in .github/workflows/ci.yml. The job stays
    continue-on-error: true until the harness has ~5 consecutive green
    runs on main — then we tighten.

Test plan

  • cd tests/e2e && npm install && npm test passes locally
    (against quay.io/coreos/etcd:v3.6.1 running on 127.0.0.1:2379)
  • npx tsc --noEmit clean
  • CI e2e (vitest) + coverage job goes green
  • CI lint, rust-unit, build-bin, build-ui, coverage-gate
    remain green

🤖 Generated with Claude Code

Per plan §12. The harness writes a fresh YAML config per test, picks
two free ports, spawns the aisix binary, and waits up to 10s for both
`/health` and `/admin/v1/health` to respond. Each test gets a unique
etcd prefix (`/aisix-e2e-<uuid>`) and best-effort cleanup on exit.

Modules:
- harness/app.ts        — spawn + wait + cleanup
- harness/etcd.ts       — JSON gateway client (ping, prefix delete)
- harness/admin.ts      — typed Admin API client
- harness/proxy.ts      — typed Proxy client
- harness/upstream-openai.ts — node http mock with stream support
- harness/http.ts       — undici wrapper that bypasses HTTP_PROXY env

First case: admin POST Model + ApiKey → wait 500ms → GET /v1/models
sees the model; chat completion forwards to the mock upstream's
/v1/chat/completions.

CI workflow already wires this in (build-bin → e2e job with etcd
service). The job stays `continue-on-error: true` until the harness
proves stable across a few CI runs; we'll tighten the gate then.

Notes for future work:
- Anthropic mock upstream still TODO
- SSE assertion helpers (waitForChunks, expectDoneEvent) still TODO
- Coverage instrumentation (RUSTFLAGS=instrument-coverage path) handled
  by the existing build-bin job; the harness only needs to invoke the
  binary.
Copilot AI review requested due to automatic review settings April 20, 2026 00:20
@moonming
moonming merged commit 1be35e5 into main Apr 20, 2026
8 checks passed
@moonming
moonming deleted the feat/e2e-harness branch April 20, 2026 00:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new TypeScript E2E test harness under tests/e2e that boots an aisix binary against a per-test etcd prefix, waits for readiness, and runs a first smoke test validating Admin→Proxy→Upstream round-tripping.

Changes:

  • Scaffolded a vitest-based E2E project (tests/e2e) with TS config + coverage output.
  • Added a harness to spawn/teardown aisix, talk to etcd (JSON gateway), call Admin/Proxy APIs, and run a local mock OpenAI-compatible upstream.
  • Added an initial smoke test case and updated CI messaging for the existing e2e job soft-gate.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
tests/e2e/vitest.config.ts Vitest runtime/coverage configuration tuned for low parallelism E2E runs.
tests/e2e/tsconfig.json TS compiler configuration for the E2E harness/tests.
tests/e2e/src/harness/app.ts Spawns aisix with a temp YAML config, waits for health, and cleans up etcd + tmpdir.
tests/e2e/src/harness/etcd.ts Minimal etcd v3 JSON gateway client (ping + deletePrefix).
tests/e2e/src/harness/admin.ts Typed Admin API client + 500ms propagation helper.
tests/e2e/src/harness/proxy.ts Thin Proxy API client for /v1/models and chat completions.
tests/e2e/src/harness/upstream-openai.ts Local Node HTTP mock for OpenAI-like endpoints including SSE streaming.
tests/e2e/src/harness/http.ts Undici request wrapper with a custom Agent to avoid proxy env vars.
tests/e2e/src/harness/ports.ts Utility to pick free TCP ports.
tests/e2e/src/harness/index.ts Barrel exports for harness components.
tests/e2e/src/cases/smoke.test.ts First smoke E2E validating Admin writes, proxy reads, and upstream forwarding.
tests/e2e/package.json Declares the E2E project dependencies/scripts (vitest + coverage).
tests/e2e/.gitignore Ignores node_modules and coverage outputs for the E2E project.
.github/workflows/ci.yml Updates the e2e job’s “soft gate” comment text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +87 to +93
child.stderr?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});
child.stdout?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});
let exitErr: string | undefined;

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If spawning the binary fails (e.g. AISIX_BIN path wrong), spawn() will emit an error event. Right now nothing listens for it, so the harness may just time out in waitForReady after 10s with a less-actionable message. Consider handling child.once("error", ...) and surfacing that error immediately (and including it in the diagnostics).

Suggested change
child.stderr?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});
child.stdout?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});
let exitErr: string | undefined;
let exitErr: string | undefined;
child.once("error", (err) => {
exitErr = `aisix failed to start: ${err.message}`;
stderrBuf += `${exitErr}\n`;
});
child.stderr?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});
child.stdout?.on("data", (c: Buffer) => {
stderrBuf += c.toString("utf8");
});

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +56
const res = await harnessRequest(`${this.endpoint}/v3/kv/deleterange`, {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ key, range_end: rangeEnd }),
});

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the success path, deletePrefix() never consumes the response body (no text()/dump()). With undici, leaving bodies unread can prevent connection reuse and leak resources across repeated calls. Consider always draining the body (e.g. await res.body.dump()) even when statusCode < 300.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +16
function freshAgent(): Agent {
return new Agent({ connectTimeout: 2000, keepAliveTimeout: 1000 });
}

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

freshAgent() creates a new undici Agent for every request, but the agent is never closed. Agents can keep sockets/timers alive and, across many polls, may create unnecessary resource usage or even keep the test process from exiting promptly. Consider either (a) reusing an agent per test/app instance and closing it during teardown, or (b) disabling keep-alive for this agent so it doesn't retain handles between requests.

Copilot uses AI. Check for mistakes.
Comment thread tests/e2e/package.json
"vitest": "2.1.9"
},
"dependencies": {
"openai": "4.65.0",

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openai is listed as a dependency, but it isn't used anywhere in tests/e2e/src yet. If it's not needed for the current smoke test, consider removing it until a test actually uses the SDK to keep installs faster and reduce supply-chain surface area.

Suggested change
"openai": "4.65.0",

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +31
const res = await harnessRequest(`${this.endpoint}/v3/maintenance/status`, {
method: "POST",
body: "{}",
headers: { "content-type": "application/json" },
signal: ctrl.signal,
});
clearTimeout(t);

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ping(), the setTimeout handle is only cleared after a successful request. If the request throws before clearTimeout(t), the timer will still fire later and keep an extra handle around (and call abort() after the function already returned). Consider wrapping the request in a try/finally to always clearTimeout(t).

Suggested change
const res = await harnessRequest(`${this.endpoint}/v3/maintenance/status`, {
method: "POST",
body: "{}",
headers: { "content-type": "application/json" },
signal: ctrl.signal,
});
clearTimeout(t);
let res;
try {
res = await harnessRequest(`${this.endpoint}/v3/maintenance/status`, {
method: "POST",
body: "{}",
headers: { "content-type": "application/json" },
signal: ctrl.signal,
});
} finally {
clearTimeout(t);
}

Copilot uses AI. Check for mistakes.
});
let exitErr: string | undefined;
child.once("exit", (code, signal) => {
if (code !== 0 && code !== null) {

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early-exit detection ignores the case where the binary exits due to a signal (code === null, signal !== null). That leaves exitErr unset and can produce misleading diagnostics (binary state: still running). Consider capturing any early termination where code !== 0 OR signal !== null.

Suggested change
if (code !== 0 && code !== null) {
if (code !== 0 || signal !== null) {

Copilot uses AI. Check for mistakes.
Comment on lines +140 to +146
const res = await harnessRequest(url, { method: "GET", headers });
lastStatus = res.statusCode;
if (res.statusCode === 200) {
await res.body.dump();
return;
}
await res.body.dump();

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waitForReady() enforces an overall timeoutMs, but each individual harnessRequest() call has no per-request deadline. A single hung HTTP request can therefore exceed timeoutMs and block the loop past the intended deadline. Consider adding an AbortController per attempt with a short timeout (and ensuring the timer is cleared) so the loop can reliably respect timeoutMs.

Suggested change
const res = await harnessRequest(url, { method: "GET", headers });
lastStatus = res.statusCode;
if (res.statusCode === 200) {
await res.body.dump();
return;
}
await res.body.dump();
const remainingMs = deadline - Date.now();
if (remainingMs <= 0) break;
const controller = new AbortController();
const requestTimeoutMs = Math.min(1_000, remainingMs);
const timeout = setTimeout(() => controller.abort(), requestTimeoutMs);
try {
const res = await harnessRequest(url, { method: "GET", headers, signal: controller.signal });
lastStatus = res.statusCode;
if (res.statusCode === 200) {
await res.body.dump();
return;
}
await res.body.dump();
} finally {
clearTimeout(timeout);
}

Copilot uses AI. Check for mistakes.
Comment on lines +43 to +46
throw new Error(
`etcd not reachable at ${process.env.AISIX_E2E_ETCD ?? "http://127.0.0.1:2379"} ` +
"(set AISIX_E2E_ETCD or run `docker run --rm -p 2379:2379 quay.io/coreos/etcd:v3.5.15`)",
);

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The etcd hint in this error message hard-codes quay.io/coreos/etcd:v3.5.15, but the PR description mentions v3.6.1. To avoid confusion for contributors, consider either aligning the version across docs/CI, or making the message version-agnostic (e.g. "run etcd v3.x").

Copilot uses AI. Check for mistakes.
Comment thread tests/e2e/tsconfig.json
"resolveJsonModule": true,
"skipLibCheck": true,
"forceConsistentCasingInFileNames": true,
"types": ["node", "vitest/globals"]

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsconfig.json includes vitest/globals in types, but vitest.config.ts sets globals: false. This mismatch can let tests typecheck while accidentally using global describe/test/expect that will be undefined at runtime. Consider removing vitest/globals from types (since tests import from vitest already) or flipping globals to true if globals are intended.

Suggested change
"types": ["node", "vitest/globals"]
"types": ["node"]

Copilot uses AI. Check for mistakes.
Comment thread tests/e2e/tsconfig.json
"forceConsistentCasingInFileNames": true,
"types": ["node", "vitest/globals"]
},
"include": ["src/**/*.ts"]

Copilot AI Apr 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsconfig.json only includes src/**/*.ts, so vitest.config.ts is not typechecked by npx tsc --noEmit. In the ui project the config file is included explicitly (e.g. vite.config.ts). Consider adding vitest.config.ts to include so config typing issues are caught in CI/local runs.

Suggested change
"include": ["src/**/*.ts"]
"include": ["src/**/*.ts", "vitest.config.ts"]

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants