From 990845ece43208b480d348f65182fd83b9981e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:32:32 +0200 Subject: [PATCH 1/7] feat(runner): add lifecycle execution and environment checks (Task 2.1) executeScenario runs one scenario through prerequisites, install, reset, launch, optional setup, steps and cleanup, each phase bounded and abortable, and emits an event per phase without ever claiming a phase that did not run. Outcomes: a missing prerequisite (not a designated test environment, a dirty environment, the wrong OS or architecture, a missing display or audio capability) is blocked and nothing is installed; only an explicit AssertionFailure (or node's AssertionError) is failed; cancellation is cancelled; a timeout, a broken hook or an unexpected error is interrupted, so infrastructure trouble is never blamed on the candidate. Ownership: anything a run creates is recorded in a ledger under the test root before it is used. Cleanup removes only what the ledger lists: a process only if its recorded start time still matches (a reused pid is never signalled, an unidentifiable live process is left alone), a path only if its resolved location is strictly inside the test root (links, junctions and ".." cannot redirect a delete, the root itself is never removed). A cleanup that cannot finish leaves the environment dirty and refuses the next run until reset. Nothing runs anywhere that lacks an explicit test-root marker, and the home directory, the filesystem root and their ancestors are never accepted. Built test-first: 57 failing tests against stubs, then 392 passing. Co-Authored-By: Claude Sonnet 5 --- packages/qa/src/index.ts | 14 + packages/qa/src/runner/environment.ts | 68 ++++ packages/qa/src/runner/execute.ts | 346 +++++++++++++++++++ packages/qa/src/runner/resources.ts | 273 +++++++++++++++ packages/qa/test/fixtures/processes.ts | 68 ++++ packages/qa/test/runner/environment.test.ts | 54 +++ packages/qa/test/runner/execute.test.ts | 356 ++++++++++++++++++++ packages/qa/test/runner/resources.test.ts | 346 +++++++++++++++++++ 8 files changed, 1525 insertions(+) create mode 100644 packages/qa/src/runner/environment.ts create mode 100644 packages/qa/src/runner/execute.ts create mode 100644 packages/qa/src/runner/resources.ts create mode 100644 packages/qa/test/fixtures/processes.ts create mode 100644 packages/qa/test/runner/environment.test.ts create mode 100644 packages/qa/test/runner/execute.test.ts create mode 100644 packages/qa/test/runner/resources.test.ts diff --git a/packages/qa/src/index.ts b/packages/qa/src/index.ts index 7839da0..f70822b 100644 --- a/packages/qa/src/index.ts +++ b/packages/qa/src/index.ts @@ -31,3 +31,17 @@ export { export { eventDigest, parseRunEvent, type AttemptRecorded, type Checkpoint, type RunEvent, type RunStarted, type UploadAcknowledged } from './runner/events.ts'; export { appendEvent, JournalError, readRun, writeSummary, type AppendResult, type JournalErrorCode, type RunState } from './runner/journal.ts'; export { renderReport, type RenderedReport, type RenderOptions } from './runner/report.ts'; +export { defaultProbes, inspectEnvironment, type EnvironmentProbes, type InspectedEnvironment } from './runner/environment.ts'; +export { + AssertionFailure, + executeScenario, + type ExecutionContext, + type Lifecycle, + type Phase, + type ResultReason, + type RunContext, + type Scenario, + type ScenarioEvent, + type ScenarioResult, +} from './runner/execute.ts'; +export { checkTestRoot, designateTestRoot, resetDirtyEnvironment, type CleanupFailure, type OwnedResource, type TestRootCheck } from './runner/resources.ts'; diff --git a/packages/qa/src/runner/environment.ts b/packages/qa/src/runner/environment.ts new file mode 100644 index 0000000..ff1a6f9 --- /dev/null +++ b/packages/qa/src/runner/environment.ts @@ -0,0 +1,68 @@ +import { execFile } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import { arch, platform, release } from 'node:os'; +import type { EnvironmentProfile } from '../model/project.ts'; +import type { MeasuredEnvironment } from '../model/result.ts'; + +/** How the runner asks the machine what it can do. Injectable so tests need no display or sound card. */ +export interface EnvironmentProbes { + display(): Promise; + audio(): Promise; +} + +/** + * Heuristics, not proof. `display` says a graphical session appears to be reachable; `audio` says the sound + * subsystem appears to be running. Neither checks that a particular device exists or that a virtual display is + * not standing in for a real one; a scenario that needs that must check it itself. + */ +export const defaultProbes: EnvironmentProbes = { + async display() { + if (platform() === 'win32') { + // Services run in a non-interactive session, which is named "Services". + const session = process.env.SESSIONNAME; + return session !== undefined && session !== '' && session !== 'Services'; + } + return Boolean(process.env.DISPLAY || process.env.WAYLAND_DISPLAY); + }, + async audio() { + if (platform() === 'win32') return (await run('sc', ['query', 'Audiosrv'])).includes('RUNNING'); + const cards = await readFile('/proc/asound/cards', 'utf8').catch(() => ''); + return cards.trim() !== '' && !cards.includes('no soundcards'); + }, +}; + +function run(command: string, args: readonly string[]): Promise { + return new Promise((resolve) => { + execFile(command, [...args], { timeout: 5000, windowsHide: true }, (error, stdout) => resolve(error ? '' : stdout)); + }); +} + +export interface InspectedEnvironment { + environment: MeasuredEnvironment; + /** Set when the machine is not what the profile asks for (operating system or architecture). */ + profileMismatch?: string; +} + +const OS_NAMES: Record = { win32: 'windows', linux: 'linux', darwin: 'macos' }; +const ARCH_NAMES: Record = { x64: 'x86_64', arm64: 'aarch64' }; + +/** Measures this machine. A probe that throws means the capability is absent, never a crash of the run. */ +export async function inspectEnvironment(profile: EnvironmentProfile, probes: EnvironmentProbes = defaultProbes, toolVersion = '0.0.0'): Promise { + const has = async (probe: () => Promise): Promise => probe().catch(() => false); + const [display, audio] = await Promise.all([has(() => probes.display()), has(() => probes.audio())]); + + const os = OS_NAMES[platform()] ?? platform(); + const architecture = ARCH_NAMES[arch()] ?? arch(); + const environment: MeasuredEnvironment = { + os, + osVersion: release(), + arch: architecture, + capabilities: [...(audio ? ['audio'] : []), ...(display ? ['display'] : [])], + toolVersion, + }; + + const problems: string[] = []; + if (os !== profile.os) problems.push(`this machine is ${os}, the profile expects ${profile.os}`); + if (architecture !== profile.arch) problems.push(`this machine is ${architecture}, the profile expects ${profile.arch}`); + return problems.length === 0 ? { environment } : { environment, profileMismatch: problems.join('; ') }; +} diff --git a/packages/qa/src/runner/execute.ts b/packages/qa/src/runner/execute.ts new file mode 100644 index 0000000..9fda0d7 --- /dev/null +++ b/packages/qa/src/runner/execute.ts @@ -0,0 +1,346 @@ +import type { ChildProcess, SpawnOptions } from 'node:child_process'; +import type { Candidate } from '../model/candidate.ts'; +import type { EnvironmentProfile } from '../model/project.ts'; +import type { Requirement, RequirementKey } from '../model/requirement.ts'; +import type { MeasuredEnvironment, Outcome } from '../model/result.ts'; +import { defaultProbes, inspectEnvironment, type EnvironmentProbes } from './environment.ts'; +import { checkTestRoot, cleanupOwnedResources, markDirty, readDirty, readLedger, recordOwned, spawnOwned, type CleanupFailure, type OwnedResource } from './resources.ts'; + +/** Thrown by a scenario or hook to say the candidate behaved wrongly. The only thing that makes a result `failed`. */ +export class AssertionFailure extends Error { + constructor(message: string) { + super(message); + this.name = 'AssertionFailure'; + } +} + +export type Phase = 'prerequisites' | 'install' | 'reset' | 'launch' | 'setup' | 'steps' | 'cleanup'; + +export interface ScenarioEvent { + scenario: string; + phase: Phase; + status: 'started' | 'finished' | 'failed'; + detail?: string; +} + +/** What a lifecycle hook or scenario may do. `own` and `spawn` record ownership before anything is used. */ +export interface RunContext { + candidate: Candidate; + profile: EnvironmentProfile; + testRoot: string; + signal: AbortSignal; + own(resource: OwnedResource): Promise; + spawn(label: string, command: string, args: readonly string[], options?: SpawnOptions): Promise; + /** Polls until `condition` is true. Running out of time is an assertion failure; abort is a cancellation. */ + waitFor(condition: () => boolean | Promise, options?: { timeoutMs?: number; intervalMs?: number; description?: string }): Promise; +} + +export interface Lifecycle { + install(ctx: RunContext): Promise; + /** Puts the application into a known state before it starts, e.g. by clearing its data. */ + reset(ctx: RunContext): Promise; + launch(ctx: RunContext): Promise; + cleanup(ctx: RunContext): Promise; +} + +export interface Scenario { + id: string; + requirement: Requirement; + setup?(ctx: RunContext): Promise; + steps(ctx: RunContext): Promise; +} + +export interface ExecutionContext { + candidate: Candidate; + profile: EnvironmentProfile; + testRoot: string; + signal: AbortSignal; + emit(event: ScenarioEvent): void | Promise; + lifecycle: Lifecycle; + probes?: EnvironmentProbes; + /** Bounds in milliseconds. Every wait is bounded; these override the defaults. */ + timeouts?: { phaseMs?: number; stepsMs?: number; cleanupMs?: number }; +} + +export type ResultReason = + | 'not-a-test-environment' + | 'dirty-environment' + | 'wrong-environment' + | 'capability-missing' + | 'assertion-failed' + | 'cancelled' + | 'timeout' + | 'infrastructure-error'; + +export interface ScenarioResult { + scenario: string; + requirement: RequirementKey; + outcome: Outcome; + reason?: ResultReason; + detail?: string; + /** Capabilities the scenario needed and the machine lacks, when `reason` is `capability-missing`. */ + missing?: string[]; + environment?: MeasuredEnvironment; + cleanup: { ok: boolean; failures: string[] }; + /** Resources still owned after cleanup, which keep the environment dirty until it is reset. */ + leftover: CleanupFailure[]; +} + +const DEFAULT_PHASE_MS = 120_000; +const DEFAULT_STEPS_MS = 300_000; +const DEFAULT_CLEANUP_MS = 60_000; + +/** Why a phase stopped early. These are reasons, not verdicts on the candidate. */ +class Cancelled extends Error { + constructor() { + super('cancelled'); + } +} +class TimedOut extends Error { + constructor(readonly phase: Phase, readonly ms: number) { + super(`${phase} timed out after ${ms} ms`); + } +} +class SinkFailed extends Error { + constructor(cause: unknown) { + super(`event sink failed: ${message(cause)}`); + } +} + +const withEnvironment = (environment: MeasuredEnvironment | undefined): { environment?: MeasuredEnvironment } => (environment === undefined ? {} : { environment }); +const message = (error: unknown): string => (error instanceof Error ? error.message : String(error)); + +/** Node's assert and most test libraries throw errors named AssertionError; those are assertions too. */ +function isAssertionFailure(error: unknown): boolean { + return error instanceof AssertionFailure || (error instanceof Error && (error.name === 'AssertionError' || (error as { code?: unknown }).code === 'ERR_ASSERTION')); +} + +interface Stop { + outcome: Outcome; + reason: ResultReason; + detail?: string; +} + +/** Maps whatever stopped a phase to an outcome. Only an explicit assertion is a verdict on the candidate. */ +function classify(phase: Phase, error: unknown): Stop { + if (error instanceof Cancelled) return { outcome: 'cancelled', reason: 'cancelled', detail: `cancelled during ${phase}` }; + if (error instanceof TimedOut) return { outcome: 'interrupted', reason: 'timeout', detail: error.message }; + if (error instanceof SinkFailed) return { outcome: 'interrupted', reason: 'infrastructure-error', detail: error.message }; + if (isAssertionFailure(error)) return { outcome: 'failed', reason: 'assertion-failed', detail: message(error) }; + return { outcome: 'interrupted', reason: 'infrastructure-error', detail: `${phase}: ${message(error)}` }; +} + +/** + * Runs one scenario against one candidate in a designated test environment. + * + * Outcomes: a prerequisite that is not met is `blocked` and nothing is installed; an assertion failure is `failed`; + * a cancellation is `cancelled`; a timeout, a hook that breaks, or an unexpected error is `interrupted`, because + * trouble in the infrastructure says nothing about the candidate. Cleanup runs whenever anything was started, and + * a cleanup that does not complete leaves the environment marked dirty until it is reset. + */ +export async function executeScenario(context: ExecutionContext, scenario: Scenario): Promise { + const scenarioId = scenario.id; + const base = { scenario: scenarioId, requirement: scenario.requirement.key, cleanup: { ok: true, failures: [] as string[] }, leftover: [] as CleanupFailure[] }; + const finish = (result: Pick & Partial): ScenarioResult => ({ ...base, ...result }); + if (context.signal.aborted) return finish({ outcome: 'cancelled', reason: 'cancelled' }); + + // Once the event sink fails nothing more is sent to it: the run stops and the failure is reported. + let sinkFailure: SinkFailed | undefined; + const emit = async (phase: Phase, status: ScenarioEvent['status'], detail?: string): Promise => { + if (sinkFailure !== undefined) return; + try { + await context.emit({ scenario: scenarioId, phase, status, ...(detail === undefined ? {} : { detail }) }); + } catch (error) { + sinkFailure = new SinkFailed(error); + throw sinkFailure; + } + }; + + // -- Prerequisites: nothing is installed or launched unless all of these hold. ------------------------------------ + let environment: MeasuredEnvironment | undefined; + let root: string; + try { + await emit('prerequisites', 'started'); + const designated = await checkTestRoot(context.testRoot); + if (!designated.ok) { + await emit('prerequisites', 'failed', designated.reason); + return finish({ outcome: 'blocked', reason: 'not-a-test-environment', detail: `${context.testRoot}: ${designated.reason}` }); + } + root = designated.root; + + const dirty = await dirtiness(root); + if (dirty !== undefined) { + await emit('prerequisites', 'failed', dirty); + return finish({ outcome: 'blocked', reason: 'dirty-environment', detail: dirty }); + } + + const inspected = await inspectEnvironment(context.profile, context.probes ?? defaultProbes); + environment = inspected.environment; + if (inspected.profileMismatch !== undefined) { + await emit('prerequisites', 'failed', inspected.profileMismatch); + return finish({ outcome: 'blocked', reason: 'wrong-environment', detail: inspected.profileMismatch, ...withEnvironment(environment) }); + } + const missing = scenario.requirement.capabilities.filter((c) => !inspected.environment.capabilities.includes(c)).sort(); + if (missing.length > 0) { + await emit('prerequisites', 'failed', `missing: ${missing.join(', ')}`); + return finish({ outcome: 'blocked', reason: 'capability-missing', missing, detail: `missing capabilities: ${missing.join(', ')}`, ...withEnvironment(environment) }); + } + await emit('prerequisites', 'finished'); + } catch (error) { + return finish({ ...classify('prerequisites', error), ...withEnvironment(environment) }); + } + + // -- The lifecycle. Each phase is bounded and abortable; a phase that fails ends the run. ------------------------- + const phaseMs = context.timeouts?.phaseMs ?? DEFAULT_PHASE_MS; + const stepsMs = context.timeouts?.stepsMs ?? DEFAULT_STEPS_MS; + const cleanupMs = context.timeouts?.cleanupMs ?? DEFAULT_CLEANUP_MS; + const { lifecycle } = context; + const contextFor = (signal: AbortSignal): RunContext => ({ + candidate: context.candidate, + profile: context.profile, + testRoot: root, + signal, + own: (resource) => recordOwned(root, resource), + spawn: (label, command, args, options) => spawnOwned(root, label, command, args, options), + waitFor: (condition, options) => waitFor(signal, condition, options), + }); + + const phases: Array<[Phase, number, (ctx: RunContext) => Promise]> = [ + ['install', phaseMs, (ctx) => lifecycle.install(ctx)], + ['reset', phaseMs, (ctx) => lifecycle.reset(ctx)], + ['launch', phaseMs, (ctx) => lifecycle.launch(ctx)], + ...(scenario.setup === undefined ? [] : [['setup', phaseMs, (ctx: RunContext) => scenario.setup!(ctx)] as [Phase, number, (ctx: RunContext) => Promise]]), + ['steps', stepsMs, (ctx) => scenario.steps(ctx)], + ]; + + let stop: Stop | undefined; + let touched = false; + for (const [phase, limitMs, run] of phases) { + if (context.signal.aborted) { + stop = { outcome: 'cancelled', reason: 'cancelled', detail: `cancelled before ${phase}` }; + break; + } + touched = true; + try { + await emit(phase, 'started'); + await bounded(phase, limitMs, context.signal, (signal) => run(contextFor(signal))); + await emit(phase, 'finished'); + } catch (error) { + stop = classify(phase, error); + await emit(phase, 'failed', stop.detail).catch(() => undefined); + break; + } + } + + // -- Cleanup: on success, on failure and on cancellation, and never cancelled by the run's own signal. ------------ + const cleanup = touched ? await cleanUp(root, lifecycle, contextFor, cleanupMs, emit) : { ok: true, failures: [] as string[], leftover: [] as CleanupFailure[] }; + + return finish({ + outcome: stop?.outcome ?? 'passed', + ...(stop === undefined ? {} : { reason: stop.reason, ...(stop.detail === undefined ? {} : { detail: stop.detail }) }), + ...withEnvironment(environment), + cleanup: { ok: cleanup.ok, failures: cleanup.failures }, + leftover: cleanup.leftover, + }); +} + +async function dirtiness(root: string): Promise { + try { + const marked = await readDirty(root); + if (marked !== undefined) return `the environment was left dirty: ${marked}`; + const owned = await readLedger(root); + if (owned.length > 0) return `${owned.length} resource(s) from an earlier run are still owned: ${owned.map((r) => r.label).join(', ')}`; + return undefined; + } catch (error) { + return `the record of owned resources cannot be read: ${message(error)}`; + } +} + +/** Runs `work` with a deadline, abortable by `outer`. A hook that ignores its signal cannot hold the run up. */ +async function bounded(phase: Phase, ms: number, outer: AbortSignal, work: (signal: AbortSignal) => Promise): Promise { + const controller = new AbortController(); + const abortWith = (reason: Error): void => { + if (!controller.signal.aborted) controller.abort(reason); + }; + const onOuterAbort = (): void => abortWith(new Cancelled()); + outer.addEventListener('abort', onOuterAbort); + const timer = setTimeout(() => abortWith(new TimedOut(phase, ms)), ms); + const interrupted = new Promise((_, reject) => { + const reject_ = (): void => reject(controller.signal.reason); + if (controller.signal.aborted) reject_(); + else controller.signal.addEventListener('abort', reject_, { once: true }); + }); + interrupted.catch(() => undefined); // no unhandled rejection when the work finishes first + const running = Promise.resolve().then(() => work(controller.signal)); + running.catch(() => undefined); // an abandoned hook may still fail later; that is no longer our concern + try { + if (outer.aborted) abortWith(new Cancelled()); + await Promise.race([running, interrupted]); + } finally { + clearTimeout(timer); + outer.removeEventListener('abort', onOuterAbort); + } +} + +async function cleanUp( + root: string, + lifecycle: Lifecycle, + contextFor: (signal: AbortSignal) => RunContext, + ms: number, + emit: (phase: Phase, status: ScenarioEvent['status'], detail?: string) => Promise, +): Promise<{ ok: boolean; failures: string[]; leftover: CleanupFailure[] }> { + const failures: string[] = []; + await emit('cleanup', 'started').catch(() => undefined); + try { + // A fresh signal: the run may have been cancelled, but its cleanup must still be allowed to finish. + await bounded('cleanup', ms, new AbortController().signal, (signal) => lifecycle.cleanup(contextFor(signal))); + } catch (error) { + failures.push(error instanceof TimedOut ? `cleanup hook ${error.message}` : `cleanup hook failed: ${message(error)}`); + } + + // Whatever the run still owns is reaped by the runner, whatever the hook did or did not do. + let leftover: CleanupFailure[] = []; + try { + leftover = (await cleanupOwnedResources(root)).failures; + } catch (error) { + failures.push(`could not clean up owned resources: ${message(error)}`); + } + for (const item of leftover) failures.push(`${item.resource.label}: ${item.reason}${item.detail === undefined ? '' : ` (${item.detail})`}`); + + if (failures.length > 0) { + try { + await markDirty(root, `cleanup failed: ${failures.join('; ')}`); + } catch (error) { + // The ledger still lists whatever was left, so the next run will see a dirty environment either way. + failures.push(`could not mark the environment dirty: ${message(error)}`); + } + } + const ok = failures.length === 0; + await emit('cleanup', ok ? 'finished' : 'failed', ok ? undefined : failures.join('; ')).catch(() => undefined); + return { ok, failures, leftover }; +} + +async function waitFor( + signal: AbortSignal, + condition: () => boolean | Promise, + options: { timeoutMs?: number; intervalMs?: number; description?: string } = {}, +): Promise { + const timeoutMs = options.timeoutMs ?? 10_000; + const intervalMs = options.intervalMs ?? 50; + const deadline = Date.now() + timeoutMs; + for (;;) { + if (signal.aborted) throw signal.reason; + if (await condition()) return; + if (Date.now() >= deadline) throw new AssertionFailure(`timed out after ${timeoutMs} ms waiting for ${options.description ?? 'a condition'}`); + // The abort listener is removed as soon as the pause ends, or every poll would leave one behind. + await new Promise((resolve) => { + const done = (): void => { + clearTimeout(timer); + signal.removeEventListener('abort', done); + resolve(); + }; + const timer = setTimeout(done, intervalMs); + signal.addEventListener('abort', done, { once: true }); + }); + } +} diff --git a/packages/qa/src/runner/resources.ts b/packages/qa/src/runner/resources.ts new file mode 100644 index 0000000..c8ff9f3 --- /dev/null +++ b/packages/qa/src/runner/resources.ts @@ -0,0 +1,273 @@ +import { execFile, spawn, type ChildProcess, type SpawnOptions } from 'node:child_process'; +import { mkdir, lstat, readFile, realpath, rm, stat } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join, parse, resolve, sep } from 'node:path'; +import { writeFileAtomic } from './journal.ts'; + +/** Something a run created and therefore may remove. Nothing else is ever touched. */ +export type OwnedResource = + | { kind: 'process'; pid: number; identity: string; label: string } + | { kind: 'path'; path: string; label: string }; + +export const TEST_ROOT_MARKER = '.release-qa-test-root'; +const LEDGER_FILE = '.release-qa-owned.json'; +const DIRTY_FILE = '.release-qa-dirty.json'; + +export type TestRootCheck = { ok: true; root: string } | { ok: false; reason: 'not-a-directory' | 'unsafe-root' | 'missing-marker' }; + +export interface CleanupFailure { + resource: OwnedResource; + reason: 'outside-test-root' | 'is-test-root' | 'is-link' | 'identity-unknown' | 'still-running' | 'remove-failed'; + detail?: string; +} + +export interface CleanupOptions { + /** How long a process gets to exit after a polite request before it is forced. */ + graceMs?: number; + /** Test seam: how a process is identified. */ + identityOf?: (pid: number) => Promise; +} + +const errorCode = (error: unknown): string | undefined => (error as NodeJS.ErrnoException).code; +const fold = (path: string): string => (process.platform === 'win32' ? path.toLowerCase() : path); +const isInside = (root: string, path: string): boolean => fold(path).startsWith(fold(root) + sep); +const sleep = (ms: number): Promise => new Promise((resolveSleep) => setTimeout(resolveSleep, ms)); + +/** The home directory, the filesystem root and anything that contains the home directory are never a test root. */ +async function isUnsafeRoot(real: string, homeDirectory: string): Promise { + const home = await realpath(homeDirectory).catch(() => resolve(homeDirectory)); + return parse(real).root === real || fold(real) === fold(home) || isInside(real, home); +} + +/** Marks a directory as somewhere the runner may install, launch and delete. Nothing runs without this. */ +export async function designateTestRoot(dir: string, options: { home?: string } = {}): Promise { + await mkdir(dir, { recursive: true }); + const real = await realpath(dir); + if (await isUnsafeRoot(real, options.home ?? homedir())) throw new Error(`unsafe test root: ${real} is the filesystem root, the home directory or contains it`); + await writeFileAtomic(join(real, TEST_ROOT_MARKER), `${JSON.stringify({ schemaVersion: 1, purpose: 'release-qa-test-environment' }, null, 2)}\n`); +} + +/** `options.home` is a test seam: which directory counts as the home directory. */ +export async function checkTestRoot(dir: string, options: { home?: string } = {}): Promise { + const info = await stat(dir).catch(() => undefined); + if (info === undefined || !info.isDirectory()) return { ok: false, reason: 'not-a-directory' }; + const real = await realpath(dir); + if (await isUnsafeRoot(real, options.home ?? homedir())) return { ok: false, reason: 'unsafe-root' }; + const marker = await readFile(join(real, TEST_ROOT_MARKER), 'utf8').catch(() => undefined); + let designated = false; + try { + designated = marker !== undefined && (JSON.parse(marker) as { purpose?: unknown }).purpose === 'release-qa-test-environment'; + } catch { + designated = false; + } + return designated ? { ok: true, root: real } : { ok: false, reason: 'missing-marker' }; +} + +// --------------------------------------------------------------------------------------------------------------- +// The ledger: what this run owns, on disk before it is used, so a crash still leaves a record. + +/** One writer at a time per root within this process; a run is the only writer of its test root. */ +const queues = new Map>(); +function serialized(root: string, work: () => Promise): Promise { + const key = resolve(root); + const next = (queues.get(key) ?? Promise.resolve()).then(work, work); + queues.set(key, next.catch(() => undefined)); + return next; +} + +export async function readLedger(root: string): Promise { + let text: string; + try { + text = await readFile(join(root, LEDGER_FILE), 'utf8'); + } catch (error) { + if (errorCode(error) === 'ENOENT') return []; + throw error; + } + // An unreadable ledger must never be mistaken for an empty one: that would forget what a crashed run owned. + // Every entry is checked, because cleanup acts on these values and a malformed one must stop it, not steer it. + const where = join(root, LEDGER_FILE); + const parsed = JSON.parse(text) as { schemaVersion?: unknown; resources?: unknown }; + if (parsed.schemaVersion !== 1 || !Array.isArray(parsed.resources)) throw new Error(`the ledger at ${where} is not a version 1 ledger`); + parsed.resources.forEach((entry: unknown, index) => { + if (!isOwnedResource(entry)) throw new Error(`the ledger at ${where} has a malformed entry at position ${index}`); + }); + return parsed.resources as OwnedResource[]; +} + +function isOwnedResource(value: unknown): value is OwnedResource { + if (typeof value !== 'object' || value === null) return false; + const entry = value as Record; + if (typeof entry.label !== 'string') return false; + if (entry.kind === 'process') return Number.isSafeInteger(entry.pid) && (entry.pid as number) > 0 && typeof entry.identity === 'string' && entry.identity !== ''; + return entry.kind === 'path' && typeof entry.path === 'string' && entry.path !== ''; +} + +async function writeLedger(root: string, resources: readonly OwnedResource[]): Promise { + await writeFileAtomic(join(root, LEDGER_FILE), `${JSON.stringify({ schemaVersion: 1, resources }, null, 2)}\n`); +} + +export function recordOwned(root: string, resource: OwnedResource): Promise { + return serialized(root, async () => writeLedger(root, [...(await readLedger(root)), resource])); +} + +// --------------------------------------------------------------------------------------------------------------- +// Processes + +function output(command: string, args: readonly string[]): Promise { + return new Promise((resolveOutput) => { + execFile(command, [...args], { timeout: 10000, windowsHide: true }, (error, stdout) => resolveOutput(error ? undefined : stdout.trim())); + }); +} + +/** + * Something that names one particular process for as long as it lives: its start time. A pid alone is not + * enough, because the operating system reuses them. Returns undefined when the process does not exist or its + * identity cannot be read, in which case nothing may be done to it. + */ +export async function processIdentity(pid: number): Promise { + if (process.platform === 'linux') { + const stat = await readFile(`/proc/${pid}/stat`, 'utf8').catch(() => undefined); + if (stat === undefined) return undefined; + // "pid (comm) state ppid ..." where comm may contain spaces and parentheses; the fields start after the last ")". + const fields = stat.slice(stat.lastIndexOf(')') + 2).split(' '); + if (fields[0] === 'Z') return undefined; // a zombie has already exited + return fields[19] === undefined ? undefined : `linux:${fields[19]}`; + } + if (process.platform === 'win32') { + const started = await output('powershell', ['-NoProfile', '-NonInteractive', '-Command', `(Get-Process -Id ${Math.trunc(pid)} -ErrorAction Stop).StartTime.ToFileTimeUtc()`]); + return started === undefined || started === '' ? undefined : `win32:${started}`; + } + const started = await output('ps', ['-o', 'lstart=', '-p', String(Math.trunc(pid))]); + return started === undefined || started === '' ? undefined : `${process.platform}:${started}`; +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return errorCode(error) === 'EPERM'; + } +} + +/** Starts a process and records it, with its identity, before handing it back. */ +export async function spawnOwned(root: string, label: string, command: string, args: readonly string[], options: SpawnOptions = {}): Promise { + const child = spawn(command, [...args], options); + await new Promise((resolveSpawn, rejectSpawn) => { + child.once('spawn', () => resolveSpawn()); + child.once('error', rejectSpawn); + }); + const pid = child.pid as number; + const identity = await processIdentity(pid); + if (identity === undefined) { + // A process that cannot be identified cannot be owned safely, so it is not left running. + child.kill('SIGKILL'); + throw new Error(`could not identify the process started for "${label}"; it was stopped`); + } + await recordOwned(root, { kind: 'process', pid, identity, label }); + return child; +} + +async function cleanProcess(resource: Extract, options: Required): Promise { + if (!isAlive(resource.pid)) return undefined; + const now = await options.identityOf(resource.pid); + if (now === undefined) { + // It may have exited while its identity was being read; otherwise it is alive and unidentifiable: hands off. + return isAlive(resource.pid) ? { resource, reason: 'identity-unknown' } : undefined; + } + if (now !== resource.identity) return undefined; // the pid was reused: the process this run owned is already gone + + const waitUntilGone = async (ms: number): Promise => { + const end = Date.now() + ms; + while (Date.now() < end) { + if (!isAlive(resource.pid)) return true; + await sleep(25); + } + return !isAlive(resource.pid); + }; + try { + process.kill(resource.pid); // polite on POSIX (SIGTERM); Windows has no polite form + } catch { + return undefined; + } + if (await waitUntilGone(options.graceMs)) return undefined; + if (process.platform !== 'win32') { + try { + process.kill(resource.pid, 'SIGKILL'); + } catch { + return undefined; + } + if (await waitUntilGone(2000)) return undefined; + } + return { resource, reason: 'still-running' }; +} + +async function cleanPath(root: string, resource: Extract): Promise { + const path = resolve(resource.path); + const link = await lstat(path).catch((error) => (errorCode(error) === 'ENOENT' ? undefined : Promise.reject(error))); + if (link === undefined) return undefined; // already gone + + // Resolve every link on the way, then judge the real location. A path that merely looks inside the root + // but leads out of it (through "..", a symlink or a junction) is refused, and its target is never touched. + const [realRoot, real] = await Promise.all([realpath(root), realpath(path)]); + if (fold(real) === fold(realRoot)) return { resource, reason: 'is-test-root' }; + if (!isInside(realRoot, real)) return { resource, reason: 'outside-test-root', detail: `${real} is not inside ${realRoot}` }; + // A link is not something this run creates for itself; removing one could be mistaken for removing its target. + if (link.isSymbolicLink()) return { resource, reason: 'is-link', detail: `${path} is a link to ${real}` }; + try { + await rm(path, { recursive: true, force: true }); + return undefined; + } catch (error) { + return { resource, reason: 'remove-failed', detail: error instanceof Error ? error.message : String(error) }; + } +} + +/** + * Removes what the ledger says this run owns, and only that. What it could not clean stays on the ledger, so the + * environment stays dirty until it is dealt with. + */ +export function cleanupOwnedResources(root: string, options: CleanupOptions = {}): Promise<{ removed: OwnedResource[]; failures: CleanupFailure[] }> { + const settings: Required = { graceMs: options.graceMs ?? 5000, identityOf: options.identityOf ?? processIdentity }; + return serialized(root, async () => { + const ledger = await readLedger(root); + const removed: OwnedResource[] = []; + const failures: CleanupFailure[] = []; + for (const resource of ledger) { + let failure: CleanupFailure | undefined; + try { + failure = resource.kind === 'process' ? await cleanProcess(resource, settings) : await cleanPath(root, resource); + } catch (error) { + failure = { resource, reason: 'remove-failed', detail: error instanceof Error ? error.message : String(error) }; + } + if (failure === undefined) removed.push(resource); + else failures.push(failure); + } + await writeLedger(root, failures.map((f) => f.resource)); + return { removed, failures }; + }); +} + +// --------------------------------------------------------------------------------------------------------------- +// Dirty environments + +/** Records that this environment must not be reused until it has been reset. */ +export async function markDirty(root: string, reason: string): Promise { + await writeFileAtomic(join(root, DIRTY_FILE), `${JSON.stringify({ schemaVersion: 1, reason }, null, 2)}\n`); +} + +export async function readDirty(root: string): Promise { + const text = await readFile(join(root, DIRTY_FILE), 'utf8').catch((error) => (errorCode(error) === 'ENOENT' ? undefined : Promise.reject(error))); + if (text === undefined) return undefined; + try { + return String((JSON.parse(text) as { reason?: unknown }).reason ?? 'marked dirty'); + } catch { + return 'marked dirty (the marker is unreadable)'; + } +} + +/** Reaps everything the ledger owns and, only when that succeeds completely, clears the dirty marker. */ +export async function resetDirtyEnvironment(root: string, options: CleanupOptions = {}): Promise<{ failures: CleanupFailure[] }> { + const { failures } = await cleanupOwnedResources(root, options); + if (failures.length === 0) await rm(join(root, DIRTY_FILE), { force: true }); + return { failures }; +} diff --git a/packages/qa/test/fixtures/processes.ts b/packages/qa/test/fixtures/processes.ts new file mode 100644 index 0000000..d3b108b --- /dev/null +++ b/packages/qa/test/fixtures/processes.ts @@ -0,0 +1,68 @@ +// Helpers for tests that need real processes. Every process started here is killed after the test, and none +// of them is ever handed to the code under test unless a test does so on purpose. +import { spawn, type ChildProcess } from 'node:child_process'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { EnvironmentProfile } from '../../src/model/project.ts'; +import { designateTestRoot } from '../../src/runner/resources.ts'; + +const started: ChildProcess[] = []; +const roots: string[] = []; + +/** A process unrelated to the runner, standing in for someone's real application. */ +export function startUnrelatedProcess(script = 'setInterval(() => {}, 1000)'): ChildProcess { + const child = spawn(process.execPath, ['-e', script], { stdio: 'ignore' }); + started.push(child); + return child; +} + +export function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +export async function eventually(check: () => boolean | Promise, timeoutMs = 10000): Promise { + const end = Date.now() + timeoutMs; + while (Date.now() < end) { + if (await check()) return; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error('condition not met in time'); +} + +/** A fresh, explicitly designated test root under the OS temp directory. */ +export async function makeTestRoot(designate = true): Promise { + const root = await mkdtemp(join(tmpdir(), 'qa-root-')); + roots.push(root); + if (designate) await designateTestRoot(root); + return root; +} + +/** A scratch directory outside any test root. Removed after the test, even when the test fails. */ +export async function makeTempDir(prefix: string): Promise { + const dir = await mkdtemp(join(tmpdir(), prefix)); + roots.push(dir); + return dir; +} + +export async function cleanUpProcessesAndRoots(): Promise { + for (const child of started) { + try { + child.kill('SIGKILL'); + } catch { + /* already gone */ + } + } + started.length = 0; + for (const root of roots.splice(0)) await rm(root, { recursive: true, force: true }); +} + +export const hostOs = (): 'windows' | 'linux' => (process.platform === 'win32' ? 'windows' : 'linux'); + +/** The profile of the machine the tests are running on. */ +export const hostProfile = (): EnvironmentProfile => ({ id: hostOs(), os: hostOs(), arch: 'x86_64' }); diff --git a/packages/qa/test/runner/environment.test.ts b/packages/qa/test/runner/environment.test.ts new file mode 100644 index 0000000..752107a --- /dev/null +++ b/packages/qa/test/runner/environment.test.ts @@ -0,0 +1,54 @@ +import { arch, release } from 'node:os'; +import { describe, expect, test } from 'vitest'; +import { defaultProbes, inspectEnvironment } from '../../src/runner/environment.ts'; +import { hostOs, hostProfile } from '../fixtures/processes.ts'; + +const probes = (display: boolean, audio: boolean) => ({ display: async () => display, audio: async () => audio }); + +describe('inspectEnvironment', () => { + test('measures the operating system, its version and the architecture of this machine', async () => { + const { environment } = await inspectEnvironment(hostProfile(), probes(true, true), '1.2.3'); + expect(environment.os).toBe(hostOs()); + expect(environment.osVersion).toBe(release()); + expect(environment.arch).toBe(arch() === 'x64' ? 'x86_64' : arch()); + expect(environment.toolVersion).toBe('1.2.3'); + }); + + test('reports a capability only when its probe says the machine has it', async () => { + expect((await inspectEnvironment(hostProfile(), probes(true, true))).environment.capabilities).toEqual(['audio', 'display']); + expect((await inspectEnvironment(hostProfile(), probes(true, false))).environment.capabilities).toEqual(['display']); + expect((await inspectEnvironment(hostProfile(), probes(false, true))).environment.capabilities).toEqual(['audio']); + expect((await inspectEnvironment(hostProfile(), probes(false, false))).environment.capabilities).toEqual([]); + }); + + test('a probe that throws counts as the capability being absent, not as a crash', async () => { + const broken = { display: async (): Promise => { throw new Error('no session'); }, audio: async () => true }; + expect((await inspectEnvironment(hostProfile(), broken)).environment.capabilities).toEqual(['audio']); + }); + + test('reports no mismatch for the profile of this machine', async () => { + const inspected = await inspectEnvironment(hostProfile(), probes(true, true)); + expect(inspected.environment.os).toBe(hostOs()); + expect(inspected.profileMismatch).toBeUndefined(); + }); + + test('reports a mismatch when the profile asks for another operating system', async () => { + const other = { ...hostProfile(), os: hostOs() === 'windows' ? ('linux' as const) : ('windows' as const) }; + const { profileMismatch } = await inspectEnvironment(other, probes(true, true)); + expect(profileMismatch).toContain(other.os); + expect(profileMismatch).toContain(hostOs()); + }); + + test('reports a mismatch when the profile asks for another architecture', async () => { + // The profile type only allows x86_64 today; a future profile for another architecture must still be refused here. + const other = { ...hostProfile(), arch: 'aarch64' } as unknown as ReturnType; + const { profileMismatch } = await inspectEnvironment(other, probes(true, true)); + expect(profileMismatch).toContain('aarch64'); + expect(profileMismatch).toContain('x86_64'); + }); + + test('the default probes answer with a boolean and never throw, whatever this machine has', async () => { + expect(typeof (await defaultProbes.display())).toBe('boolean'); + expect(typeof (await defaultProbes.audio())).toBe('boolean'); + }); +}); diff --git a/packages/qa/test/runner/execute.test.ts b/packages/qa/test/runner/execute.test.ts new file mode 100644 index 0000000..cda7f90 --- /dev/null +++ b/packages/qa/test/runner/execute.test.ts @@ -0,0 +1,356 @@ +import assert from 'node:assert'; +import { getEventListeners } from 'node:events'; +import { readFile, rm, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { afterEach, describe, expect, test } from 'vitest'; +import { AssertionFailure, executeScenario, type ExecutionContext, type Lifecycle, type Scenario, type ScenarioEvent } from '../../src/runner/execute.ts'; +import { readDirty, readLedger, resetDirtyEnvironment, spawnOwned } from '../../src/runner/resources.ts'; +import { candidate, requirement } from '../fixtures/records.ts'; +import { cleanUpProcessesAndRoots, eventually, hostOs, hostProfile, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess } from '../fixtures/processes.ts'; + +afterEach(cleanUpProcessesAndRoots); + +const sleeper = ['-e', 'setInterval(() => {}, 1000)']; +const never = () => new Promise(() => {}); + +function lifecycleOf(calls: string[], overrides: Partial = {}): Lifecycle { + const record = (name: string) => async () => { calls.push(name); }; + return { install: record('install'), reset: record('reset'), launch: record('launch'), cleanup: record('cleanup'), ...overrides }; +} + +async function arrange(overrides: Partial = {}) { + const testRoot = await makeTestRoot(); + const calls: string[] = []; + const events: ScenarioEvent[] = []; + const controller = new AbortController(); + const context: ExecutionContext = { + candidate: candidate(), + profile: hostProfile(), + testRoot, + signal: controller.signal, + emit: (event) => { events.push(event); }, + lifecycle: lifecycleOf(calls), + probes: { display: async () => true, audio: async () => true }, + timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 2000 }, + ...overrides, + }; + return { testRoot, calls, events, controller, context }; +} + +const scenarioOf = (overrides: Partial = {}): Scenario => ({ + id: 'persistence', + requirement: requirement({ key: `${hostOs()}/persistence` }), + steps: async () => {}, + ...overrides, +}); +const started = (events: readonly ScenarioEvent[]) => events.filter((e) => e.status === 'started').map((e) => e.phase); + +describe('a passing run', () => { + test('runs the phases in order, emits an event for each, and cleans up', async () => { + const { context, calls, events } = await arrange(); + const result = await executeScenario(context, scenarioOf({ setup: async () => { calls.push('setup'); }, steps: async () => { calls.push('steps'); } })); + + expect(result).toMatchObject({ outcome: 'passed', cleanup: { ok: true, failures: [] }, leftover: [] }); + expect(result.reason).toBeUndefined(); + expect(calls).toEqual(['install', 'reset', 'launch', 'setup', 'steps', 'cleanup']); + expect(started(events)).toEqual(['prerequisites', 'install', 'reset', 'launch', 'setup', 'steps', 'cleanup']); + expect(events.filter((e) => e.status !== 'started').map((e) => e.status)).toEqual(Array(7).fill('finished')); + expect(result.environment?.os).toBe(hostOs()); + }); + + test('skips the setup phase, and its events, when the scenario has none', async () => { + const { context, events } = await arrange(); + await executeScenario(context, scenarioOf()); + expect(started(events)).toContain('steps'); + expect(started(events)).not.toContain('setup'); + }); +}); + +describe('blocked: prerequisites that are not met', () => { + test('refuses to run anywhere that was not explicitly designated as a test environment', async () => { + const undesignated = await makeTestRoot(false); + const { context, calls } = await arrange({ testRoot: undesignated }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'blocked', reason: 'not-a-test-environment' }); + expect(calls).toEqual([]); + }); + + test('is blocked without installing anything when the display the scenario needs is missing', async () => { + const { context, calls, events } = await arrange({ probes: { display: async () => false, audio: async () => true } }); + const result = await executeScenario(context, scenarioOf({ requirement: requirement({ key: `${hostOs()}/persistence`, capabilities: ['display'] }) })); + expect(result).toMatchObject({ outcome: 'blocked', reason: 'capability-missing', missing: ['display'] }); + expect(calls).toEqual([]); + expect(started(events)).toEqual(['prerequisites']); + expect(events.at(-1)).toMatchObject({ phase: 'prerequisites', status: 'failed' }); + }); + + test('is blocked when the audio capability the scenario needs is missing', async () => { + const { context, calls } = await arrange({ probes: { display: async () => true, audio: async () => false } }); + const result = await executeScenario(context, scenarioOf({ requirement: requirement({ key: `${hostOs()}/persistence`, capabilities: ['audio'] }) })); + expect(result).toMatchObject({ outcome: 'blocked', reason: 'capability-missing', missing: ['audio'] }); + expect(calls).toEqual([]); + }); + + test('lists every missing capability, sorted', async () => { + const { context } = await arrange({ probes: { display: async () => false, audio: async () => false } }); + const result = await executeScenario(context, scenarioOf({ requirement: requirement({ key: `${hostOs()}/persistence`, capabilities: ['display', 'audio'] }) })); + expect(result.missing).toEqual(['audio', 'display']); + }); + + test('does not need a capability the scenario never asked for', async () => { + const { context } = await arrange({ probes: { display: async () => false, audio: async () => false } }); + expect((await executeScenario(context, scenarioOf())).outcome).toBe('passed'); + }); + + test('is blocked when the machine is not what the profile asks for', async () => { + const other = { ...hostProfile(), os: hostOs() === 'windows' ? ('linux' as const) : ('windows' as const) }; + const { context, calls } = await arrange({ profile: other }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'blocked', reason: 'wrong-environment' }); + expect(calls).toEqual([]); + }); +}); + +describe('a dirty environment', () => { + test('refuses the next run while a process from an earlier run is still owned, and leaves that process alone', async () => { + const { context, calls, testRoot } = await arrange(); + const leftover = await spawnOwned(testRoot, 'earlier helper', process.execPath, sleeper, { stdio: 'ignore' }); + + const refused = await executeScenario(context, scenarioOf()); + + expect(refused).toMatchObject({ outcome: 'blocked', reason: 'dirty-environment' }); + expect(calls).toEqual([]); + expect(isAlive(leftover.pid as number)).toBe(true); + }); + + test('accepts runs again once the environment has been reset', async () => { + const { context, testRoot } = await arrange(); + const leftover = await spawnOwned(testRoot, 'earlier helper', process.execPath, sleeper, { stdio: 'ignore' }); + expect((await executeScenario(context, scenarioOf())).outcome).toBe('blocked'); + + await resetDirtyEnvironment(testRoot, { graceMs: 300 }); + await eventually(() => !isAlive(leftover.pid as number)); + + expect((await executeScenario(context, scenarioOf())).outcome).toBe('passed'); + }); + + test('a failed cleanup hook leaves the environment marked dirty, so the next run is refused', async () => { + const calls: string[] = []; + const { context, testRoot } = await arrange({ lifecycle: lifecycleOf(calls, { cleanup: async () => { throw new Error('uninstaller crashed'); } }) }); + + const first = await executeScenario(context, scenarioOf()); + + expect(first.outcome).toBe('passed'); + expect(first.cleanup.ok).toBe(false); + expect(first.cleanup.failures.join(' ')).toContain('uninstaller crashed'); + expect(await readDirty(testRoot)).toContain('cleanup'); + expect((await executeScenario(context, scenarioOf())).reason).toBe('dirty-environment'); + + await resetDirtyEnvironment(testRoot); + expect(await readDirty(testRoot)).toBeUndefined(); + }); + + test('a cleanup hook that hangs is cut off and also leaves the environment dirty', async () => { + const { context, testRoot } = await arrange({ lifecycle: lifecycleOf([], { cleanup: never }), timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 100 } }); + const result = await executeScenario(context, scenarioOf()); + expect(result.cleanup.ok).toBe(false); + expect(result.cleanup.failures.join(' ')).toMatch(/timed out/); + expect(await readDirty(testRoot)).toBeDefined(); + }); +}); + +describe('failed: only an assertion says the candidate misbehaved', () => { + test('an assertion failure in the steps is a failed result, and cleanup still runs', async () => { + const { context, calls } = await arrange(); + const result = await executeScenario(context, scenarioOf({ steps: async () => { throw new AssertionFailure('saved value was not shown after restart'); } })); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed', detail: 'saved value was not shown after restart' }); + expect(calls.at(-1)).toBe('cleanup'); + }); + + test("node's own assertion errors count as assertion failures", async () => { + const { context } = await arrange(); + const result = await executeScenario(context, scenarioOf({ steps: async () => { assert.strictEqual(1, 2); } })); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed' }); + }); + + test('an assertion failure raised by a lifecycle hook is also a failure of the candidate', async () => { + const calls: string[] = []; + const { context, events } = await arrange({ lifecycle: lifecycleOf(calls, { launch: async () => { throw new AssertionFailure('the window never appeared'); } }) }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed' }); + expect(started(events)).not.toContain('steps'); + }); + + test('waitFor that runs out of time is an assertion failure naming what it waited for', async () => { + const { context } = await arrange(); + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => false, { timeoutMs: 60, intervalMs: 10, description: 'the saved value to appear' }) })); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed' }); + expect(result.detail).toContain('the saved value to appear'); + }); + + test('waiting through many polls does not pile up abort listeners on the signal', async () => { + // Timer resolution is coarse on Windows, so allow generously more time than 30 polls can take. + const { context } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 20000, cleanupMs: 2000 } }); + let listeners = Number.POSITIVE_INFINITY; + let polls = 0; + const result = await executeScenario(context, scenarioOf({ + steps: async (ctx) => { + await ctx.waitFor(() => ++polls > 30, { timeoutMs: 15000, intervalMs: 1 }); + listeners = getEventListeners(ctx.signal, 'abort').length; + }, + })); + expect(result.outcome).toBe('passed'); + expect(listeners).toBeLessThan(5); + }); + + test('waitFor returns as soon as the condition holds', async () => { + const { context } = await arrange(); + let polls = 0; + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => ++polls >= 3, { timeoutMs: 2000, intervalMs: 5 }) })); + expect(result.outcome).toBe('passed'); + expect(polls).toBe(3); + }); +}); + +describe('interrupted: trouble that is not the candidate', () => { + test('a launch that fails is an infrastructure interruption, not a failed candidate, and nothing after it runs', async () => { + const calls: string[] = []; + const { context, events } = await arrange({ lifecycle: lifecycleOf(calls, { launch: async () => { throw new Error('spawn ENOENT'); } }) }); + const result = await executeScenario(context, scenarioOf({ setup: async () => { calls.push('setup'); }, steps: async () => { calls.push('steps'); } })); + + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + expect(result.detail).toContain('spawn ENOENT'); + expect(calls).toEqual(['install', 'reset', 'cleanup']); + // The record never claims a phase that did not happen. + expect(started(events)).toEqual(['prerequisites', 'install', 'reset', 'launch', 'cleanup']); + expect(events.filter((e) => e.phase === 'launch').map((e) => e.status)).toEqual(['started', 'failed']); + }); + + test('an ordinary error thrown by the steps is an interruption, not a verdict on the candidate', async () => { + const { context } = await arrange(); + const result = await executeScenario(context, scenarioOf({ steps: async () => { throw new TypeError('cannot read properties of undefined'); } })); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + }); + + test('steps that run out of time are interrupted, cleanup runs, and the run returns promptly', async () => { + const { context, calls } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 80, cleanupMs: 2000 } }); + const began = Date.now(); + const result = await executeScenario(context, scenarioOf({ steps: never })); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'timeout' }); + expect(Date.now() - began).toBeLessThan(1500); + expect(calls.at(-1)).toBe('cleanup'); + }); + + test('an event sink that fails stops the run and is reported, but cleanup still happens', async () => { + const calls: string[] = []; + const { context } = await arrange({ + lifecycle: lifecycleOf(calls), + emit: (event) => { if (event.phase === 'launch' && event.status === 'started') throw new Error('disk full'); }, + }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + expect(result.detail).toMatch(/event sink/); + expect(result.detail).toContain('disk full'); + expect(calls).toContain('cleanup'); + }); +}); + +describe('cancelled', () => { + test('cancelling during install stops the run there and still cleans up, with a signal that is not itself cancelled', async () => { + const calls: string[] = []; + let cleanupSawCancellation: boolean | undefined; + const { context, controller } = await arrange({ + lifecycle: lifecycleOf(calls, { cleanup: async (ctx) => { calls.push('cleanup'); cleanupSawCancellation = ctx.signal.aborted; }, install: (ctx) => new Promise((_, reject) => { calls.push('install'); ctx.signal.addEventListener('abort', () => reject(new Error('stopped'))); }) }), + }); + const running = executeScenario(context, scenarioOf()); + await eventually(() => calls.includes('install')); + controller.abort(); + + const result = await running; + + expect(result).toMatchObject({ outcome: 'cancelled', reason: 'cancelled' }); + expect(calls).toEqual(['install', 'cleanup']); + expect(cleanupSawCancellation).toBe(false); + expect(result.cleanup.ok).toBe(true); + }); + + test('a hook that ignores cancellation cannot keep the run waiting', async () => { + const calls: string[] = []; + const { context, controller } = await arrange({ lifecycle: lifecycleOf(calls, { install: () => { calls.push('install'); return never(); } }) }); + const running = executeScenario(context, scenarioOf()); + await eventually(() => calls.includes('install')); + controller.abort(); + expect((await running).outcome).toBe('cancelled'); + expect(calls.at(-1)).toBe('cleanup'); + }); + + test('a run cancelled before it starts touches nothing', async () => { + const { context, calls, controller, events } = await arrange(); + controller.abort(); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'cancelled', reason: 'cancelled' }); + expect(calls).toEqual([]); + expect(events).toEqual([]); + }); + + test('cancelling while waiting in the steps ends the wait as a cancellation', async () => { + const { context, controller, calls } = await arrange(); + const running = executeScenario(context, scenarioOf({ steps: async (ctx) => { calls.push('waiting'); await ctx.waitFor(() => false, { timeoutMs: 5000, intervalMs: 10 }); } })); + await eventually(() => calls.includes('waiting')); + controller.abort(); + expect((await running).outcome).toBe('cancelled'); + }); +}); + +describe('owning only what the run created', () => { + test('stops a process the scenario spawned even if the cleanup hook forgot it, and leaves an unrelated process alone', async () => { + const { context, testRoot } = await arrange(); + const unrelated = startUnrelatedProcess(); + let spawnedPid = 0; + + const result = await executeScenario(context, scenarioOf({ + steps: async (ctx) => { + const child = await ctx.spawn('helper', process.execPath, sleeper, { stdio: 'ignore' }); + spawnedPid = child.pid as number; + }, + })); + + expect(result.outcome).toBe('passed'); + await eventually(() => !isAlive(spawnedPid)); + expect(isAlive(unrelated.pid as number)).toBe(true); + expect(await readLedger(testRoot)).toEqual([]); + }); + + test('reaps what the run owned even when the steps failed', async () => { + const { context } = await arrange(); + let spawnedPid = 0; + const result = await executeScenario(context, scenarioOf({ + steps: async (ctx) => { + spawnedPid = (await ctx.spawn('helper', process.execPath, sleeper, { stdio: 'ignore' })).pid as number; + throw new AssertionFailure('boom'); + }, + })); + expect(result.outcome).toBe('failed'); + await eventually(() => !isAlive(spawnedPid)); + }); + + test('a resource that cannot be safely removed is reported as left over and keeps the environment dirty', async () => { + const { context, testRoot } = await arrange(); + const outside = startUnrelatedProcess(); + // A throwaway directory outside the test root: never a real one, in case the code under test is wrong. + const notOurs = await makeTempDir('qa-not-ours-'); + await writeFile(join(notOurs, 'precious.txt'), 'keep me'); + const result = await executeScenario(context, scenarioOf({ + steps: async (ctx) => { + // Claiming a path outside the test root: cleanup must refuse to remove it. + await ctx.own({ kind: 'path', path: notOurs, label: 'not ours to delete' }); + }, + })); + expect(result.leftover.map((l) => l.reason)).toEqual(['outside-test-root']); + expect(result.cleanup.ok).toBe(false); + expect(await readDirty(testRoot)).toBeDefined(); + expect(isAlive(outside.pid as number)).toBe(true); + expect(await readFile(join(notOurs, 'precious.txt'), 'utf8')).toBe('keep me'); + await rm(notOurs, { recursive: true, force: true }); + }); +}); diff --git a/packages/qa/test/runner/resources.test.ts b/packages/qa/test/runner/resources.test.ts new file mode 100644 index 0000000..43377e8 --- /dev/null +++ b/packages/qa/test/runner/resources.test.ts @@ -0,0 +1,346 @@ +import { mkdir, readFile, rm, stat, symlink, writeFile } from 'node:fs/promises'; +import { homedir, tmpdir } from 'node:os'; +import { join, parse } from 'node:path'; +import { afterEach, describe, expect, test } from 'vitest'; +import { + checkTestRoot, + cleanupOwnedResources, + designateTestRoot, + markDirty, + processIdentity, + readDirty, + readLedger, + recordOwned, + resetDirtyEnvironment, + spawnOwned, + TEST_ROOT_MARKER, +} from '../../src/runner/resources.ts'; +import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess } from '../fixtures/processes.ts'; + +afterEach(cleanUpProcessesAndRoots); + +const exists = (path: string) => stat(path).then(() => true, () => false); +const sleeper = ['-e', 'setInterval(() => {}, 1000)']; + +describe('designating a test root', () => { + test('a designated directory is accepted and reported by its resolved path', async () => { + const root = await makeTestRoot(); + const check = await checkTestRoot(root); + expect(check.ok).toBe(true); + expect(await readFile(join(root, TEST_ROOT_MARKER), 'utf8')).toContain('release-qa'); + }); + + test('a directory nobody designated is refused, however harmless it looks', async () => { + const root = await makeTestRoot(false); + expect(await checkTestRoot(root)).toEqual({ ok: false, reason: 'missing-marker' }); + await designateTestRoot(root); + expect((await checkTestRoot(root)).ok).toBe(true); + }); + + test.each([ + ['a file of the same name from another tool', JSON.stringify({ purpose: 'something-else' })], + ['a marker that is not JSON', 'not json'], + ['an empty marker', ''], + ])('a marker holding %s does not designate a test root', async (_label, content) => { + const root = await makeTestRoot(false); + await writeFile(join(root, TEST_ROOT_MARKER), content); + expect(await checkTestRoot(root)).toEqual({ ok: false, reason: 'missing-marker' }); + }); + + test('a path that is not a directory is refused', async () => { + const root = await makeTestRoot(); + const file = join(root, 'a-file'); + await writeFile(file, 'x'); + expect(await checkTestRoot(file)).toEqual({ ok: false, reason: 'not-a-directory' }); + expect(await checkTestRoot(join(root, 'missing'))).toEqual({ ok: false, reason: 'not-a-directory' }); + }); + + // These tests never write into a real home directory: a wrong implementation would mark it, and that is + // exactly the mistake they exist to catch. They use a throwaway "home" and only ever *read* the real one. + test('the real home directory and the filesystem root are never a test root (read-only check)', async () => { + expect(await checkTestRoot(homedir())).toEqual({ ok: false, reason: 'unsafe-root' }); + expect(await checkTestRoot(parse(tmpdir()).root)).toEqual({ ok: false, reason: 'unsafe-root' }); + }); + + test('the home directory, and a directory that contains it, are never a test root, marker or not', async () => { + const outer = await makeTempDir('qa-outer-'); + const home = join(outer, 'home'); + await mkdir(home); + expect(await checkTestRoot(home, { home })).toEqual({ ok: false, reason: 'unsafe-root' }); + expect(await checkTestRoot(outer, { home })).toEqual({ ok: false, reason: 'unsafe-root' }); + // A directory inside the home directory is fine. + const inside = join(home, 'qa-tests'); + await mkdir(inside); + await designateTestRoot(inside, { home }); + expect((await checkTestRoot(inside, { home })).ok).toBe(true); + await rm(outer, { recursive: true, force: true }); + }); + + test('designating refuses to mark the home directory or a directory that contains it, and writes nothing', async () => { + const outer = await makeTempDir('qa-outer-'); + const home = join(outer, 'home'); + await mkdir(home); + await expect(designateTestRoot(home, { home })).rejects.toThrow(/unsafe/i); + await expect(designateTestRoot(outer, { home })).rejects.toThrow(/unsafe/i); + expect(await exists(join(home, TEST_ROOT_MARKER))).toBe(false); + expect(await exists(join(outer, TEST_ROOT_MARKER))).toBe(false); + await rm(outer, { recursive: true, force: true }); + }); +}); + +describe('the ledger of owned resources', () => { + test('a recorded resource is on disk before anything else can happen', async () => { + const root = await makeTestRoot(); + await recordOwned(root, { kind: 'path', path: join(root, 'app'), label: 'install dir' }); + expect(await readLedger(root)).toEqual([{ kind: 'path', path: join(root, 'app'), label: 'install dir' }]); + }); + + test('an empty or missing ledger reads as empty', async () => { + expect(await readLedger(await makeTestRoot())).toEqual([]); + }); + + test.each([ + ['a process without a pid', { kind: 'process', identity: 'x', label: 'p' }], + ['a process without an identity', { kind: 'process', pid: 42, label: 'p' }], + ['a path that is not a string', { kind: 'path', path: 7, label: 'p' }], + ['an unknown kind', { kind: 'socket', label: 'p' }], + ['something that is not an object', 'a string'], + ])('a ledger holding %s is refused instead of being trusted', async (_label, entry) => { + const root = await makeTestRoot(); + await writeFile(join(root, '.release-qa-owned.json'), JSON.stringify({ schemaVersion: 1, resources: [entry] })); + await expect(readLedger(root)).rejects.toThrow(/ledger/); + }); + + test('a ledger that is unreadable is not silently treated as empty', async () => { + const root = await makeTestRoot(); + await writeFile(join(root, '.release-qa-owned.json'), '{ not json'); + await expect(readLedger(root)).rejects.toThrow(); + }); +}); + +describe('spawning owned processes', () => { + test('records the process and its identity in the ledger before handing it back', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + const [entry] = await readLedger(root); + expect(entry).toMatchObject({ kind: 'process', pid: child.pid, label: 'helper' }); + expect(entry?.kind === 'process' && entry.identity.length).toBeGreaterThan(0); + expect(isAlive(child.pid as number)).toBe(true); + }); + + test('the identity of a live process is stable and differs between processes', async () => { + const a = startUnrelatedProcess(); + const b = startUnrelatedProcess(); + const first = await processIdentity(a.pid as number); + expect(first).toBeDefined(); + expect(await processIdentity(a.pid as number)).toBe(first); + expect(await processIdentity(b.pid as number)).not.toBe(first); + }); + + test('a process that does not exist has no identity', async () => { + const child = startUnrelatedProcess(); + const pid = child.pid as number; + expect(await processIdentity(pid)).toBeDefined(); + child.kill('SIGKILL'); + await eventually(() => !isAlive(pid)); + expect(await processIdentity(pid)).toBeUndefined(); + }); +}); + +describe('cleaning up', () => { + test('stops the processes it owns and leaves an unrelated process running', async () => { + const root = await makeTestRoot(); + const owned = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + const unrelated = startUnrelatedProcess(); + + const result = await cleanupOwnedResources(root, { graceMs: 500 }); + + expect(result.failures).toEqual([]); + expect(result.removed).toHaveLength(1); + await eventually(() => !isAlive(owned.pid as number)); + expect(isAlive(unrelated.pid as number)).toBe(true); + expect(await readLedger(root)).toEqual([]); + }); + + test('never signals a process whose pid was reused by something else', async () => { + const root = await makeTestRoot(); + const stranger = startUnrelatedProcess(); + // The ledger remembers a process that once had this pid; the pid now belongs to a stranger. + await recordOwned(root, { kind: 'process', pid: stranger.pid as number, identity: 'the-original-process', label: 'gone' }); + expect(await readLedger(root)).toHaveLength(1); + + const result = await cleanupOwnedResources(root, { graceMs: 200 }); + + expect(isAlive(stranger.pid as number)).toBe(true); + expect(result.failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + + test('refuses to kill a live process it cannot identify, and keeps it on the ledger', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + const result = await cleanupOwnedResources(root, { graceMs: 200, identityOf: async () => undefined }); + expect(isAlive(child.pid as number)).toBe(true); + expect(result.failures.map((f) => f.reason)).toEqual(['identity-unknown']); + expect(await readLedger(root)).toHaveLength(1); + }); + + test('a process that already exited is dropped from the ledger without a failure', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + child.kill('SIGKILL'); + await eventually(() => !isAlive(child.pid as number)); + expect((await cleanupOwnedResources(root)).failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + + test.skipIf(process.platform === 'win32')('forces a process that ignores the polite request', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'stubborn', process.execPath, ['-e', "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)"], { stdio: 'ignore' }); + await new Promise((resolve) => setTimeout(resolve, 300)); + const result = await cleanupOwnedResources(root, { graceMs: 200 }); + expect(result.failures).toEqual([]); + await eventually(() => !isAlive(child.pid as number)); + }); + + test('removes directories and files it owns inside the test root', async () => { + const root = await makeTestRoot(); + const dir = join(root, 'install'); + await mkdir(join(dir, 'nested'), { recursive: true }); + await writeFile(join(dir, 'nested', 'app.exe'), 'x'); + const file = join(root, 'settings.json'); + await writeFile(file, '{}'); + await recordOwned(root, { kind: 'path', path: dir, label: 'install' }); + await recordOwned(root, { kind: 'path', path: file, label: 'settings' }); + + const result = await cleanupOwnedResources(root); + + expect(result.failures).toEqual([]); + expect(await exists(dir)).toBe(false); + expect(await exists(file)).toBe(false); + expect(await exists(root)).toBe(true); + expect(await readLedger(root)).toEqual([]); + }); + + test('a path that is already gone is not a failure', async () => { + const root = await makeTestRoot(); + await recordOwned(root, { kind: 'path', path: join(root, 'never-created'), label: 'x' }); + expect(await readLedger(root)).toHaveLength(1); + expect((await cleanupOwnedResources(root)).failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + + test('refuses a path outside the test root and leaves it untouched', async () => { + const root = await makeTestRoot(); + const outside = await makeTempDir('qa-outside-'); + await writeFile(join(outside, 'precious.txt'), 'keep me'); + await recordOwned(root, { kind: 'path', path: outside, label: 'wrong' }); + + const result = await cleanupOwnedResources(root); + + expect(result.failures.map((f) => f.reason)).toEqual(['outside-test-root']); + expect(await readFile(join(outside, 'precious.txt'), 'utf8')).toBe('keep me'); + expect(await readLedger(root)).toHaveLength(1); + await rm(outside, { recursive: true, force: true }); + }); + + test('refuses to remove the test root itself', async () => { + const root = await makeTestRoot(); + await recordOwned(root, { kind: 'path', path: root, label: 'the root' }); + const result = await cleanupOwnedResources(root); + expect(result.failures.map((f) => f.reason)).toEqual(['is-test-root']); + expect(await exists(join(root, TEST_ROOT_MARKER))).toBe(true); + }); + + test('refuses a path that reaches outside the root through ".."', async () => { + const root = await makeTestRoot(); + const sibling = await makeTempDir('qa-sibling-'); + await writeFile(join(sibling, 'precious.txt'), 'keep me'); + await recordOwned(root, { kind: 'path', path: join(root, '..', sibling.split(/[\\/]/).pop() as string), label: 'sneaky' }); + const result = await cleanupOwnedResources(root); + expect(result.failures.map((f) => f.reason)).toEqual(['outside-test-root']); + expect(await readFile(join(sibling, 'precious.txt'), 'utf8')).toBe('keep me'); + await rm(sibling, { recursive: true, force: true }); + }); + + test('refuses a directory inside the root that is really a link to somewhere else, and leaves the target alone', async () => { + const root = await makeTestRoot(); + const target = await makeTempDir('qa-target-'); + await writeFile(join(target, 'precious.txt'), 'keep me'); + const link = join(root, 'looks-inside'); + await symlink(target, link, process.platform === 'win32' ? 'junction' : 'dir'); + await recordOwned(root, { kind: 'path', path: link, label: 'link' }); + + const result = await cleanupOwnedResources(root); + + expect(result.failures.map((f) => f.reason)).toEqual(['outside-test-root']); + expect(await readFile(join(target, 'precious.txt'), 'utf8')).toBe('keep me'); + await rm(target, { recursive: true, force: true }); + }); + + test('refuses a link even when it points at somewhere else inside the root, and leaves both alone', async () => { + const root = await makeTestRoot(); + const target = join(root, 'real'); + await mkdir(target); + await writeFile(join(target, 'keep.txt'), 'keep me'); + const link = join(root, 'alias'); + await symlink(target, link, process.platform === 'win32' ? 'junction' : 'dir'); + await recordOwned(root, { kind: 'path', path: link, label: 'alias' }); + + const result = await cleanupOwnedResources(root); + + expect(result.failures.map((f) => f.reason)).toEqual(['is-link']); + expect(await readFile(join(target, 'keep.txt'), 'utf8')).toBe('keep me'); + expect(await exists(link)).toBe(true); + }); + + test('keeps only the resources it could not clean on the ledger', async () => { + const root = await makeTestRoot(); + const fine = join(root, 'fine'); + await mkdir(fine); + const outside = await makeTempDir('qa-outside-'); + await recordOwned(root, { kind: 'path', path: fine, label: 'fine' }); + await recordOwned(root, { kind: 'path', path: outside, label: 'outside' }); + + const result = await cleanupOwnedResources(root); + + expect(result.removed.map((r) => r.label)).toEqual(['fine']); + expect((await readLedger(root)).map((r) => r.label)).toEqual(['outside']); + await rm(outside, { recursive: true, force: true }); + }); +}); + +describe('a dirty environment', () => { + test('is dirty when a marker says so, with the reason', async () => { + const root = await makeTestRoot(); + expect(await readDirty(root)).toBeUndefined(); + await markDirty(root, 'cleanup hook failed'); + expect(await readDirty(root)).toBe('cleanup hook failed'); + }); + + test('is cleaned by a reset: owned processes stop, the marker clears, the ledger empties', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + await markDirty(root, 'runner crashed'); + + const result = await resetDirtyEnvironment(root, { graceMs: 300 }); + + expect(result.failures).toEqual([]); + await eventually(() => !isAlive(child.pid as number)); + expect(await readDirty(root)).toBeUndefined(); + expect(await readLedger(root)).toEqual([]); + }); + + test('stays dirty when a reset could not clean everything', async () => { + const root = await makeTestRoot(); + const outside = await makeTempDir('qa-outside-'); + await recordOwned(root, { kind: 'path', path: outside, label: 'outside' }); + await markDirty(root, 'x'); + + const result = await resetDirtyEnvironment(root); + + expect(result.failures).toHaveLength(1); + expect(await readDirty(root)).toBeDefined(); + await rm(outside, { recursive: true, force: true }); + }); +}); From a773615ca0e69dd1612a2922d9471a98925cb27b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:07:16 +0200 Subject: [PATCH 2/7] fix(runner): bound every wait, fail closed on dirty state, lock the test root (Task 2.1 review) Addresses the review of the first executor: - every wait is bounded and abortable: prerequisites, probes, the event sink, waitFor conditions - hooks that outlive their phase are tracked, refused new ownership, and leave the environment dirty if they never stop - one run at a time per test root (lock file with process identity, stale locks taken over) - a dirty marker that cannot be written is remembered in memory; an unreadable marker or ledger blocks the run - cleanup event-sink failures are reported in the result and mark the environment dirty - escalation to SIGKILL re-checks the process identity first; failed signals keep the resource as still-running - spawnOwned stops a child it cannot record, and tolerates commands that exit at once - probes that throw synchronously count as absent capabilities - test fixtures no longer leak processes or directories, and no longer assume an x86_64 host Co-Authored-By: Claude Sonnet 5 --- packages/qa/src/runner/environment.ts | 3 +- packages/qa/src/runner/execute.ts | 367 ++++++++++++------ packages/qa/src/runner/resources.ts | 151 +++++-- packages/qa/test/fixtures/execution.ts | 40 ++ packages/qa/test/fixtures/processes.ts | 30 +- packages/qa/test/runner/environment.test.ts | 20 +- .../qa/test/runner/execute-hardening.test.ts | 279 +++++++++++++ .../qa/test/runner/execute-late-lock.test.ts | 42 ++ packages/qa/test/runner/execute.test.ts | 4 +- .../test/runner/resources-hardening.test.ts | 212 ++++++++++ packages/qa/test/runner/resources.test.ts | 8 +- 11 files changed, 997 insertions(+), 159 deletions(-) create mode 100644 packages/qa/test/fixtures/execution.ts create mode 100644 packages/qa/test/runner/execute-hardening.test.ts create mode 100644 packages/qa/test/runner/execute-late-lock.test.ts create mode 100644 packages/qa/test/runner/resources-hardening.test.ts diff --git a/packages/qa/src/runner/environment.ts b/packages/qa/src/runner/environment.ts index ff1a6f9..ffa0c3f 100644 --- a/packages/qa/src/runner/environment.ts +++ b/packages/qa/src/runner/environment.ts @@ -48,7 +48,8 @@ const ARCH_NAMES: Record = { x64: 'x86_64', arm64: 'aarch64' }; /** Measures this machine. A probe that throws means the capability is absent, never a crash of the run. */ export async function inspectEnvironment(profile: EnvironmentProfile, probes: EnvironmentProbes = defaultProbes, toolVersion = '0.0.0'): Promise { - const has = async (probe: () => Promise): Promise => probe().catch(() => false); + // Run inside a promise so a probe that throws before returning one is caught too, not only one that rejects. + const has = (probe: () => Promise): Promise => Promise.resolve().then(probe).catch(() => false); const [display, audio] = await Promise.all([has(() => probes.display()), has(() => probes.audio())]); const os = OS_NAMES[platform()] ?? platform(); diff --git a/packages/qa/src/runner/execute.ts b/packages/qa/src/runner/execute.ts index 9fda0d7..174fcbb 100644 --- a/packages/qa/src/runner/execute.ts +++ b/packages/qa/src/runner/execute.ts @@ -4,7 +4,19 @@ import type { EnvironmentProfile } from '../model/project.ts'; import type { Requirement, RequirementKey } from '../model/requirement.ts'; import type { MeasuredEnvironment, Outcome } from '../model/result.ts'; import { defaultProbes, inspectEnvironment, type EnvironmentProbes } from './environment.ts'; -import { checkTestRoot, cleanupOwnedResources, markDirty, readDirty, readLedger, recordOwned, spawnOwned, type CleanupFailure, type OwnedResource } from './resources.ts'; +import { + acquireTestRoot, + checkTestRoot, + cleanupOwnedResources, + markDirty, + readDirty, + readLedger, + recordOwned, + spawnOwned, + type CleanupFailure, + type OwnedResource, + type RootLock, +} from './resources.ts'; /** Thrown by a scenario or hook to say the candidate behaved wrongly. The only thing that makes a result `failed`. */ export class AssertionFailure extends Error { @@ -29,7 +41,9 @@ export interface RunContext { profile: EnvironmentProfile; testRoot: string; signal: AbortSignal; + /** Records a resource this run created. Refused once the phase that asked has ended. */ own(resource: OwnedResource): Promise; + /** Starts a process the run owns. Refused once the phase that asked has ended. */ spawn(label: string, command: string, args: readonly string[], options?: SpawnOptions): Promise; /** Polls until `condition` is true. Running out of time is an assertion failure; abort is a cancellation. */ waitFor(condition: () => boolean | Promise, options?: { timeoutMs?: number; intervalMs?: number; description?: string }): Promise; @@ -58,12 +72,16 @@ export interface ExecutionContext { emit(event: ScenarioEvent): void | Promise; lifecycle: Lifecycle; probes?: EnvironmentProbes; - /** Bounds in milliseconds. Every wait is bounded; these override the defaults. */ - timeouts?: { phaseMs?: number; stepsMs?: number; cleanupMs?: number }; + /** + * Bounds in milliseconds; every wait is bounded, including probes and the event sink. `abandonedGraceMs` is how + * long a hook that was cut off gets to stop by itself before the environment is declared dirty. + */ + timeouts?: { phaseMs?: number; stepsMs?: number; cleanupMs?: number; abandonedGraceMs?: number }; } export type ResultReason = | 'not-a-test-environment' + | 'environment-busy' | 'dirty-environment' | 'wrong-environment' | 'capability-missing' @@ -89,6 +107,7 @@ export interface ScenarioResult { const DEFAULT_PHASE_MS = 120_000; const DEFAULT_STEPS_MS = 300_000; const DEFAULT_CLEANUP_MS = 60_000; +const DEFAULT_ABANDONED_GRACE_MS = 1_000; /** Why a phase stopped early. These are reasons, not verdicts on the candidate. */ class Cancelled extends Error { @@ -97,8 +116,12 @@ class Cancelled extends Error { } } class TimedOut extends Error { - constructor(readonly phase: Phase, readonly ms: number) { + readonly phase: Phase; + readonly ms: number; + constructor(phase: Phase, ms: number) { super(`${phase} timed out after ${ms} ms`); + this.phase = phase; + this.ms = ms; } } class SinkFailed extends Error { @@ -109,6 +132,7 @@ class SinkFailed extends Error { const withEnvironment = (environment: MeasuredEnvironment | undefined): { environment?: MeasuredEnvironment } => (environment === undefined ? {} : { environment }); const message = (error: unknown): string => (error instanceof Error ? error.message : String(error)); +const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); /** Node's assert and most test libraries throw errors named AssertionError; those are assertions too. */ function isAssertionFailure(error: unknown): boolean { @@ -130,13 +154,55 @@ function classify(phase: Phase, error: unknown): Stop { return { outcome: 'interrupted', reason: 'infrastructure-error', detail: `${phase}: ${message(error)}` }; } +/** Work that was cut off before it finished. It may still be doing things, which the run must not ignore. */ +interface Abandoned { + phase: Phase; + settled: boolean; + done: Promise; +} + +/** + * Races `work` against a deadline and an outer abort. `onTimeout` supplies the error for the deadline. When the work + * loses the race it is not stopped (JavaScript cannot), so it is reported through `abandoned` if it has not settled. + */ +function race(work: (signal: AbortSignal) => Promise, ms: number, outer: AbortSignal, onTimeout: () => Error, phase: Phase, abandoned?: Abandoned[]): Promise { + const controller = new AbortController(); + const stop = (reason: Error): void => { + if (!controller.signal.aborted) controller.abort(reason); + }; + const onOuterAbort = (): void => stop(new Cancelled()); + outer.addEventListener('abort', onOuterAbort); + const timer = setTimeout(() => stop(onTimeout()), ms); + + const record: Abandoned = { phase, settled: false, done: Promise.resolve() }; + const running = Promise.resolve().then(() => work(controller.signal)); + record.done = running.then( + () => { record.settled = true; }, + () => { record.settled = true; }, + ); + const interrupted = new Promise((_, reject) => { + const fail = (): void => reject(controller.signal.reason); + if (controller.signal.aborted) fail(); + else controller.signal.addEventListener('abort', fail, { once: true }); + }); + interrupted.catch(() => undefined); // no unhandled rejection when the work finishes first + if (outer.aborted) stop(new Cancelled()); + + return Promise.race([running, interrupted]).finally(() => { + clearTimeout(timer); + outer.removeEventListener('abort', onOuterAbort); + if (controller.signal.aborted && !record.settled) abandoned?.push(record); + }); +} + /** * Runs one scenario against one candidate in a designated test environment. * * Outcomes: a prerequisite that is not met is `blocked` and nothing is installed; an assertion failure is `failed`; * a cancellation is `cancelled`; a timeout, a hook that breaks, or an unexpected error is `interrupted`, because - * trouble in the infrastructure says nothing about the candidate. Cleanup runs whenever anything was started, and - * a cleanup that does not complete leaves the environment marked dirty until it is reset. + * trouble in the infrastructure says nothing about the candidate. Only one run at a time may use a test root. + * Cleanup runs whenever anything was started, and a cleanup that does not complete (including hooks that were cut + * off and may still be running) leaves the environment marked dirty until it is reset. */ export async function executeScenario(context: ExecutionContext, scenario: Scenario): Promise { const scenarioId = scenario.id; @@ -144,104 +210,163 @@ export async function executeScenario(context: ExecutionContext, scenario: Scena const finish = (result: Pick & Partial): ScenarioResult => ({ ...base, ...result }); if (context.signal.aborted) return finish({ outcome: 'cancelled', reason: 'cancelled' }); - // Once the event sink fails nothing more is sent to it: the run stops and the failure is reported. - let sinkFailure: SinkFailed | undefined; - const emit = async (phase: Phase, status: ScenarioEvent['status'], detail?: string): Promise => { + const phaseMs = context.timeouts?.phaseMs ?? DEFAULT_PHASE_MS; + const stepsMs = context.timeouts?.stepsMs ?? DEFAULT_STEPS_MS; + const cleanupMs = context.timeouts?.cleanupMs ?? DEFAULT_CLEANUP_MS; + const graceMs = context.timeouts?.abandonedGraceMs ?? DEFAULT_ABANDONED_GRACE_MS; + + // Once the event sink fails, nothing more is sent to it: the run stops and the failure is reported. A sink that + // never answers is cut off like anything else, so it cannot hold up cancellation or cleanup. + let sinkFailure: Error | undefined; + const emitOn = (signal: AbortSignal, limitMs: number) => async (phase: Phase, status: ScenarioEvent['status'], detail?: string): Promise => { if (sinkFailure !== undefined) return; try { - await context.emit({ scenario: scenarioId, phase, status, ...(detail === undefined ? {} : { detail }) }); + await race( + () => Promise.resolve().then(() => context.emit({ scenario: scenarioId, phase, status, ...(detail === undefined ? {} : { detail }) })), + limitMs, + signal, + () => new Error(`timed out after ${limitMs} ms`), + phase, + ); } catch (error) { - sinkFailure = new SinkFailed(error); + sinkFailure = error instanceof Cancelled ? error : new SinkFailed(error); throw sinkFailure; } }; + const emit = emitOn(context.signal, phaseMs); - // -- Prerequisites: nothing is installed or launched unless all of these hold. ------------------------------------ + const abandoned: Abandoned[] = []; + let lock: Extract | undefined; + let closed = false; // set when the run is over, so a late lock acquisition by cut-off work is given straight back let environment: MeasuredEnvironment | undefined; - let root: string; + try { - await emit('prerequisites', 'started'); - const designated = await checkTestRoot(context.testRoot); - if (!designated.ok) { - await emit('prerequisites', 'failed', designated.reason); - return finish({ outcome: 'blocked', reason: 'not-a-test-environment', detail: `${context.testRoot}: ${designated.reason}` }); - } - root = designated.root; + // -- Prerequisites: nothing is installed or launched unless all of these hold. ------------------------------ + type Prerequisites = { ready: true; root: string } | { ready: false; result: ScenarioResult }; + let prerequisites: Prerequisites; + try { + prerequisites = await race( + async () => { + await emit('prerequisites', 'started'); + const designated = await checkTestRoot(context.testRoot); + if (!designated.ok) { + await emit('prerequisites', 'failed', designated.reason); + return { ready: false, result: finish({ outcome: 'blocked', reason: 'not-a-test-environment', detail: `${context.testRoot}: ${designated.reason}` }) }; + } + const root = designated.root; - const dirty = await dirtiness(root); - if (dirty !== undefined) { - await emit('prerequisites', 'failed', dirty); - return finish({ outcome: 'blocked', reason: 'dirty-environment', detail: dirty }); - } + const acquired = await acquireTestRoot(root); + if (!acquired.ok) { + await emit('prerequisites', 'failed', acquired.heldBy); + return { ready: false, result: finish({ outcome: 'blocked', reason: 'environment-busy', detail: `the test root is in use by ${acquired.heldBy}` }) }; + } + if (closed) { + await acquired.release(); // the prerequisites were cut off and the run is over; do not keep a lock nobody will free + return { ready: false, result: finish({ outcome: 'interrupted', reason: 'infrastructure-error' }) }; + } + lock = acquired; - const inspected = await inspectEnvironment(context.profile, context.probes ?? defaultProbes); - environment = inspected.environment; - if (inspected.profileMismatch !== undefined) { - await emit('prerequisites', 'failed', inspected.profileMismatch); - return finish({ outcome: 'blocked', reason: 'wrong-environment', detail: inspected.profileMismatch, ...withEnvironment(environment) }); - } - const missing = scenario.requirement.capabilities.filter((c) => !inspected.environment.capabilities.includes(c)).sort(); - if (missing.length > 0) { - await emit('prerequisites', 'failed', `missing: ${missing.join(', ')}`); - return finish({ outcome: 'blocked', reason: 'capability-missing', missing, detail: `missing capabilities: ${missing.join(', ')}`, ...withEnvironment(environment) }); + const dirty = await dirtiness(root); + if (dirty !== undefined) { + await emit('prerequisites', 'failed', dirty); + return { ready: false, result: finish({ outcome: 'blocked', reason: 'dirty-environment', detail: dirty }) }; + } + + const inspected = await inspectEnvironment(context.profile, context.probes ?? defaultProbes); + environment = inspected.environment; + if (inspected.profileMismatch !== undefined) { + await emit('prerequisites', 'failed', inspected.profileMismatch); + return { ready: false, result: finish({ outcome: 'blocked', reason: 'wrong-environment', detail: inspected.profileMismatch, ...withEnvironment(environment) }) }; + } + const missing = scenario.requirement.capabilities.filter((c) => !inspected.environment.capabilities.includes(c)).sort(); + if (missing.length > 0) { + await emit('prerequisites', 'failed', `missing: ${missing.join(', ')}`); + return { ready: false, result: finish({ outcome: 'blocked', reason: 'capability-missing', missing, detail: `missing capabilities: ${missing.join(', ')}`, ...withEnvironment(environment) }) }; + } + await emit('prerequisites', 'finished'); + return { ready: true, root }; + }, + phaseMs, + context.signal, + () => new TimedOut('prerequisites', phaseMs), + 'prerequisites', + abandoned, + ); + } catch (error) { + return finish({ ...classify('prerequisites', error), ...withEnvironment(environment) }); } - await emit('prerequisites', 'finished'); - } catch (error) { - return finish({ ...classify('prerequisites', error), ...withEnvironment(environment) }); - } + if (!prerequisites.ready) return prerequisites.result; + const root = prerequisites.root; - // -- The lifecycle. Each phase is bounded and abortable; a phase that fails ends the run. ------------------------- - const phaseMs = context.timeouts?.phaseMs ?? DEFAULT_PHASE_MS; - const stepsMs = context.timeouts?.stepsMs ?? DEFAULT_STEPS_MS; - const cleanupMs = context.timeouts?.cleanupMs ?? DEFAULT_CLEANUP_MS; - const { lifecycle } = context; - const contextFor = (signal: AbortSignal): RunContext => ({ - candidate: context.candidate, - profile: context.profile, - testRoot: root, - signal, - own: (resource) => recordOwned(root, resource), - spawn: (label, command, args, options) => spawnOwned(root, label, command, args, options), - waitFor: (condition, options) => waitFor(signal, condition, options), - }); + // -- The lifecycle. Each phase is bounded and abortable; a phase that fails ends the run. -------------------- + const { lifecycle } = context; + const contextFor = (signal: AbortSignal): RunContext => { + // A phase that has ended can no longer create anything: a hook that was cut off but is still running must not + // leave a process or a resource behind after cleanup has looked at the ledger. + const assertActive = (): void => { + if (signal.aborted) throw new Error('this run has been stopped and can no longer take ownership of anything'); + }; + return { + candidate: context.candidate, + profile: context.profile, + testRoot: root, + signal, + own: async (resource) => { + assertActive(); + await recordOwned(root, resource); + }, + spawn: async (label, command, args, options) => { + assertActive(); + return spawnOwned(root, label, command, args, options); + }, + waitFor: (condition, options) => waitFor(signal, condition, options), + }; + }; - const phases: Array<[Phase, number, (ctx: RunContext) => Promise]> = [ - ['install', phaseMs, (ctx) => lifecycle.install(ctx)], - ['reset', phaseMs, (ctx) => lifecycle.reset(ctx)], - ['launch', phaseMs, (ctx) => lifecycle.launch(ctx)], - ...(scenario.setup === undefined ? [] : [['setup', phaseMs, (ctx: RunContext) => scenario.setup!(ctx)] as [Phase, number, (ctx: RunContext) => Promise]]), - ['steps', stepsMs, (ctx) => scenario.steps(ctx)], - ]; - - let stop: Stop | undefined; - let touched = false; - for (const [phase, limitMs, run] of phases) { - if (context.signal.aborted) { - stop = { outcome: 'cancelled', reason: 'cancelled', detail: `cancelled before ${phase}` }; - break; - } - touched = true; - try { - await emit(phase, 'started'); - await bounded(phase, limitMs, context.signal, (signal) => run(contextFor(signal))); - await emit(phase, 'finished'); - } catch (error) { - stop = classify(phase, error); - await emit(phase, 'failed', stop.detail).catch(() => undefined); - break; + const phases: Array<[Phase, number, (ctx: RunContext) => Promise]> = [ + ['install', phaseMs, (ctx) => lifecycle.install(ctx)], + ['reset', phaseMs, (ctx) => lifecycle.reset(ctx)], + ['launch', phaseMs, (ctx) => lifecycle.launch(ctx)], + ...(scenario.setup === undefined ? [] : [['setup', phaseMs, (ctx: RunContext) => scenario.setup!(ctx)] as [Phase, number, (ctx: RunContext) => Promise]]), + ['steps', stepsMs, (ctx) => scenario.steps(ctx)], + ]; + + let stop: Stop | undefined; + let touched = false; + for (const [phase, limitMs, run] of phases) { + if (context.signal.aborted) { + stop = { outcome: 'cancelled', reason: 'cancelled', detail: `cancelled before ${phase}` }; + break; + } + touched = true; + try { + await emit(phase, 'started'); + await race((signal) => run(contextFor(signal)), limitMs, context.signal, () => new TimedOut(phase, limitMs), phase, abandoned); + await emit(phase, 'finished'); + } catch (error) { + stop = classify(phase, error); + await emit(phase, 'failed', stop.detail).catch(() => undefined); + break; + } } - } - // -- Cleanup: on success, on failure and on cancellation, and never cancelled by the run's own signal. ------------ - const cleanup = touched ? await cleanUp(root, lifecycle, contextFor, cleanupMs, emit) : { ok: true, failures: [] as string[], leftover: [] as CleanupFailure[] }; + // -- Cleanup: on success, on failure and on cancellation, and never cancelled by the run's own signal. -------- + const noCancel = new AbortController().signal; + const cleanup = touched + ? await cleanUp(root, lifecycle, contextFor, { cleanupMs, graceMs }, emitOn(noCancel, cleanupMs), () => sinkFailure, abandoned) + : { ok: true, failures: [] as string[], leftover: [] as CleanupFailure[] }; - return finish({ - outcome: stop?.outcome ?? 'passed', - ...(stop === undefined ? {} : { reason: stop.reason, ...(stop.detail === undefined ? {} : { detail: stop.detail }) }), - ...withEnvironment(environment), - cleanup: { ok: cleanup.ok, failures: cleanup.failures }, - leftover: cleanup.leftover, - }); + return finish({ + outcome: stop?.outcome ?? 'passed', + ...(stop === undefined ? {} : { reason: stop.reason, ...(stop.detail === undefined ? {} : { detail: stop.detail }) }), + ...withEnvironment(environment), + cleanup: { ok: cleanup.ok, failures: cleanup.failures }, + leftover: cleanup.leftover, + }); + } finally { + closed = true; + await lock?.release().catch(() => undefined); + } } async function dirtiness(root: string): Promise { @@ -256,44 +381,29 @@ async function dirtiness(root: string): Promise { } } -/** Runs `work` with a deadline, abortable by `outer`. A hook that ignores its signal cannot hold the run up. */ -async function bounded(phase: Phase, ms: number, outer: AbortSignal, work: (signal: AbortSignal) => Promise): Promise { - const controller = new AbortController(); - const abortWith = (reason: Error): void => { - if (!controller.signal.aborted) controller.abort(reason); - }; - const onOuterAbort = (): void => abortWith(new Cancelled()); - outer.addEventListener('abort', onOuterAbort); - const timer = setTimeout(() => abortWith(new TimedOut(phase, ms)), ms); - const interrupted = new Promise((_, reject) => { - const reject_ = (): void => reject(controller.signal.reason); - if (controller.signal.aborted) reject_(); - else controller.signal.addEventListener('abort', reject_, { once: true }); - }); - interrupted.catch(() => undefined); // no unhandled rejection when the work finishes first - const running = Promise.resolve().then(() => work(controller.signal)); - running.catch(() => undefined); // an abandoned hook may still fail later; that is no longer our concern - try { - if (outer.aborted) abortWith(new Cancelled()); - await Promise.race([running, interrupted]); - } finally { - clearTimeout(timer); - outer.removeEventListener('abort', onOuterAbort); - } -} - async function cleanUp( root: string, lifecycle: Lifecycle, contextFor: (signal: AbortSignal) => RunContext, - ms: number, + limits: { cleanupMs: number; graceMs: number }, emit: (phase: Phase, status: ScenarioEvent['status'], detail?: string) => Promise, + currentSinkFailure: () => Error | undefined, + abandoned: readonly Abandoned[], ): Promise<{ ok: boolean; failures: string[]; leftover: CleanupFailure[] }> { const failures: string[] = []; + const sinkFailedBefore = currentSinkFailure() !== undefined; await emit('cleanup', 'started').catch(() => undefined); + + // A hook that was cut off may still be running. Give it a moment to stop by itself; if it does not, nothing can + // be said about what it is still doing to the environment, so the environment is not clean. + if (abandoned.length > 0) { + await Promise.race([Promise.all(abandoned.map((a) => a.done)), sleep(limits.graceMs)]); + for (const hook of abandoned) if (!hook.settled) failures.push(`the ${hook.phase} hook was abandoned and is still running`); + } + try { // A fresh signal: the run may have been cancelled, but its cleanup must still be allowed to finish. - await bounded('cleanup', ms, new AbortController().signal, (signal) => lifecycle.cleanup(contextFor(signal))); + await race((signal) => lifecycle.cleanup(contextFor(signal)), limits.cleanupMs, new AbortController().signal, () => new TimedOut('cleanup', limits.cleanupMs), 'cleanup'); } catch (error) { failures.push(error instanceof TimedOut ? `cleanup hook ${error.message}` : `cleanup hook failed: ${message(error)}`); } @@ -307,19 +417,23 @@ async function cleanUp( } for (const item of leftover) failures.push(`${item.resource.label}: ${item.reason}${item.detail === undefined ? '' : ` (${item.detail})`}`); + await emit('cleanup', failures.length === 0 ? 'finished' : 'failed', failures.length === 0 ? undefined : failures.join('; ')).catch(() => undefined); + // A record of the cleanup that could not be delivered means the record is incomplete, which is not a clean result. + const sinkFailure = currentSinkFailure(); + if (sinkFailure !== undefined && !sinkFailedBefore) failures.push(sinkFailure.message); + if (failures.length > 0) { try { await markDirty(root, `cleanup failed: ${failures.join('; ')}`); } catch (error) { - // The ledger still lists whatever was left, so the next run will see a dirty environment either way. + // markDirty remembers the root in memory before it writes, so this process still refuses to reuse it. failures.push(`could not mark the environment dirty: ${message(error)}`); } } - const ok = failures.length === 0; - await emit('cleanup', ok ? 'finished' : 'failed', ok ? undefined : failures.join('; ')).catch(() => undefined); - return { ok, failures, leftover }; + return { ok: failures.length === 0, failures, leftover }; } +/** Polls a condition, giving up at the deadline even if the condition itself never answers. */ async function waitFor( signal: AbortSignal, condition: () => boolean | Promise, @@ -328,10 +442,21 @@ async function waitFor( const timeoutMs = options.timeoutMs ?? 10_000; const intervalMs = options.intervalMs ?? 50; const deadline = Date.now() + timeoutMs; + const expired = (): AssertionFailure => new AssertionFailure(`timed out after ${timeoutMs} ms waiting for ${options.description ?? 'a condition'}`); + for (;;) { if (signal.aborted) throw signal.reason; - if (await condition()) return; - if (Date.now() >= deadline) throw new AssertionFailure(`timed out after ${timeoutMs} ms waiting for ${options.description ?? 'a condition'}`); + const remaining = deadline - Date.now(); + if (remaining <= 0) throw expired(); + // The condition may hang or answer late, so it is raced against the deadline and the abort like everything else. + const answered = await new Promise((resolve, reject) => { + const timer = setTimeout(() => { cleanUp(); reject(expired()); }, remaining); + const onAbort = (): void => { cleanUp(); reject(signal.reason); }; + const cleanUp = (): void => { clearTimeout(timer); signal.removeEventListener('abort', onAbort); }; + signal.addEventListener('abort', onAbort, { once: true }); + Promise.resolve().then(condition).then((value) => { cleanUp(); resolve(value); }, (error: unknown) => { cleanUp(); reject(error); }); + }); + if (answered) return; // The abort listener is removed as soon as the pause ends, or every poll would leave one behind. await new Promise((resolve) => { const done = (): void => { diff --git a/packages/qa/src/runner/resources.ts b/packages/qa/src/runner/resources.ts index c8ff9f3..d418ff8 100644 --- a/packages/qa/src/runner/resources.ts +++ b/packages/qa/src/runner/resources.ts @@ -1,5 +1,6 @@ import { execFile, spawn, type ChildProcess, type SpawnOptions } from 'node:child_process'; -import { mkdir, lstat, readFile, realpath, rm, stat } from 'node:fs/promises'; +import { realpathSync } from 'node:fs'; +import { mkdir, lstat, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join, parse, resolve, sep } from 'node:path'; import { writeFileAtomic } from './journal.ts'; @@ -26,6 +27,19 @@ export interface CleanupOptions { graceMs?: number; /** Test seam: how a process is identified. */ identityOf?: (pid: number) => Promise; + /** Test seam: how a process is signalled. */ + kill?: (pid: number, signal?: NodeJS.Signals) => void; + /** Whether a process that ignores the polite request is forced. Defaults to true wherever the operating system has a polite request. */ + escalate?: boolean; +} + +/** The same directory always has the same key, however it was spelled. */ +function keyOf(root: string): string { + try { + return realpathSync.native(root); + } catch { + return resolve(root); + } } const errorCode = (error: unknown): string | undefined => (error as NodeJS.ErrnoException).code; @@ -150,7 +164,11 @@ function isAlive(pid: number): boolean { } } -/** Starts a process and records it, with its identity, before handing it back. */ +/** + * Starts a process and records it, with its identity, before handing it back. A child that has already exited has + * nothing left to own and is returned without a record (its pid may even belong to someone else by now, so an + * identity read after its exit is never trusted). If the record cannot be written the child is stopped. + */ export async function spawnOwned(root: string, label: string, command: string, args: readonly string[], options: SpawnOptions = {}): Promise { const child = spawn(command, [...args], options); await new Promise((resolveSpawn, rejectSpawn) => { @@ -158,48 +176,69 @@ export async function spawnOwned(root: string, label: string, command: string, a child.once('error', rejectSpawn); }); const pid = child.pid as number; + let exited = false; + child.once('exit', () => { + exited = true; + }); + const identity = await processIdentity(pid); + const hasExited = (): boolean => exited || child.exitCode !== null || child.signalCode !== null; + if (hasExited()) return child; if (identity === undefined) { - // A process that cannot be identified cannot be owned safely, so it is not left running. + // Alive but not identifiable: it cannot be owned safely, so it is not left running. child.kill('SIGKILL'); throw new Error(`could not identify the process started for "${label}"; it was stopped`); } - await recordOwned(root, { kind: 'process', pid, identity, label }); + try { + await recordOwned(root, { kind: 'process', pid, identity, label }); + } catch (error) { + // An untracked child would outlive every cleanup, since cleanup only knows what the ledger lists. + child.kill('SIGKILL'); + throw error; + } return child; } async function cleanProcess(resource: Extract, options: Required): Promise { - if (!isAlive(resource.pid)) return undefined; - const now = await options.identityOf(resource.pid); + const { pid } = resource; + if (!isAlive(pid)) return undefined; + const now = await options.identityOf(pid); if (now === undefined) { // It may have exited while its identity was being read; otherwise it is alive and unidentifiable: hands off. - return isAlive(resource.pid) ? { resource, reason: 'identity-unknown' } : undefined; + return isAlive(pid) ? { resource, reason: 'identity-unknown' } : undefined; } if (now !== resource.identity) return undefined; // the pid was reused: the process this run owned is already gone const waitUntilGone = async (ms: number): Promise => { const end = Date.now() + ms; while (Date.now() < end) { - if (!isAlive(resource.pid)) return true; + if (!isAlive(pid)) return true; await sleep(25); } - return !isAlive(resource.pid); + return !isAlive(pid); }; - try { - process.kill(resource.pid); // polite on POSIX (SIGTERM); Windows has no polite form - } catch { - return undefined; - } - if (await waitUntilGone(options.graceMs)) return undefined; - if (process.platform !== 'win32') { + /** A signal that could not be delivered is only harmless if the process turned out to be gone anyway. */ + const send = async (signal?: NodeJS.Signals): Promise => { try { - process.kill(resource.pid, 'SIGKILL'); + options.kill(pid, signal); + return true; } catch { - return undefined; + return waitUntilGone(300); } - if (await waitUntilGone(2000)) return undefined; - } - return { resource, reason: 'still-running' }; + }; + const stillRunning: CleanupFailure = { resource, reason: 'still-running' }; + + if (!(await send())) return stillRunning; // polite on POSIX (SIGTERM); Windows has no polite form + if (await waitUntilGone(options.graceMs)) return undefined; + if (!options.escalate) return stillRunning; + + // Forcing is the dangerous step: during the grace period the process may have exited and its pid been reused, + // so it is identified again immediately before, and only the very same process is ever forced. + const again = await options.identityOf(pid); + if (again === undefined) return isAlive(pid) ? { resource, reason: 'identity-unknown' } : undefined; + if (again !== resource.identity) return undefined; + if (!(await send('SIGKILL'))) return stillRunning; + return (await waitUntilGone(2000)) ? undefined : stillRunning; } async function cleanPath(root: string, resource: Extract): Promise { @@ -227,7 +266,12 @@ async function cleanPath(root: string, resource: Extract { - const settings: Required = { graceMs: options.graceMs ?? 5000, identityOf: options.identityOf ?? processIdentity }; + const settings: Required = { + graceMs: options.graceMs ?? 5000, + identityOf: options.identityOf ?? processIdentity, + kill: options.kill ?? ((pid, signal) => void process.kill(pid, signal)), + escalate: options.escalate ?? process.platform !== 'win32', + }; return serialized(root, async () => { const ledger = await readLedger(root); const removed: OwnedResource[] = []; @@ -250,12 +294,18 @@ export function cleanupOwnedResources(root: string, options: CleanupOptions = {} // --------------------------------------------------------------------------------------------------------------- // Dirty environments +/** Dirty roots remembered by this process, so a marker that could not be written still keeps the root closed. */ +const dirtyInMemory = new Map(); + /** Records that this environment must not be reused until it has been reset. */ export async function markDirty(root: string, reason: string): Promise { + dirtyInMemory.set(keyOf(root), reason); // first: even if the write below fails, this process will not reuse the root await writeFileAtomic(join(root, DIRTY_FILE), `${JSON.stringify({ schemaVersion: 1, reason }, null, 2)}\n`); } export async function readDirty(root: string): Promise { + const remembered = dirtyInMemory.get(keyOf(root)); + if (remembered !== undefined) return remembered; const text = await readFile(join(root, DIRTY_FILE), 'utf8').catch((error) => (errorCode(error) === 'ENOENT' ? undefined : Promise.reject(error))); if (text === undefined) return undefined; try { @@ -268,6 +318,61 @@ export async function readDirty(root: string): Promise { /** Reaps everything the ledger owns and, only when that succeeds completely, clears the dirty marker. */ export async function resetDirtyEnvironment(root: string, options: CleanupOptions = {}): Promise<{ failures: CleanupFailure[] }> { const { failures } = await cleanupOwnedResources(root, options); - if (failures.length === 0) await rm(join(root, DIRTY_FILE), { force: true }); + if (failures.length === 0) { + dirtyInMemory.delete(keyOf(root)); + await rm(join(root, DIRTY_FILE), { force: true }); + } return { failures }; } + +export type RootLock = { ok: true; release(): Promise } | { ok: false; heldBy: string }; + +const LOCK_FILE = '.release-qa-lock.json'; +/** Roots this process holds, so a second run in the same process is refused without touching the disk. */ +const heldHere = new Set(); +let ownIdentity: Promise | undefined; +/** This process's own identity, read once: on Windows reading it costs a PowerShell start. */ +const identityOfThisProcess = (): Promise => (ownIdentity ??= processIdentity(process.pid)); + +/** + * At most one run at a time may use a test root, or two runs would reset the environment under each other and + * clean up each other's resources. The lock names its owner by pid and identity, so one left behind by a process + * that has since exited (or whose pid was reused) is recognised as stale, while anything unreadable counts as held. + */ +export async function acquireTestRoot(root: string): Promise { + const key = keyOf(root); + if (heldHere.has(key)) return { ok: false, heldBy: `this process (pid ${process.pid})` }; + const path = join(root, LOCK_FILE); + const mine = JSON.stringify({ pid: process.pid, identity: (await identityOfThisProcess()) ?? 'unknown' }); + + for (let attempt = 0; attempt < 2; attempt++) { + try { + await writeFile(path, mine, { flag: 'wx' }); + heldHere.add(key); + return { + ok: true, + release: async () => { + heldHere.delete(key); + // Only remove a lock that is still ours; someone may have taken over a lock they judged stale. + const current = await readFile(path, 'utf8').catch(() => undefined); + if (current === mine) await rm(path, { force: true }); + }, + }; + } catch (error) { + if (errorCode(error) !== 'EEXIST') throw error; + } + + let holder: { pid: number; identity: string }; + try { + holder = JSON.parse(await readFile(path, 'utf8')) as { pid: number; identity: string }; + if (!Number.isSafeInteger(holder.pid) || typeof holder.identity !== 'string') throw new Error('malformed'); + } catch { + return { ok: false, heldBy: `a lock file that cannot be read (${path})` }; + } + const identity = isAlive(holder.pid) ? await processIdentity(holder.pid) : undefined; + const stale = !isAlive(holder.pid) || (identity !== undefined && identity !== holder.identity); + if (!stale) return { ok: false, heldBy: `pid ${holder.pid}` }; + await rm(path, { force: true }); + } + return { ok: false, heldBy: 'a lock that kept reappearing' }; +} diff --git a/packages/qa/test/fixtures/execution.ts b/packages/qa/test/fixtures/execution.ts new file mode 100644 index 0000000..44ab35e --- /dev/null +++ b/packages/qa/test/fixtures/execution.ts @@ -0,0 +1,40 @@ +// Shared setup for tests that run scenarios through executeScenario. +import type { ExecutionContext, Lifecycle, Scenario, ScenarioEvent } from '../../src/runner/execute.ts'; +import { candidate, requirement } from './records.ts'; +import { hostOs, hostProfile, makeTestRoot } from './processes.ts'; + +export const never = (): Promise => new Promise(() => {}); +export const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +export function lifecycleOf(calls: string[], overrides: Partial = {}): Lifecycle { + const record = (name: string) => async () => { calls.push(name); }; + return { install: record('install'), reset: record('reset'), launch: record('launch'), cleanup: record('cleanup'), ...overrides }; +} + +export async function arrange(overrides: Partial = {}) { + const testRoot = await makeTestRoot(); + const calls: string[] = []; + const events: ScenarioEvent[] = []; + const controller = new AbortController(); + const context: ExecutionContext = { + candidate: candidate(), + profile: hostProfile(), + testRoot, + signal: controller.signal, + emit: (event) => { events.push(event); }, + lifecycle: lifecycleOf(calls), + probes: { display: async () => true, audio: async () => true }, + timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 2000 }, + ...overrides, + }; + return { testRoot, calls, events, controller, context }; +} + +export const scenarioOf = (overrides: Partial = {}): Scenario => ({ + id: 'persistence', + requirement: requirement({ key: `${hostOs()}/persistence` }), + steps: async () => {}, + ...overrides, +}); + +export const started = (events: readonly ScenarioEvent[]) => events.filter((e) => e.status === 'started').map((e) => e.phase); diff --git a/packages/qa/test/fixtures/processes.ts b/packages/qa/test/fixtures/processes.ts index d3b108b..ee09e0e 100644 --- a/packages/qa/test/fixtures/processes.ts +++ b/packages/qa/test/fixtures/processes.ts @@ -2,7 +2,7 @@ // of them is ever handed to the code under test unless a test does so on purpose. import { spawn, type ChildProcess } from 'node:child_process'; import { mkdtemp, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; +import { arch, tmpdir } from 'node:os'; import { join } from 'node:path'; import type { EnvironmentProfile } from '../../src/model/project.ts'; import { designateTestRoot } from '../../src/runner/resources.ts'; @@ -13,6 +13,15 @@ const roots: string[] = []; /** A process unrelated to the runner, standing in for someone's real application. */ export function startUnrelatedProcess(script = 'setInterval(() => {}, 1000)'): ChildProcess { const child = spawn(process.execPath, ['-e', script], { stdio: 'ignore' }); + // A failed spawn must fail one test, not crash the whole worker with an unhandled 'error' event. + child.on('error', () => undefined); + started.push(child); + return child; +} + +/** Registers a process started some other way (for example by spawnOwned) so it is stopped after the test. */ +export function trackProcess(child: ChildProcess): ChildProcess { + child.on('error', () => undefined); started.push(child); return child; } @@ -51,18 +60,29 @@ export async function makeTempDir(prefix: string): Promise { } export async function cleanUpProcessesAndRoots(): Promise { - for (const child of started) { + const children = started.splice(0); + for (const child of children) { try { child.kill('SIGKILL'); } catch { /* already gone */ } } - started.length = 0; - for (const root of roots.splice(0)) await rm(root, { recursive: true, force: true }); + // On Windows a child that is still exiting can hold a directory open, so wait for each to be gone first. + await Promise.all( + children.map((child) => (child.exitCode !== null || child.signalCode !== null ? undefined : new Promise((resolve) => { child.once('exit', () => resolve()); setTimeout(resolve, 3000); }))), + ); + // Every removal is attempted, whatever any other one does. + await Promise.allSettled(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); } export const hostOs = (): 'windows' | 'linux' => (process.platform === 'win32' ? 'windows' : 'linux'); +/** The architecture name the runner reports for this machine (x64 is x86_64, arm64 is aarch64). */ +export const hostArch = (): string => (arch() === 'x64' ? 'x86_64' : arch() === 'arm64' ? 'aarch64' : arch()); + +/** An architecture that is not this machine's, for tests of a mismatch. */ +export const otherArch = (): string => (hostArch() === 'x86_64' ? 'aarch64' : 'x86_64'); + /** The profile of the machine the tests are running on. */ -export const hostProfile = (): EnvironmentProfile => ({ id: hostOs(), os: hostOs(), arch: 'x86_64' }); +export const hostProfile = (): EnvironmentProfile => ({ id: hostOs(), os: hostOs(), arch: hostArch() as EnvironmentProfile['arch'] }); diff --git a/packages/qa/test/runner/environment.test.ts b/packages/qa/test/runner/environment.test.ts index 752107a..f6b15f5 100644 --- a/packages/qa/test/runner/environment.test.ts +++ b/packages/qa/test/runner/environment.test.ts @@ -1,7 +1,7 @@ -import { arch, release } from 'node:os'; +import { release } from 'node:os'; import { describe, expect, test } from 'vitest'; import { defaultProbes, inspectEnvironment } from '../../src/runner/environment.ts'; -import { hostOs, hostProfile } from '../fixtures/processes.ts'; +import { hostArch, hostOs, hostProfile, otherArch } from '../fixtures/processes.ts'; const probes = (display: boolean, audio: boolean) => ({ display: async () => display, audio: async () => audio }); @@ -10,7 +10,7 @@ describe('inspectEnvironment', () => { const { environment } = await inspectEnvironment(hostProfile(), probes(true, true), '1.2.3'); expect(environment.os).toBe(hostOs()); expect(environment.osVersion).toBe(release()); - expect(environment.arch).toBe(arch() === 'x64' ? 'x86_64' : arch()); + expect(environment.arch).toBe(hostArch()); expect(environment.toolVersion).toBe('1.2.3'); }); @@ -26,6 +26,12 @@ describe('inspectEnvironment', () => { expect((await inspectEnvironment(hostProfile(), broken)).environment.capabilities).toEqual(['audio']); }); + test('a probe that throws before it even returns a promise is also just an absent capability', async () => { + const boom = (): Promise => { throw new Error('thrown synchronously'); }; + const inspected = await inspectEnvironment(hostProfile(), { display: boom, audio: boom }); + expect(inspected.environment.capabilities).toEqual([]); + }); + test('reports no mismatch for the profile of this machine', async () => { const inspected = await inspectEnvironment(hostProfile(), probes(true, true)); expect(inspected.environment.os).toBe(hostOs()); @@ -40,11 +46,11 @@ describe('inspectEnvironment', () => { }); test('reports a mismatch when the profile asks for another architecture', async () => { - // The profile type only allows x86_64 today; a future profile for another architecture must still be refused here. - const other = { ...hostProfile(), arch: 'aarch64' } as unknown as ReturnType; + // The profile type only allows x86_64 today; a profile for any architecture that is not this machine's must be refused. + const other = { ...hostProfile(), arch: otherArch() } as unknown as ReturnType; const { profileMismatch } = await inspectEnvironment(other, probes(true, true)); - expect(profileMismatch).toContain('aarch64'); - expect(profileMismatch).toContain('x86_64'); + expect(profileMismatch).toContain(otherArch()); + expect(profileMismatch).toContain(hostArch()); }); test('the default probes answer with a boolean and never throw, whatever this machine has', async () => { diff --git a/packages/qa/test/runner/execute-hardening.test.ts b/packages/qa/test/runner/execute-hardening.test.ts new file mode 100644 index 0000000..61728fe --- /dev/null +++ b/packages/qa/test/runner/execute-hardening.test.ts @@ -0,0 +1,279 @@ +// Behaviours found by review of the first version of the executor: ways a run could hang, leak, or reuse an +// environment that was not properly cleaned. +import { mkdir, rm, stat, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { afterEach, describe, expect, test } from 'vitest'; +import { executeScenario } from '../../src/runner/execute.ts'; +import { processIdentity, readDirty, readLedger } from '../../src/runner/resources.ts'; +import { arrange, lifecycleOf, never, scenarioOf, sleep } from '../fixtures/execution.ts'; +import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, startUnrelatedProcess } from '../fixtures/processes.ts'; + +afterEach(cleanUpProcessesAndRoots); + +const exists = (path: string) => stat(path).then(() => true, () => false); +const sleeper = ['-e', 'setInterval(() => {}, 1000)']; + +describe('fail closed: an environment that may be dirty is never reused', () => { + test('when the dirty marker cannot be written, this process still refuses the next run', async () => { + const { context, testRoot } = await arrange({ + lifecycle: lifecycleOf([], { + // A directory where the marker file belongs makes writing the marker fail. It appears mid-run: a marker + // that is unreadable before the run starts is refused earlier, as a dirty environment. + install: async (ctx) => { await mkdir(join(ctx.testRoot, '.release-qa-dirty.json')); }, + cleanup: async () => { throw new Error('uninstaller crashed'); }, + }), + }); + + const first = await executeScenario(context, scenarioOf()); + + expect(first.cleanup.ok).toBe(false); + expect(first.cleanup.failures.join(' ')).toContain('could not mark the environment dirty'); + await rm(join(testRoot, '.release-qa-dirty.json'), { recursive: true }); + expect(await readDirty(testRoot)).toBeDefined(); // remembered in memory even though it never reached the disk + + const second = await executeScenario(context, scenarioOf()); + expect(second).toMatchObject({ outcome: 'blocked', reason: 'dirty-environment' }); + }); +}); + +describe('a record that cannot be read', () => { + test.each([ + ['the ledger of owned resources', '.release-qa-owned.json'], + ['the dirty marker', '.release-qa-dirty.json'], + ])('%s blocks the run instead of being treated as empty', async (_label, name) => { + const calls: string[] = []; + const { context, testRoot } = await arrange({ lifecycle: lifecycleOf(calls) }); + await mkdir(join(testRoot, name)); // a directory where the file belongs cannot be read as one + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'blocked', reason: 'dirty-environment' }); + expect(calls).toEqual([]); + }); +}); + +describe('waitFor', () => { + test('gives up on a condition that never answers, as an assertion failure naming what it waited for', async () => { + const { context } = await arrange(); + const began = Date.now(); + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => never().then(() => true), { timeoutMs: 80, description: 'the app to answer' }) })); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed' }); + expect(result.detail).toContain('the app to answer'); + expect(Date.now() - began).toBeLessThan(1500); + }); + + test('treats a condition that only answers after the deadline as a timeout, not a success', async () => { + const { context } = await arrange(); + const late = () => sleep(250).then(() => true); + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(late, { timeoutMs: 60, description: 'a late answer' }) })); + expect(result).toMatchObject({ outcome: 'failed', reason: 'assertion-failed' }); + }); + + test('a condition that never answers does not hold up cancellation', async () => { + const { context, controller } = await arrange(); + const running = executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => never().then(() => true), { timeoutMs: 60_000 }) })); + await sleep(80); + const began = Date.now(); + controller.abort(); + const result = await running; + expect(result.outcome).toBe('cancelled'); + expect(Date.now() - began).toBeLessThan(1500); + // The wait stopped by itself, so the step is not an abandoned hook and the environment is not dirty. + expect(result.cleanup).toEqual({ ok: true, failures: [] }); + }); + + test('does not turn an error thrown by the condition into a timeout', async () => { + const { context } = await arrange(); + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => { throw new TypeError('the driver is gone'); }, { timeoutMs: 500 }) })); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + expect(result.detail).toContain('the driver is gone'); + }); +}); + +describe('every wait is bounded, including prerequisites and the event sink', () => { + test('a probe that never answers is cut off by the phase deadline', async () => { + const { context, calls } = await arrange({ probes: { display: async () => true, audio: never as unknown as () => Promise }, timeouts: { phaseMs: 80, stepsMs: 2000, cleanupMs: 2000 } }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'timeout' }); + expect(result.detail).toContain('prerequisites'); + expect(calls).toEqual([]); + }); + + test('a probe that never answers does not block cancellation', async () => { + const { context, controller } = await arrange({ probes: { display: async () => true, audio: never as unknown as () => Promise } }); + const running = executeScenario(context, scenarioOf()); + await sleep(50); + controller.abort(); + expect((await running).outcome).toBe('cancelled'); + }); + + test('an event sink that never answers is cut off, the run stops, and cleanup still happens', async () => { + const calls: string[] = []; + const { context } = await arrange({ + lifecycle: lifecycleOf(calls), + emit: (event) => (event.phase === 'install' && event.status === 'started' ? never() : undefined), + timeouts: { phaseMs: 80, stepsMs: 2000, cleanupMs: 2000 }, + }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + expect(result.detail).toMatch(/event sink/); + expect(calls).toContain('cleanup'); + }); + + test('once the sink has failed it is not used again, not even to report the failure or the cleanup', async () => { + const seen: string[] = []; + const { context } = await arrange({ + emit: (event) => { + seen.push(`${event.phase}:${event.status}`); + if (event.phase === 'install' && event.status === 'started') throw new Error('sink is gone'); + }, + }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'infrastructure-error' }); + expect(seen.at(-1)).toBe('install:started'); + expect(seen.some((entry) => entry.startsWith('cleanup'))).toBe(false); + }); + + test('a failing event sink during cleanup is reported in the result and leaves the environment dirty', async () => { + const { context, testRoot } = await arrange({ emit: (event) => { if (event.phase === 'cleanup') throw new Error('sink is full'); } }); + const result = await executeScenario(context, scenarioOf()); + expect(result.outcome).toBe('passed'); + expect(result.cleanup.ok).toBe(false); + expect(result.cleanup.failures.join(' ')).toMatch(/event sink/); + expect(await readDirty(testRoot)).toBeDefined(); + }); +}); + +describe('hooks that outlive their phase', () => { + test('a hook that ignores cancellation and never finishes leaves the environment dirty', async () => { + const calls: string[] = []; + const { context, controller, testRoot } = await arrange({ + lifecycle: lifecycleOf(calls, { install: () => { calls.push('install'); return never(); } }), + timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 2000, abandonedGraceMs: 50 }, + }); + const running = executeScenario(context, scenarioOf()); + await eventually(() => calls.includes('install')); + controller.abort(); + + const result = await running; + + expect(result.outcome).toBe('cancelled'); + expect(result.cleanup.ok).toBe(false); + expect(result.cleanup.failures.join(' ')).toMatch(/install.*(abandoned|still running)/i); + expect(await readDirty(testRoot)).toBeDefined(); + // The first run's signal is spent, so the next run gets its own. + expect((await executeScenario({ ...context, signal: new AbortController().signal }, scenarioOf())).reason).toBe('dirty-environment'); + }); + + test('a hook that stops soon after being cut off does not dirty the environment', async () => { + const { context } = await arrange({ + lifecycle: lifecycleOf([], { install: () => sleep(80) }), + timeouts: { phaseMs: 20, stepsMs: 2000, cleanupMs: 2000, abandonedGraceMs: 1000 }, + }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'timeout' }); + expect(result.cleanup.ok).toBe(true); + }); + + test('a late spawn from a hook that was cut off is refused and starts nothing', async () => { + let refused: unknown; + // The child would write this file if it were ever allowed to run; it lives in a scratch directory, never the repo. + const markerFile = join(await makeTempDir('qa-late-'), 'late-spawn-marker'); + const { context, testRoot } = await arrange({ + lifecycle: lifecycleOf([], { + install: async (ctx) => { + await sleep(150); + try { + await ctx.spawn('late', process.execPath, ['-e', `require('fs').writeFileSync(${JSON.stringify(markerFile)}, 'x'); setInterval(() => {}, 1000)`], { stdio: 'ignore' }); + } catch (error) { + refused = error; + } + }, + }), + timeouts: { phaseMs: 30, stepsMs: 2000, cleanupMs: 2000, abandonedGraceMs: 400 }, + }); + + await executeScenario(context, scenarioOf()); + await sleep(500); + + expect(refused).toBeInstanceOf(Error); + expect(String((refused as Error).message)).toMatch(/stopped|no longer/i); + expect(await readLedger(testRoot)).toEqual([]); + expect(await exists(markerFile)).toBe(false); + }); + + test('a late claim of ownership from a hook that was cut off is refused', async () => { + let refused: unknown; + let root = ''; + const { context, testRoot } = await arrange({ + lifecycle: lifecycleOf([], { + install: async (ctx) => { + await sleep(150); + try { + await ctx.own({ kind: 'path', path: join(root, 'late'), label: 'late' }); + } catch (error) { + refused = error; + } + }, + }), + timeouts: { phaseMs: 30, stepsMs: 2000, cleanupMs: 2000, abandonedGraceMs: 400 }, + }); + root = testRoot; + await executeScenario(context, scenarioOf()); + await sleep(500); + expect(refused).toBeInstanceOf(Error); + expect(await readLedger(testRoot)).toEqual([]); + }); +}); + +describe('one run at a time per test root', () => { + test('a second run on a root that is in use is refused, and the first is unaffected', async () => { + const calls: string[] = []; + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const { context } = await arrange({ lifecycle: lifecycleOf(calls) }); + const first = executeScenario(context, scenarioOf({ steps: async () => { calls.push('first-steps'); await gate; } })); + await eventually(() => calls.includes('first-steps')); + + const second = await executeScenario(context, scenarioOf()); + + expect(second).toMatchObject({ outcome: 'blocked', reason: 'environment-busy' }); + expect(second.detail).toContain(String(process.pid)); + expect(calls.filter((c) => c === 'install')).toHaveLength(1); + + release(); + expect((await first).outcome).toBe('passed'); + expect((await executeScenario(context, scenarioOf())).outcome).toBe('passed'); + }); + + test.each([ + ['fails', () => scenarioOf({ steps: async () => { throw new Error('boom'); } })], + ['is blocked', () => scenarioOf()], + ])('the root is free again after a run that %s', async (label) => { + const { context } = await arrange(label === 'is blocked' ? { probes: { display: async () => false, audio: async () => false } } : {}); + const requirementNeedingDisplay = scenarioOf({ requirement: { ...scenarioOf().requirement, capabilities: label === 'is blocked' ? ['display'] : [] } }); + await executeScenario(context, label === 'is blocked' ? requirementNeedingDisplay : scenarioOf({ steps: async () => { throw new Error('boom'); } })); + const again = await executeScenario({ ...context, probes: { display: async () => true, audio: async () => true } }, scenarioOf()); + expect(again.outcome).toBe('passed'); + }); + + test('the root is free again after a cancelled run', async () => { + const { context, controller } = await arrange(); + const running = executeScenario(context, scenarioOf({ steps: (ctx) => ctx.waitFor(() => false, { timeoutMs: 5000, intervalMs: 10 }) })); + await sleep(80); + controller.abort(); + expect((await running).outcome).toBe('cancelled'); + expect((await executeScenario({ ...context, signal: new AbortController().signal }, scenarioOf())).outcome).toBe('passed'); + }); + + test('a lock left by a process that is gone does not block a run', async () => { + const { context, testRoot } = await arrange(); + const dead = startUnrelatedProcess(); + const pid = dead.pid as number; + const identity = await processIdentity(pid); + dead.kill('SIGKILL'); + await eventually(() => !isAlive(pid)); + await writeFile(join(testRoot, '.release-qa-lock.json'), JSON.stringify({ pid, identity })); + expect((await executeScenario(context, scenarioOf())).outcome).toBe('passed'); + }); +}); + +void sleeper; diff --git a/packages/qa/test/runner/execute-late-lock.test.ts b/packages/qa/test/runner/execute-late-lock.test.ts new file mode 100644 index 0000000..39a8a2c --- /dev/null +++ b/packages/qa/test/runner/execute-late-lock.test.ts @@ -0,0 +1,42 @@ +// A lock acquired by prerequisites that were cut off must not outlive the run, or nothing could use the root again. +import { stat } from 'node:fs/promises'; +import { join } from 'node:path'; +import { afterEach, expect, test, vi } from 'vitest'; +import { executeScenario } from '../../src/runner/execute.ts'; +import { acquireTestRoot } from '../../src/runner/resources.ts'; +import { arrange, scenarioOf, sleep } from '../fixtures/execution.ts'; +import { cleanUpProcessesAndRoots } from '../fixtures/processes.ts'; + +const slow = vi.hoisted(() => ({ delayMs: 0 })); + +vi.mock('../../src/runner/resources.ts', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + acquireTestRoot: async (root: string) => { + await new Promise((resolve) => setTimeout(resolve, slow.delayMs)); + return real.acquireTestRoot(root); + }, + }; +}); + +afterEach(async () => { + slow.delayMs = 0; + await cleanUpProcessesAndRoots(); +}); + +test('a lock taken after the prerequisites were cut off is given straight back', async () => { + const { context, testRoot } = await arrange({ timeouts: { phaseMs: 40, stepsMs: 2000, cleanupMs: 2000 } }); + slow.delayMs = 250; // the disk is slow: the deadline passes before the lock is taken + + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'interrupted', reason: 'timeout' }); + + await sleep(600); + const lockFile = join(testRoot, '.release-qa-lock.json'); + expect(await stat(lockFile).then(() => true, () => false)).toBe(false); + slow.delayMs = 0; + const lock = await acquireTestRoot(testRoot); + expect(lock.ok).toBe(true); + if (lock.ok) await lock.release(); +}); diff --git a/packages/qa/test/runner/execute.test.ts b/packages/qa/test/runner/execute.test.ts index cda7f90..3df921a 100644 --- a/packages/qa/test/runner/execute.test.ts +++ b/packages/qa/test/runner/execute.test.ts @@ -6,7 +6,7 @@ import { afterEach, describe, expect, test } from 'vitest'; import { AssertionFailure, executeScenario, type ExecutionContext, type Lifecycle, type Scenario, type ScenarioEvent } from '../../src/runner/execute.ts'; import { readDirty, readLedger, resetDirtyEnvironment, spawnOwned } from '../../src/runner/resources.ts'; import { candidate, requirement } from '../fixtures/records.ts'; -import { cleanUpProcessesAndRoots, eventually, hostOs, hostProfile, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess } from '../fixtures/processes.ts'; +import { cleanUpProcessesAndRoots, eventually, hostOs, hostProfile, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; afterEach(cleanUpProcessesAndRoots); @@ -115,6 +115,7 @@ describe('a dirty environment', () => { test('refuses the next run while a process from an earlier run is still owned, and leaves that process alone', async () => { const { context, calls, testRoot } = await arrange(); const leftover = await spawnOwned(testRoot, 'earlier helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(leftover); const refused = await executeScenario(context, scenarioOf()); @@ -126,6 +127,7 @@ describe('a dirty environment', () => { test('accepts runs again once the environment has been reset', async () => { const { context, testRoot } = await arrange(); const leftover = await spawnOwned(testRoot, 'earlier helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(leftover); expect((await executeScenario(context, scenarioOf())).outcome).toBe('blocked'); await resetDirtyEnvironment(testRoot, { graceMs: 300 }); diff --git a/packages/qa/test/runner/resources-hardening.test.ts b/packages/qa/test/runner/resources-hardening.test.ts new file mode 100644 index 0000000..b516c8e --- /dev/null +++ b/packages/qa/test/runner/resources-hardening.test.ts @@ -0,0 +1,212 @@ +// Behaviours found by review of the first version of the resource code. Each one is a way that cleanup or +// ownership could touch, or fail to stop, the wrong process. +import { mkdir, readFile, stat, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { afterEach, describe, expect, test } from 'vitest'; +import { acquireTestRoot, cleanupOwnedResources, processIdentity, readLedger, recordOwned, spawnOwned } from '../../src/runner/resources.ts'; +import { cleanUpProcessesAndRoots, eventually, isAlive, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; + +afterEach(cleanUpProcessesAndRoots); + +const sleeper = ['-e', 'setInterval(() => {}, 1000)']; +const exists = (path: string) => stat(path).then(() => true, () => false); +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +const owner = async (root: string) => JSON.parse(await readFile(join(root, '.release-qa-lock.json'), 'utf8')) as { pid: number }; + +describe('forcing a process that ignores the polite request', () => { + // The polite request has no effect here (the seam records it and does nothing), as if the process ignored it. + const fakeKill = () => { + const signals: string[] = []; + return { signals, kill: (_pid: number, signal?: NodeJS.Signals) => { signals.push(signal ?? 'SIGTERM'); } }; + }; + + test('never force-kills a pid that now belongs to another process', async () => { + const root = await makeTestRoot(); + const stranger = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: stranger.pid as number, identity: 'original', label: 'gone' }); + const { signals, kill } = fakeKill(); + let lookups = 0; + + const result = await cleanupOwnedResources(root, { graceMs: 100, escalate: true, kill, identityOf: async () => (++lookups === 1 ? 'original' : 'someone-else') }); + + expect(signals).toEqual(['SIGTERM']); + expect(isAlive(stranger.pid as number)).toBe(true); + expect(result.failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + + test('force-kills when the very same process is still there after the grace period', async () => { + const root = await makeTestRoot(); + const child = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: child.pid as number, identity: 'original', label: 'stubborn' }); + const { signals, kill } = fakeKill(); + + const result = await cleanupOwnedResources(root, { graceMs: 100, escalate: true, kill, identityOf: async () => 'original' }); + + expect(signals).toEqual(['SIGTERM', 'SIGKILL']); + // The fake signals do nothing, so the process is honestly reported as still running. + expect(result.failures.map((f) => f.reason)).toEqual(['still-running']); + expect(await readLedger(root)).toHaveLength(1); + }); + + test('leaves a process alone, and reports it, when it cannot be re-identified before escalating', async () => { + const root = await makeTestRoot(); + const child = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: child.pid as number, identity: 'original', label: 'stubborn' }); + const { signals, kill } = fakeKill(); + let lookups = 0; + + const result = await cleanupOwnedResources(root, { graceMs: 100, escalate: true, kill, identityOf: async () => (++lookups === 1 ? 'original' : undefined) }); + + expect(signals).toEqual(['SIGTERM']); + expect(result.failures.map((f) => f.reason)).toEqual(['identity-unknown']); + expect(isAlive(child.pid as number)).toBe(true); + }); + + test('does not escalate at all where the operating system has no polite request', async () => { + const root = await makeTestRoot(); + const child = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: child.pid as number, identity: 'original', label: 'x' }); + const { signals, kill } = fakeKill(); + const result = await cleanupOwnedResources(root, { graceMs: 100, escalate: false, kill, identityOf: async () => 'original' }); + expect(signals).toEqual(['SIGTERM']); + expect(result.failures.map((f) => f.reason)).toEqual(['still-running']); + }); +}); + +describe('a signal that fails', () => { + test('is a failure while the process is still alive, not a successful cleanup', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); + const denied = () => { throw Object.assign(new Error('operation not permitted'), { code: 'EPERM' }); }; + + const result = await cleanupOwnedResources(root, { graceMs: 100, kill: denied }); + + expect(result.failures.map((f) => f.reason)).toEqual(['still-running']); + expect(isAlive(child.pid as number)).toBe(true); + expect(await readLedger(root)).toHaveLength(1); + }); + + test('is fine when the process turned out to be gone already', async () => { + const root = await makeTestRoot(); + const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); + const gone = () => { + child.kill('SIGKILL'); + throw Object.assign(new Error('no such process'), { code: 'ESRCH' }); + }; + await eventually(() => true); + const result = await cleanupOwnedResources(root, { graceMs: 1000, kill: gone }); + expect(result.failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); +}); + +describe('spawnOwned', () => { + test('does not leave the child running when the ledger cannot be written', async () => { + const root = await makeTestRoot(); + const pidFile = join(root, 'pid.txt'); + // A directory where the ledger file belongs makes every ledger read or write fail. + await mkdir(join(root, '.release-qa-owned.json')); + const script = `require('fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)); setInterval(() => {}, 1000)`; + + await expect(spawnOwned(root, 'helper', process.execPath, ['-e', script], { stdio: 'ignore' })).rejects.toThrow(); + + // The child either never got as far as writing its pid (already stopped) or must be stopped now. + await wait(1500); + if (await exists(pidFile)) { + const pid = Number(await readFile(pidFile, 'utf8')); + await eventually(() => !isAlive(pid), 5000); + } + }); + + test('a command that exits at once is not an error, and is not recorded as owned', async () => { + const root = await makeTestRoot(); + const spawned = await Promise.all(Array.from({ length: 8 }, () => spawnOwned(root, 'quick', process.execPath, ['-e', '0'], { stdio: 'ignore' }).then((c) => trackProcess(c), (error: unknown) => error))); + const errors = spawned.filter((entry) => entry instanceof Error); + expect(errors).toEqual([]); + // Whatever was recorded, cleanup must be able to finish without failures. + expect((await cleanupOwnedResources(root, { graceMs: 200 })).failures).toEqual([]); + }); +}); + +describe('the test root lock', () => { + test('refuses a second owner while the first holds the root, and hands it over after release', async () => { + const root = await makeTestRoot(); + const first = await acquireTestRoot(root); + expect(first.ok).toBe(true); + + const second = await acquireTestRoot(root); + expect(second.ok).toBe(false); + expect(!second.ok && second.heldBy).toContain(String(process.pid)); + + if (first.ok) await first.release(); + const third = await acquireTestRoot(root); + expect(third.ok).toBe(true); + if (third.ok) await third.release(); + }); + + test('takes over a lock left by a process that no longer exists', async () => { + const root = await makeTestRoot(); + const dead = startUnrelatedProcess(); + const pid = dead.pid as number; + const identity = await processIdentity(pid); + dead.kill('SIGKILL'); + await eventually(() => !isAlive(pid)); + await writeFile(join(root, '.release-qa-lock.json'), JSON.stringify({ pid, identity })); + + const lock = await acquireTestRoot(root); + + expect(lock.ok).toBe(true); + expect((await owner(root)).pid).toBe(process.pid); // the stale lock was replaced by ours + if (lock.ok) await lock.release(); + }); + + test('takes over a lock whose pid now belongs to a different process', async () => { + const root = await makeTestRoot(); + const stranger = startUnrelatedProcess(); + await writeFile(join(root, '.release-qa-lock.json'), JSON.stringify({ pid: stranger.pid, identity: 'the-original-owner' })); + const lock = await acquireTestRoot(root); + expect(lock.ok).toBe(true); + expect((await owner(root)).pid).toBe(process.pid); + if (lock.ok) await lock.release(); + }); + + test('respects a lock held by a live process with a matching identity', async () => { + const root = await makeTestRoot(); + const other = startUnrelatedProcess(); + const identity = await processIdentity(other.pid as number); + await writeFile(join(root, '.release-qa-lock.json'), JSON.stringify({ pid: other.pid, identity })); + + const lock = await acquireTestRoot(root); + + expect(lock.ok).toBe(false); + expect(!lock.ok && lock.heldBy).toContain(String(other.pid)); + }); + + test('treats a lock file it cannot understand as held, never as free', async () => { + const root = await makeTestRoot(); + await writeFile(join(root, '.release-qa-lock.json'), 'not json'); + const lock = await acquireTestRoot(root); + expect(lock.ok).toBe(false); + }); + + test('releasing does not remove a lock that someone else has since taken over', async () => { + const root = await makeTestRoot(); + const lock = await acquireTestRoot(root); + expect(lock.ok).toBe(true); + expect((await owner(root)).pid).toBe(process.pid); + await writeFile(join(root, '.release-qa-lock.json'), JSON.stringify({ pid: 1, identity: 'someone-else' })); + if (lock.ok) await lock.release(); + expect(JSON.parse(await readFile(join(root, '.release-qa-lock.json'), 'utf8'))).toEqual({ pid: 1, identity: 'someone-else' }); + }); + + test('a released lock leaves no file behind', async () => { + const root = await makeTestRoot(); + const lock = await acquireTestRoot(root); + expect(await exists(join(root, '.release-qa-lock.json'))).toBe(true); + if (lock.ok) await lock.release(); + expect(await exists(join(root, '.release-qa-lock.json'))).toBe(false); + }); +}); diff --git a/packages/qa/test/runner/resources.test.ts b/packages/qa/test/runner/resources.test.ts index 43377e8..29b9bc4 100644 --- a/packages/qa/test/runner/resources.test.ts +++ b/packages/qa/test/runner/resources.test.ts @@ -15,7 +15,7 @@ import { spawnOwned, TEST_ROOT_MARKER, } from '../../src/runner/resources.ts'; -import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess } from '../fixtures/processes.ts'; +import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; afterEach(cleanUpProcessesAndRoots); @@ -122,6 +122,7 @@ describe('spawning owned processes', () => { test('records the process and its identity in the ledger before handing it back', async () => { const root = await makeTestRoot(); const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); const [entry] = await readLedger(root); expect(entry).toMatchObject({ kind: 'process', pid: child.pid, label: 'helper' }); expect(entry?.kind === 'process' && entry.identity.length).toBeGreaterThan(0); @@ -151,6 +152,7 @@ describe('cleaning up', () => { test('stops the processes it owns and leaves an unrelated process running', async () => { const root = await makeTestRoot(); const owned = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(owned); const unrelated = startUnrelatedProcess(); const result = await cleanupOwnedResources(root, { graceMs: 500 }); @@ -179,6 +181,7 @@ describe('cleaning up', () => { test('refuses to kill a live process it cannot identify, and keeps it on the ledger', async () => { const root = await makeTestRoot(); const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); const result = await cleanupOwnedResources(root, { graceMs: 200, identityOf: async () => undefined }); expect(isAlive(child.pid as number)).toBe(true); expect(result.failures.map((f) => f.reason)).toEqual(['identity-unknown']); @@ -188,6 +191,7 @@ describe('cleaning up', () => { test('a process that already exited is dropped from the ledger without a failure', async () => { const root = await makeTestRoot(); const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); child.kill('SIGKILL'); await eventually(() => !isAlive(child.pid as number)); expect((await cleanupOwnedResources(root)).failures).toEqual([]); @@ -197,6 +201,7 @@ describe('cleaning up', () => { test.skipIf(process.platform === 'win32')('forces a process that ignores the polite request', async () => { const root = await makeTestRoot(); const child = await spawnOwned(root, 'stubborn', process.execPath, ['-e', "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)"], { stdio: 'ignore' }); + trackProcess(child); await new Promise((resolve) => setTimeout(resolve, 300)); const result = await cleanupOwnedResources(root, { graceMs: 200 }); expect(result.failures).toEqual([]); @@ -321,6 +326,7 @@ describe('a dirty environment', () => { test('is cleaned by a reset: owned processes stop, the marker clears, the ledger empties', async () => { const root = await makeTestRoot(); const child = await spawnOwned(root, 'helper', process.execPath, sleeper, { stdio: 'ignore' }); + trackProcess(child); await markDirty(root, 'runner crashed'); const result = await resetDirtyEnvironment(root, { graceMs: 300 }); From 334ff8b159a0bfff0055fc842692771946bd10ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:09:59 +0200 Subject: [PATCH 3/7] test(runner): keep Windows CI from timing out on the first process-identity read The first lock in a process reads its own identity, which on Windows starts PowerShell and can take seconds on a busy runner. Tests now read it before any deadline starts, the test timeout allows for it, and a failed read is no longer remembered for the life of the process. Co-Authored-By: Claude Sonnet 5 --- packages/qa/src/runner/resources.ts | 9 +++++++-- packages/qa/test/fixtures/execution.ts | 5 +++++ packages/qa/vitest.config.ts | 9 +++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 packages/qa/vitest.config.ts diff --git a/packages/qa/src/runner/resources.ts b/packages/qa/src/runner/resources.ts index d418ff8..1655dd4 100644 --- a/packages/qa/src/runner/resources.ts +++ b/packages/qa/src/runner/resources.ts @@ -331,8 +331,13 @@ const LOCK_FILE = '.release-qa-lock.json'; /** Roots this process holds, so a second run in the same process is refused without touching the disk. */ const heldHere = new Set(); let ownIdentity: Promise | undefined; -/** This process's own identity, read once: on Windows reading it costs a PowerShell start. */ -const identityOfThisProcess = (): Promise => (ownIdentity ??= processIdentity(process.pid)); +/** This process's own identity, read once: on Windows reading it costs a PowerShell start. A failed read is not kept. */ +async function identityOfThisProcess(): Promise { + ownIdentity ??= processIdentity(process.pid); + const identity = await ownIdentity; + if (identity === undefined) ownIdentity = undefined; + return identity; +} /** * At most one run at a time may use a test root, or two runs would reset the environment under each other and diff --git a/packages/qa/test/fixtures/execution.ts b/packages/qa/test/fixtures/execution.ts index 44ab35e..6ed4d64 100644 --- a/packages/qa/test/fixtures/execution.ts +++ b/packages/qa/test/fixtures/execution.ts @@ -1,5 +1,6 @@ // Shared setup for tests that run scenarios through executeScenario. import type { ExecutionContext, Lifecycle, Scenario, ScenarioEvent } from '../../src/runner/execute.ts'; +import { acquireTestRoot } from '../../src/runner/resources.ts'; import { candidate, requirement } from './records.ts'; import { hostOs, hostProfile, makeTestRoot } from './processes.ts'; @@ -13,6 +14,10 @@ export function lifecycleOf(calls: string[], overrides: Partial = {}) export async function arrange(overrides: Partial = {}) { const testRoot = await makeTestRoot(); + // The first lock in a process reads its own identity, which on Windows starts PowerShell and can take seconds on a + // busy machine. Do that here, outside any deadline, so tests with short deadlines measure the code, not the machine. + const warmUp = await acquireTestRoot(testRoot); + if (warmUp.ok) await warmUp.release(); const calls: string[] = []; const events: ScenarioEvent[] = []; const controller = new AbortController(); diff --git a/packages/qa/vitest.config.ts b/packages/qa/vitest.config.ts new file mode 100644 index 0000000..19a1299 --- /dev/null +++ b/packages/qa/vitest.config.ts @@ -0,0 +1,9 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + // Reading a process's identity starts PowerShell on Windows, which can take several seconds on a busy CI runner. + testTimeout: 30_000, + hookTimeout: 30_000, + }, +}); From 451436707f2bb4a1260afbee2d7ac8bbe8227e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:33:59 +0200 Subject: [PATCH 4/7] test(runner): share the executor fixtures, scope the longer timeout, remove timing fragility - execute.test.ts uses the shared fixtures (and so the identity warm-up) instead of its own copies - the longer test timeout applies only to the files that read process identity, not the whole package - the late-lock test waits for the late acquisition and polls for the release instead of sleeping - the process-identity test no longer assumes two processes started together have different start times - remove an unused constant Co-Authored-By: Claude Sonnet 5 --- .../qa/test/runner/execute-hardening.test.ts | 7 ++-- .../qa/test/runner/execute-late-lock.test.ts | 23 +++++++---- packages/qa/test/runner/execute.test.ts | 41 +++---------------- .../test/runner/resources-hardening.test.ts | 5 ++- packages/qa/test/runner/resources.test.ts | 8 +++- packages/qa/vitest.config.ts | 9 ---- 6 files changed, 37 insertions(+), 56 deletions(-) delete mode 100644 packages/qa/vitest.config.ts diff --git a/packages/qa/test/runner/execute-hardening.test.ts b/packages/qa/test/runner/execute-hardening.test.ts index 61728fe..a273014 100644 --- a/packages/qa/test/runner/execute-hardening.test.ts +++ b/packages/qa/test/runner/execute-hardening.test.ts @@ -2,16 +2,18 @@ // environment that was not properly cleaned. import { mkdir, rm, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; -import { afterEach, describe, expect, test } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; import { executeScenario } from '../../src/runner/execute.ts'; import { processIdentity, readDirty, readLedger } from '../../src/runner/resources.ts'; import { arrange, lifecycleOf, never, scenarioOf, sleep } from '../fixtures/execution.ts'; import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, startUnrelatedProcess } from '../fixtures/processes.ts'; +// Reading a process's identity starts PowerShell on Windows, which can take seconds on a busy CI runner. +vi.setConfig({ testTimeout: 30_000 }); + afterEach(cleanUpProcessesAndRoots); const exists = (path: string) => stat(path).then(() => true, () => false); -const sleeper = ['-e', 'setInterval(() => {}, 1000)']; describe('fail closed: an environment that may be dirty is never reused', () => { test('when the dirty marker cannot be written, this process still refuses the next run', async () => { @@ -276,4 +278,3 @@ describe('one run at a time per test root', () => { }); }); -void sleeper; diff --git a/packages/qa/test/runner/execute-late-lock.test.ts b/packages/qa/test/runner/execute-late-lock.test.ts index 39a8a2c..3e1d689 100644 --- a/packages/qa/test/runner/execute-late-lock.test.ts +++ b/packages/qa/test/runner/execute-late-lock.test.ts @@ -4,10 +4,12 @@ import { join } from 'node:path'; import { afterEach, expect, test, vi } from 'vitest'; import { executeScenario } from '../../src/runner/execute.ts'; import { acquireTestRoot } from '../../src/runner/resources.ts'; -import { arrange, scenarioOf, sleep } from '../fixtures/execution.ts'; -import { cleanUpProcessesAndRoots } from '../fixtures/processes.ts'; +import { arrange, scenarioOf } from '../fixtures/execution.ts'; +import { cleanUpProcessesAndRoots, eventually } from '../fixtures/processes.ts'; -const slow = vi.hoisted(() => ({ delayMs: 0 })); +vi.setConfig({ testTimeout: 30_000 }); + +const slow = vi.hoisted(() => ({ delayMs: 0, onAcquired: undefined as (() => void) | undefined })); vi.mock('../../src/runner/resources.ts', async (importOriginal) => { const real = await importOriginal(); @@ -15,26 +17,33 @@ vi.mock('../../src/runner/resources.ts', async (importOriginal) => { ...real, acquireTestRoot: async (root: string) => { await new Promise((resolve) => setTimeout(resolve, slow.delayMs)); - return real.acquireTestRoot(root); + const lock = await real.acquireTestRoot(root); + slow.onAcquired?.(); + return lock; }, }; }); afterEach(async () => { slow.delayMs = 0; + slow.onAcquired = undefined; await cleanUpProcessesAndRoots(); }); test('a lock taken after the prerequisites were cut off is given straight back', async () => { const { context, testRoot } = await arrange({ timeouts: { phaseMs: 40, stepsMs: 2000, cleanupMs: 2000 } }); + const lockFile = join(testRoot, '.release-qa-lock.json'); + const exists = () => stat(lockFile).then(() => true, () => false); slow.delayMs = 250; // the disk is slow: the deadline passes before the lock is taken + const lateAcquisition = new Promise((resolve) => { slow.onAcquired = resolve; }); const result = await executeScenario(context, scenarioOf()); expect(result).toMatchObject({ outcome: 'interrupted', reason: 'timeout' }); - await sleep(600); - const lockFile = join(testRoot, '.release-qa-lock.json'); - expect(await stat(lockFile).then(() => true, () => false)).toBe(false); + // Wait for the lock to actually be taken, then for it to be handed back; a check made before the late acquisition + // would find no lock either way and prove nothing. + await lateAcquisition; + await eventually(async () => !(await exists())); slow.delayMs = 0; const lock = await acquireTestRoot(testRoot); expect(lock.ok).toBe(true); diff --git a/packages/qa/test/runner/execute.test.ts b/packages/qa/test/runner/execute.test.ts index 3df921a..af6564f 100644 --- a/packages/qa/test/runner/execute.test.ts +++ b/packages/qa/test/runner/execute.test.ts @@ -2,48 +2,19 @@ import assert from 'node:assert'; import { getEventListeners } from 'node:events'; import { readFile, rm, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; -import { afterEach, describe, expect, test } from 'vitest'; -import { AssertionFailure, executeScenario, type ExecutionContext, type Lifecycle, type Scenario, type ScenarioEvent } from '../../src/runner/execute.ts'; +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { AssertionFailure, executeScenario } from '../../src/runner/execute.ts'; import { readDirty, readLedger, resetDirtyEnvironment, spawnOwned } from '../../src/runner/resources.ts'; import { candidate, requirement } from '../fixtures/records.ts'; +import { arrange, lifecycleOf, never, scenarioOf, started } from '../fixtures/execution.ts'; import { cleanUpProcessesAndRoots, eventually, hostOs, hostProfile, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; +// Reading a process's identity starts PowerShell on Windows, which can take seconds on a busy CI runner. +vi.setConfig({ testTimeout: 30_000 }); + afterEach(cleanUpProcessesAndRoots); const sleeper = ['-e', 'setInterval(() => {}, 1000)']; -const never = () => new Promise(() => {}); - -function lifecycleOf(calls: string[], overrides: Partial = {}): Lifecycle { - const record = (name: string) => async () => { calls.push(name); }; - return { install: record('install'), reset: record('reset'), launch: record('launch'), cleanup: record('cleanup'), ...overrides }; -} - -async function arrange(overrides: Partial = {}) { - const testRoot = await makeTestRoot(); - const calls: string[] = []; - const events: ScenarioEvent[] = []; - const controller = new AbortController(); - const context: ExecutionContext = { - candidate: candidate(), - profile: hostProfile(), - testRoot, - signal: controller.signal, - emit: (event) => { events.push(event); }, - lifecycle: lifecycleOf(calls), - probes: { display: async () => true, audio: async () => true }, - timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 2000 }, - ...overrides, - }; - return { testRoot, calls, events, controller, context }; -} - -const scenarioOf = (overrides: Partial = {}): Scenario => ({ - id: 'persistence', - requirement: requirement({ key: `${hostOs()}/persistence` }), - steps: async () => {}, - ...overrides, -}); -const started = (events: readonly ScenarioEvent[]) => events.filter((e) => e.status === 'started').map((e) => e.phase); describe('a passing run', () => { test('runs the phases in order, emits an event for each, and cleans up', async () => { diff --git a/packages/qa/test/runner/resources-hardening.test.ts b/packages/qa/test/runner/resources-hardening.test.ts index b516c8e..1d13014 100644 --- a/packages/qa/test/runner/resources-hardening.test.ts +++ b/packages/qa/test/runner/resources-hardening.test.ts @@ -2,10 +2,13 @@ // ownership could touch, or fail to stop, the wrong process. import { mkdir, readFile, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; -import { afterEach, describe, expect, test } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; import { acquireTestRoot, cleanupOwnedResources, processIdentity, readLedger, recordOwned, spawnOwned } from '../../src/runner/resources.ts'; import { cleanUpProcessesAndRoots, eventually, isAlive, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; +// Reading a process's identity starts PowerShell on Windows, which can take seconds on a busy CI runner. +vi.setConfig({ testTimeout: 30_000 }); + afterEach(cleanUpProcessesAndRoots); const sleeper = ['-e', 'setInterval(() => {}, 1000)']; diff --git a/packages/qa/test/runner/resources.test.ts b/packages/qa/test/runner/resources.test.ts index 29b9bc4..af6acc2 100644 --- a/packages/qa/test/runner/resources.test.ts +++ b/packages/qa/test/runner/resources.test.ts @@ -1,7 +1,7 @@ import { mkdir, readFile, rm, stat, symlink, writeFile } from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { join, parse } from 'node:path'; -import { afterEach, describe, expect, test } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; import { checkTestRoot, cleanupOwnedResources, @@ -17,6 +17,9 @@ import { } from '../../src/runner/resources.ts'; import { cleanUpProcessesAndRoots, eventually, isAlive, makeTempDir, makeTestRoot, startUnrelatedProcess, trackProcess } from '../fixtures/processes.ts'; +// Reading a process's identity starts PowerShell on Windows, which can take seconds on a busy CI runner. +vi.setConfig({ testTimeout: 30_000 }); + afterEach(cleanUpProcessesAndRoots); const exists = (path: string) => stat(path).then(() => true, () => false); @@ -131,6 +134,9 @@ describe('spawning owned processes', () => { test('the identity of a live process is stable and differs between processes', async () => { const a = startUnrelatedProcess(); + // A start time has the granularity of the operating system's clock (10 ms on Linux); two processes started in the + // same tick share one, which is harmless because an identity is only ever compared under the same pid. + await new Promise((resolve) => setTimeout(resolve, 100)); const b = startUnrelatedProcess(); const first = await processIdentity(a.pid as number); expect(first).toBeDefined(); diff --git a/packages/qa/vitest.config.ts b/packages/qa/vitest.config.ts deleted file mode 100644 index 19a1299..0000000 --- a/packages/qa/vitest.config.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { defineConfig } from 'vitest/config'; - -export default defineConfig({ - test: { - // Reading a process's identity starts PowerShell on Windows, which can take several seconds on a busy CI runner. - testTimeout: 30_000, - hookTimeout: 30_000, - }, -}); From 691e27812a229c8e30a7c9ea848f3d4a76cc5dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:49:52 +0200 Subject: [PATCH 5/7] fix(runner): give a process that is still exiting time to go before calling it unidentifiable On Linux a child that has just exited is a zombie until it is reaped: it still answers "alive" but its identity cannot be read. Cleanup reported such a process as identity-unknown, and spawnOwned killed and rejected a short-lived command whose exit had not been reported yet. Both now wait briefly for the process to finish leaving; one that is still there and unidentifiable is left alone and reported as before. Co-Authored-By: Claude Sonnet 5 --- packages/qa/src/runner/resources.ts | 27 +++++++++++------- .../test/runner/resources-hardening.test.ts | 28 +++++++++++++++++++ packages/qa/test/runner/resources.test.ts | 5 ++-- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/packages/qa/src/runner/resources.ts b/packages/qa/src/runner/resources.ts index 1655dd4..a22e379 100644 --- a/packages/qa/src/runner/resources.ts +++ b/packages/qa/src/runner/resources.ts @@ -185,7 +185,10 @@ export async function spawnOwned(root: string, label: string, command: string, a const hasExited = (): boolean => exited || child.exitCode !== null || child.signalCode !== null; if (hasExited()) return child; if (identity === undefined) { - // Alive but not identifiable: it cannot be owned safely, so it is not left running. + // A child that has just exited is briefly unreadable (on Linux a zombie) before its exit is reported: give it a + // moment. One that is still there afterwards cannot be owned safely, so it is not left running. + await Promise.race([new Promise((resolveExit) => child.once('exit', () => resolveExit())), sleep(500)]); + if (hasExited()) return child; child.kill('SIGKILL'); throw new Error(`could not identify the process started for "${label}"; it was stopped`); } @@ -201,14 +204,6 @@ export async function spawnOwned(root: string, label: string, command: string, a async function cleanProcess(resource: Extract, options: Required): Promise { const { pid } = resource; - if (!isAlive(pid)) return undefined; - const now = await options.identityOf(pid); - if (now === undefined) { - // It may have exited while its identity was being read; otherwise it is alive and unidentifiable: hands off. - return isAlive(pid) ? { resource, reason: 'identity-unknown' } : undefined; - } - if (now !== resource.identity) return undefined; // the pid was reused: the process this run owned is already gone - const waitUntilGone = async (ms: number): Promise => { const end = Date.now() + ms; while (Date.now() < end) { @@ -217,6 +212,18 @@ async function cleanProcess(resource: Extract => ((await waitUntilGone(500)) ? undefined : { resource, reason: 'identity-unknown' }); + + if (!isAlive(pid)) return undefined; + const now = await options.identityOf(pid); + if (now === undefined) return unidentifiable(); + if (now !== resource.identity) return undefined; // the pid was reused: the process this run owned is already gone + /** A signal that could not be delivered is only harmless if the process turned out to be gone anyway. */ const send = async (signal?: NodeJS.Signals): Promise => { try { @@ -235,7 +242,7 @@ async function cleanProcess(resource: Extract { expect(isAlive(child.pid as number)).toBe(true); }); + // On Linux a process that has just exited is briefly a zombie: it still answers "alive" but has no readable + // identity. That is a process on its way out, not one that cannot be identified. + test('does not report a process as unidentifiable when it is gone a moment later', async () => { + const root = await makeTestRoot(); + const child = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: child.pid as number, identity: 'original', label: 'leaving' }); + setTimeout(() => child.kill('SIGKILL'), 100); + + const result = await cleanupOwnedResources(root, { graceMs: 100, identityOf: async () => undefined }); + + expect(result.failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + + test('does not report a process as unidentifiable when it is gone a moment after the grace period', async () => { + const root = await makeTestRoot(); + const child = startUnrelatedProcess(); + await recordOwned(root, { kind: 'process', pid: child.pid as number, identity: 'original', label: 'leaving' }); + const { kill } = fakeKill(); + let lookups = 0; + setTimeout(() => child.kill('SIGKILL'), 250); + + const result = await cleanupOwnedResources(root, { graceMs: 100, escalate: true, kill, identityOf: async () => (++lookups === 1 ? 'original' : undefined) }); + + expect(result.failures).toEqual([]); + expect(await readLedger(root)).toEqual([]); + }); + test('does not escalate at all where the operating system has no polite request', async () => { const root = await makeTestRoot(); const child = startUnrelatedProcess(); diff --git a/packages/qa/test/runner/resources.test.ts b/packages/qa/test/runner/resources.test.ts index af6acc2..355e6a2 100644 --- a/packages/qa/test/runner/resources.test.ts +++ b/packages/qa/test/runner/resources.test.ts @@ -134,8 +134,9 @@ describe('spawning owned processes', () => { test('the identity of a live process is stable and differs between processes', async () => { const a = startUnrelatedProcess(); - // A start time has the granularity of the operating system's clock (10 ms on Linux); two processes started in the - // same tick share one, which is harmless because an identity is only ever compared under the same pid. + // A start time has the granularity of the operating system's clock (10 ms on Linux), so two processes started in + // the same tick would share one and this test would fail for the wrong reason. Cleanup never relies on identities + // differing across pids: it only compares one pid's identity with what was recorded for that same pid. await new Promise((resolve) => setTimeout(resolve, 100)); const b = startUnrelatedProcess(); const first = await processIdentity(a.pid as number); From f60e38df16febc2237c5a58ac2be2e62f5ed7ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 10:58:37 +0200 Subject: [PATCH 6/7] fix(runner): bound the reaping of owned resources by the cleanup deadline Reaping waits for each owned process in turn (a grace period each), and had no deadline of its own, so a ledger with several stubborn processes could hold a run up for as long as the sum of their waits. It is now bounded by cleanupMs; what it does not finish stays on the ledger, is reported in cleanup.failures, and keeps the environment dirty. Co-Authored-By: Claude Sonnet 5 --- packages/qa/src/runner/execute.ts | 14 +++--- .../test/runner/execute-cleanup-bound.test.ts | 43 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 packages/qa/test/runner/execute-cleanup-bound.test.ts diff --git a/packages/qa/src/runner/execute.ts b/packages/qa/src/runner/execute.ts index 174fcbb..f21a95b 100644 --- a/packages/qa/src/runner/execute.ts +++ b/packages/qa/src/runner/execute.ts @@ -73,8 +73,9 @@ export interface ExecutionContext { lifecycle: Lifecycle; probes?: EnvironmentProbes; /** - * Bounds in milliseconds; every wait is bounded, including probes and the event sink. `abandonedGraceMs` is how - * long a hook that was cut off gets to stop by itself before the environment is declared dirty. + * Bounds in milliseconds; every wait is bounded, including probes and the event sink. `cleanupMs` applies to the + * cleanup hook and, separately, to reaping the resources the run still owns. `abandonedGraceMs` is how long a hook + * that was cut off gets to stop by itself before the environment is declared dirty. */ timeouts?: { phaseMs?: number; stepsMs?: number; cleanupMs?: number; abandonedGraceMs?: number }; } @@ -408,12 +409,15 @@ async function cleanUp( failures.push(error instanceof TimedOut ? `cleanup hook ${error.message}` : `cleanup hook failed: ${message(error)}`); } - // Whatever the run still owns is reaped by the runner, whatever the hook did or did not do. + // Whatever the run still owns is reaped by the runner, whatever the hook did or did not do. Reaping waits for each + // process in turn, so it has its own deadline; what it does not finish stays on the ledger and keeps the + // environment dirty. let leftover: CleanupFailure[] = []; try { - leftover = (await cleanupOwnedResources(root)).failures; + const reaped = await race(() => cleanupOwnedResources(root), limits.cleanupMs, new AbortController().signal, () => new TimedOut('cleanup', limits.cleanupMs), 'cleanup'); + leftover = reaped.failures; } catch (error) { - failures.push(`could not clean up owned resources: ${message(error)}`); + failures.push(error instanceof TimedOut ? `reaping owned resources timed out after ${limits.cleanupMs} ms; what is left stays on the ledger` : `could not clean up owned resources: ${message(error)}`); } for (const item of leftover) failures.push(`${item.resource.label}: ${item.reason}${item.detail === undefined ? '' : ` (${item.detail})`}`); diff --git a/packages/qa/test/runner/execute-cleanup-bound.test.ts b/packages/qa/test/runner/execute-cleanup-bound.test.ts new file mode 100644 index 0000000..94e1b14 --- /dev/null +++ b/packages/qa/test/runner/execute-cleanup-bound.test.ts @@ -0,0 +1,43 @@ +// Reaping what a run owns can take a long time (a grace period per process), so it is bounded like everything else. +import { afterEach, expect, test, vi } from 'vitest'; +import { executeScenario } from '../../src/runner/execute.ts'; +import { readDirty } from '../../src/runner/resources.ts'; +import { arrange, never, scenarioOf } from '../fixtures/execution.ts'; +import { cleanUpProcessesAndRoots } from '../fixtures/processes.ts'; + +vi.setConfig({ testTimeout: 30_000 }); + +const reaper = vi.hoisted(() => ({ hang: false })); + +vi.mock('../../src/runner/resources.ts', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + cleanupOwnedResources: (...args: Parameters) => (reaper.hang ? never().then(() => ({ removed: [], failures: [] })) : real.cleanupOwnedResources(...args)), + }; +}); + +afterEach(async () => { + reaper.hang = false; + await cleanUpProcessesAndRoots(); +}); + +test('reaping owned resources that never finishes is cut off at the cleanup deadline and leaves the environment dirty', async () => { + const { context, testRoot } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 150 } }); + reaper.hang = true; + + const began = Date.now(); + const result = await executeScenario(context, scenarioOf()); + + expect(Date.now() - began).toBeLessThan(5000); + expect(result.outcome).toBe('passed'); // the candidate did nothing wrong; the environment is what is in doubt + expect(result.cleanup.ok).toBe(false); + expect(result.cleanup.failures.join(' ')).toMatch(/owned resources.*timed out/i); + expect(await readDirty(testRoot)).toBeDefined(); +}); + +test('reaping that finishes in time is unaffected', async () => { + const { context } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 2000 } }); + const result = await executeScenario(context, scenarioOf()); + expect(result).toMatchObject({ outcome: 'passed', cleanup: { ok: true, failures: [] } }); +}); From 796ab268f2797420ea455d5c88a75a992ab690a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= <81354124+Andreas-Froyland@users.noreply.github.com> Date: Mon, 21 Sep 2026 11:07:59 +0200 Subject: [PATCH 7/7] test(runner): pin the cleanup deadline and show leftovers stay on the ledger Co-Authored-By: Claude Sonnet 5 --- .../qa/test/runner/execute-cleanup-bound.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/qa/test/runner/execute-cleanup-bound.test.ts b/packages/qa/test/runner/execute-cleanup-bound.test.ts index 94e1b14..655c3fa 100644 --- a/packages/qa/test/runner/execute-cleanup-bound.test.ts +++ b/packages/qa/test/runner/execute-cleanup-bound.test.ts @@ -1,7 +1,8 @@ // Reaping what a run owns can take a long time (a grace period per process), so it is bounded like everything else. +import { join } from 'node:path'; import { afterEach, expect, test, vi } from 'vitest'; import { executeScenario } from '../../src/runner/execute.ts'; -import { readDirty } from '../../src/runner/resources.ts'; +import { readDirty, readLedger } from '../../src/runner/resources.ts'; import { arrange, never, scenarioOf } from '../fixtures/execution.ts'; import { cleanUpProcessesAndRoots } from '../fixtures/processes.ts'; @@ -23,17 +24,22 @@ afterEach(async () => { }); test('reaping owned resources that never finishes is cut off at the cleanup deadline and leaves the environment dirty', async () => { - const { context, testRoot } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 150 } }); + const { context, testRoot } = await arrange({ timeouts: { phaseMs: 2000, stepsMs: 2000, cleanupMs: 300 } }); + const owned = { kind: 'path', path: join(testRoot, 'left-behind'), label: 'left behind' } as const; reaper.hang = true; const began = Date.now(); - const result = await executeScenario(context, scenarioOf()); + const result = await executeScenario(context, scenarioOf({ steps: (ctx) => ctx.own(owned) })); + const elapsed = Date.now() - began; - expect(Date.now() - began).toBeLessThan(5000); + // The wait ended because the cleanup deadline passed: not sooner, and not at some much later limit. + expect(elapsed).toBeGreaterThanOrEqual(250); + expect(elapsed).toBeLessThan(1500); expect(result.outcome).toBe('passed'); // the candidate did nothing wrong; the environment is what is in doubt expect(result.cleanup.ok).toBe(false); expect(result.cleanup.failures.join(' ')).toMatch(/owned resources.*timed out/i); expect(await readDirty(testRoot)).toBeDefined(); + expect(await readLedger(testRoot)).toEqual([owned]); // what reaping did not get to is still on the ledger }); test('reaping that finishes in time is unaffected', async () => {