From a3a7d4914b9d464bc977ce31a71734374b11f8a1 Mon Sep 17 00:00:00 2001 From: Mike Cao Date: Fri, 11 Sep 2026 19:22:28 -0700 Subject: [PATCH] Harden API test Docker setup and stabilize coverage fixtures --- .dockerignore | 5 +++- Dockerfile | 2 ++ scripts/test-api.ts | 49 +++++++++++++++++++++++++++++++-- tests/api/coverage/oracle.ts | 9 +++++- tests/api/global-setup.ts | 53 ++++++++++++++++++++++++++++++++++-- tests/api/paths.ts | 8 +++++- tests/api/seed/dataset.ts | 15 +++++++--- 7 files changed, 130 insertions(+), 11 deletions(-) diff --git a/.dockerignore b/.dockerignore index fda7aa837..4f5a8e74e 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,8 +1,11 @@ .git +.next +coverage +geo docker-compose.yml Dockerfile .DS_Store -node_modules +**/node_modules .idea .env .env.* diff --git a/Dockerfile b/Dockerfile index 9cfabb0b7..adcfdb7df 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,6 +28,8 @@ WORKDIR /app # build:openapi shells out to pnpm, so the builder stage needs it too RUN npm install -g pnpm@${PNPM_VERSION} COPY --from=deps /app/node_modules ./node_modules +COPY --from=deps /app/packages/api-client/node_modules ./packages/api-client/node_modules +COPY --from=deps /app/packages/mcp/node_modules ./packages/mcp/node_modules COPY . . COPY docker/proxy.ts ./src diff --git a/scripts/test-api.ts b/scripts/test-api.ts index 690258159..61b4e852c 100644 --- a/scripts/test-api.ts +++ b/scripts/test-api.ts @@ -12,6 +12,8 @@ */ import { spawnSync } from 'node:child_process'; import { mkdirSync, writeFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { connect } from 'node:net'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { assertDisposableTarget } from '../tests/api/paths'; @@ -167,6 +169,24 @@ function composeArgs(profiles: string[], args: string[]) { ]; } +// Docker reports a taken host port as an opaque "ports are not available" error +// after the image has already been built, so check up front. Probe by connecting +// rather than binding: on Windows a second listen() on a busy port can succeed +// unless the owner set SO_EXCLUSIVEADDRUSE, so a bind test gives false negatives. +function isPortFree(port: number) { + return new Promise(resolve => { + const socket = connect({ host: '127.0.0.1', port }); + const done = (free: boolean) => { + socket.destroy(); + resolve(free); + }; + socket.setTimeout(1000); + socket.once('connect', () => done(false)); + socket.once('timeout', () => done(true)); + socket.once('error', () => done(true)); + }); +} + async function waitForHeartbeat(url: string, timeoutMs: number) { const deadline = Date.now() + timeoutMs; let lastError = ''; @@ -251,6 +271,24 @@ async function main() { } console.log(`Using the already-running server at ${BASE_URL} (PLAYWRIGHT_BASE_URL is set).`); } else { + // A kept stack from a previous run legitimately owns the port; only + // complain when something else is listening on it. + const ownStackRunning = capture( + 'docker', + composeArgs([profile], ['ps', '-q', '--status', 'running', appService]), + ).trim(); + + if (!ownStackRunning && !(await isPortFree(Number(PORT)))) { + console.error( + `Port ${PORT} is already in use on this machine, so the umami container cannot be published there. +` + + `Stop whatever is listening on ${PORT} (e.g. another dev server), or pick a different port: +` + + ` UMAMI_TEST_PORT=3200 pnpm test:api`, + ); + return 2; + } + console.log(`Starting test stack (profile: ${profile}, port: ${PORT})...`); const upArgs = ['up', '-d', '--wait', '--wait-timeout', '300']; @@ -282,7 +320,14 @@ async function main() { return 0; } - const playwrightArgs = ['exec', 'playwright', 'test', '-c', 'playwright.api.config.ts']; + // Invoke the Playwright CLI through the current node binary instead of + // `pnpm exec`: on Windows that resolves to pnpm.cmd, which Node refuses to + // spawn without a shell (EINVAL, CVE-2024-27980 hardening). + const playwrightCli = path.join( + path.dirname(createRequire(import.meta.url).resolve('@playwright/test/package.json')), + 'cli.js', + ); + const playwrightArgs = [playwrightCli, 'test', '-c', 'playwright.api.config.ts']; if (options.grep) { playwrightArgs.push('--grep', options.grep); @@ -290,7 +335,7 @@ async function main() { playwrightArgs.push(...options.passthrough); - exitCode = run(process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm', playwrightArgs, { + exitCode = run(process.execPath, playwrightArgs, { PLAYWRIGHT_BASE_URL: BASE_URL, PLAYWRIGHT_SKIP_WEB_SERVER: '1', UMAMI_TEST_DB: profile, diff --git a/tests/api/coverage/oracle.ts b/tests/api/coverage/oracle.ts index 4cc9d8527..a002afbcf 100644 --- a/tests/api/coverage/oracle.ts +++ b/tests/api/coverage/oracle.ts @@ -1,4 +1,4 @@ -import { readFileSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import { OPENAPI_FILE } from '../paths'; /** @@ -26,6 +26,13 @@ export function loadOperations(): ApiOperation[] { return cached; } + if (!existsSync(OPENAPI_FILE)) { + throw new Error( + `Coverage oracle ${OPENAPI_FILE} is missing. It is written by tests/api/global-setup.ts; ` + + 'run the suite through `pnpm test:api` or `pnpm test:api:run` rather than importing specs directly.', + ); + } + const document = JSON.parse(readFileSync(OPENAPI_FILE, 'utf8')); const operations: ApiOperation[] = []; diff --git a/tests/api/global-setup.ts b/tests/api/global-setup.ts index c02ed29f5..fab7cc441 100644 --- a/tests/api/global-setup.ts +++ b/tests/api/global-setup.ts @@ -1,9 +1,15 @@ /* eslint-disable no-console */ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { copyFileSync, existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { type FullConfig, request } from '@playwright/test'; import { ApiClient } from './client'; import { disableCoverageRecording } from './coverage/recorder'; -import { assertDisposableTarget, COVERAGE_DIR, SEED_FILE } from './paths'; +import { + assertDisposableTarget, + COVERAGE_DIR, + LOCAL_OPENAPI_FILE, + OPENAPI_FILE, + SEED_FILE, +} from './paths'; import { seedEnvironment } from './seed/setup'; const HEARTBEAT_TIMEOUT_MS = 120_000; @@ -34,6 +40,48 @@ async function waitForHeartbeat(baseURL: string, timeoutMs: number) { ); } +/** + * The coverage oracle must describe the server actually under test, so fetch its + * generated OpenAPI document rather than assuming a local `pnpm build` ran. + * public/openapi.json is a gitignored build artifact and is usually absent in a + * checkout that only runs the Compose stack. + */ +async function fetchOpenApiDocument(baseURL: string) { + const url = `${baseURL}/openapi.json`; + + try { + const response = await fetch(url); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + + const document = await response.json(); + + if (!document?.paths) { + throw new Error('response has no "paths" object'); + } + + writeFileSync(OPENAPI_FILE, JSON.stringify(document)); + return; + } catch (error) { + if (existsSync(LOCAL_OPENAPI_FILE)) { + console.warn( + `Could not fetch ${url} (${(error as Error).message}); ` + + `using the locally generated ${LOCAL_OPENAPI_FILE} as the coverage oracle instead.`, + ); + copyFileSync(LOCAL_OPENAPI_FILE, OPENAPI_FILE); + return; + } + + throw new Error( + `Could not fetch the OpenAPI document from ${url} (${(error as Error).message}) and no ` + + `local ${LOCAL_OPENAPI_FILE} exists. The server under test must serve its generated spec ` + + '(it is built by `pnpm build:openapi`), or generate it locally with `pnpm openapi:generate`.', + ); + } +} + export default async function globalSetup(config: FullConfig) { // Seeding traffic must not count towards endpoint coverage. disableCoverageRecording(); @@ -58,6 +106,7 @@ export default async function globalSetup(config: FullConfig) { console.log(`Waiting for ${baseURL}/api/heartbeat ...`); await waitForHeartbeat(baseURL, HEARTBEAT_TIMEOUT_MS); + await fetchOpenApiDocument(baseURL); if (skipSeed) { console.log(`Reusing seed state from ${SEED_FILE} (API_SKIP_SEED is set).`); diff --git a/tests/api/paths.ts b/tests/api/paths.ts index b2f4adab1..f9f183180 100644 --- a/tests/api/paths.ts +++ b/tests/api/paths.ts @@ -19,7 +19,13 @@ export const RUNTIME_DIR = path.join( ); export const COVERAGE_DIR = path.join(RUNTIME_DIR, 'coverage'); export const SEED_FILE = path.join(RUNTIME_DIR, 'seed.json'); -export const OPENAPI_FILE = path.join(ROOT_DIR, 'public/openapi.json'); +/** + * Coverage oracle: the OpenAPI document of the server under test, downloaded by + * global setup from `${BASE_URL}/openapi.json` (see tests/api/global-setup.ts). + */ +export const OPENAPI_FILE = path.join(RUNTIME_DIR, 'openapi.json'); +/** Locally generated spec (`pnpm openapi:generate`), used only as a fallback. */ +export const LOCAL_OPENAPI_FILE = path.join(ROOT_DIR, 'public/openapi.json'); const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]', '::1']); diff --git a/tests/api/seed/dataset.ts b/tests/api/seed/dataset.ts index 71ab1c61e..85237c368 100644 --- a/tests/api/seed/dataset.ts +++ b/tests/api/seed/dataset.ts @@ -182,6 +182,13 @@ function customEvent( export function buildDataset(now = Date.now()): Dataset { const nowSec = Math.floor(now / 1000); const range = { startAt: (nowSec - DAYS * DAY) * 1000, endAt: now }; + // Backdated traffic is anchored to noon UTC of each calendar day rather than + // to `now`, so the per-persona hour offsets below never straddle a UTC day + // boundary. Specs bucket by day in UTC and assert e.g. "3 purchases on each of + // 3 distinct days"; anchoring to `now` made that fail for runs started within + // a few hours after 00:00 UTC. + const todayStart = nowSec - (nowSec % DAY); + const dayAnchor = (daysAgo: number) => todayStart - daysAgo * DAY + 12 * HOUR; const visits: Visit[] = []; const batch: SendPayload[] = []; let pageviews = 0; @@ -190,7 +197,7 @@ export function buildDataset(now = Date.now()): Dataset { // Backdated traffic: days 1..6 ago (today only receives the live events below). for (let day = DAYS - 1; day >= 1; day--) { PERSONAS.forEach((persona, index) => { - const start = nowSec - day * DAY - (index + 1) * HOUR; + const start = dayAnchor(day) - (index + 1) * HOUR; const pageCount = 2 + ((day + index) % 3); const pages = PAGES.slice(0, pageCount); @@ -296,7 +303,7 @@ export function buildDataset(now = Date.now()): Dataset { // Secondary website (owned by the regular user): two personas, five days. for (let day = 5; day >= 1; day--) { PERSONAS.slice(0, 2).forEach((persona, index) => { - const timestamp = nowSec - day * DAY - (index + 1) * HOUR; + const timestamp = dayAnchor(day) - (index + 1) * HOUR; visits.push({ events: [ @@ -314,13 +321,13 @@ export function buildDataset(now = Date.now()): Dataset { 0, 'purchase', { revenue: 19.99, currency: CURRENCY, quantity: 1 }, - nowSec - 2 * DAY, + dayAnchor(2), ), ); // Link clicks and pixel hits (sessions are keyed by the link/pixel id). PERSONAS.slice(0, 5).forEach((persona, index) => { - const timestamp = nowSec - (index + 1) * DAY - 30 * 60; + const timestamp = dayAnchor(index + 1) - 30 * 60; batch.push({ type: 'event',