diff --git a/README.md b/README.md index 750c2d8..9ee90f1 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,8 @@ Edit the `HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT` value in the installed pl The lifecycle and utility APIs below are framework-agnostic. They manage Harper child processes and a cross-process loopback address pool. Use them in the setup/teardown hooks of whichever test framework you prefer. +`startHarper`, `setupHarperWithFixture`, `killHarper` and `teardownHarper` all take the **context** — the object with a `harper` property — not the node held in `ctx.harper`. Passing the node throws a `TypeError`: the teardown pair tells you to wrap it (`teardownHarper({ harper: node })`), while the start pair tells you to pass the context the node came from, since starting from the node would publish a fresh one over it and abandon the instance already running. It used to be accepted silently, which left the node running until the runner exited. A context whose `harper` was never populated is still a no-op, so teardown after a `before` hook that threw stays safe. + ### `startHarper(ctx, options?)` Allocates a loopback address from the pool, creates a temporary install directory, starts a Harper process, and waits for it to be ready. Populates `ctx.harper` with the instance details. Call in a setup/`before()` hook. diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index 026ee03..11f733b 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -530,13 +530,14 @@ export async function setupHarperWithFixture( fixturePath: string, options?: StartHarperOptions ): Promise { + assertHarperTestContext(ctx, 'setupHarperWithFixture', START_FROM_THE_CONTEXT); const dataRootDirPrefix = join( process.env.HARPER_INTEGRATION_TEST_INSTALL_PARENT_DIR || tmpdir(), 'harper-integration-test-' ); const dataRootDir = await mkdtemp(dataRootDirPrefix); await cp(fixturePath, join(dataRootDir, 'components', basename(fixturePath)), { recursive: true, dereference: true }); - ctx.harper = { dataRootDir }; + publishHarperNode(ctx, { dataRootDir }); return startHarper(ctx, options); } @@ -566,6 +567,7 @@ export async function setupHarperWithFixture( * ``` */ export async function startHarper(ctx: HarperTestContext, options?: StartHarperOptions): Promise { + assertHarperTestContext(ctx, 'startHarper', START_FROM_THE_CONTEXT); const dataRootDirPrefix = join( process.env.HARPER_INTEGRATION_TEST_INSTALL_PARENT_DIR || tmpdir(), `harper-integration-test-` @@ -629,7 +631,7 @@ export async function startHarper(ctx: HarperTestContext, options?: StartHarperO maxMs: options?.startupMaxMs, }); - ctx.harper = { + publishHarperNode(ctx, { dataRootDir, admin: { username: DEFAULT_ADMIN_USERNAME, @@ -641,7 +643,7 @@ export async function startHarper(ctx: HarperTestContext, options?: StartHarperO process: result.process, logDir, startupOutput: { stdout: result.stdout, stderr: result.stderr }, - }; + }); return ctx as StartedHarperTestContext; } @@ -721,6 +723,50 @@ function trackHarperProcess(proc: ChildProcess): void { } } +/** Identifies the objects published as `ctx.harper`; a shallow copy of a node does not carry it. */ +const HARPER_NODE = Symbol('harperNode'); + +/** + * The one place a node becomes `ctx.harper`, so every published node carries the brand. + * Exported for tests, not from `index.ts`. + */ +export function publishHarperNode(ctx: HarperTestContext, node: Partial): void { + ctx.harper = markHarperNode(node); +} + +/** Exported for tests, not from `index.ts`. */ +export function markHarperNode(node: T): T { + return Object.defineProperty(node, HARPER_NODE, { value: true, enumerable: false }); +} + +/** + * Rejects the node where the context belongs. Brand rather than field names: this runs when + * `ctx.harper` is falsy, the same path a failed `before` hook takes, so rejecting on a name a + * caller's own context might use (`hostname`, `httpURL`, `process`) would bury the real error. + */ +function assertHarperTestContext(ctx: unknown, fnName: string, remedy: string): void { + if (ctx === null || typeof ctx !== 'object' || Array.isArray(ctx)) { + // An array reaches the no-op the same way a node does: `[a, b].harper` is undefined. + const isArray = Array.isArray(ctx); + const received = ctx === null ? 'null' : isArray ? 'an array' : typeof ctx; + const advice = isArray ? ' Pass each context separately.' : ''; + throw new TypeError(`${fnName}(ctx) requires a test context object, received ${received}.${advice}`); + } + if (HARPER_NODE in ctx) { + throw new TypeError(`${fnName}(ctx) expects the test context, but received the Harper node it holds. ${remedy}`); + } +} + +/** + * Remedies are per-direction, not generated from the function name: telling a caller who reached a + * *start* function with a live node to wrap it would have them overwrite `ctx.harper` and abandon the + * instance already running on it. + */ +const WRAP_THE_NODE = (fnName: string) => + `Wrap it: ${fnName}({ harper: node }) — otherwise the node keeps running until the runner exits.`; +const START_FROM_THE_CONTEXT = + 'Pass the context the node came from. Starting from the node itself would publish a fresh one over it and abandon the instance already running.'; + /** * Kill harper process (can be used for teardown, or killing it before a restart). * @@ -734,6 +780,7 @@ function trackHarperProcess(proc: ChildProcess): void { * {@link DEFAULT_TEARDOWN_GRACE_MS}. */ export async function killHarper(ctx: StartedHarperTestContext, options?: { graceMs?: number }): Promise { + assertHarperTestContext(ctx, 'killHarper', WRAP_THE_NODE('killHarper')); const proc = ctx.harper?.process; if (!proc) return; // Already exited — nothing to do. @@ -790,6 +837,7 @@ export async function killHarper(ctx: StartedHarperTestContext, options?: { grac * ``` */ export async function teardownHarper(ctx: StartedHarperTestContext): Promise { + assertHarperTestContext(ctx, 'teardownHarper', WRAP_THE_NODE('teardownHarper')); if (!ctx.harper) return; await killHarper(ctx); diff --git a/test/harperLifecycle.test.ts b/test/harperLifecycle.test.ts index a710dcd..0f1ba75 100644 --- a/test/harperLifecycle.test.ts +++ b/test/harperLifecycle.test.ts @@ -2,12 +2,17 @@ import { test, before, after } from 'node:test'; import { ok, strictEqual, match, rejects } from 'node:assert'; import { spawn, type ChildProcess } from 'node:child_process'; import { once } from 'node:events'; -import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, writeFileSync, rmSync, existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { setTimeout as sleep } from 'node:timers/promises'; import { killHarper, + teardownHarper, + markHarperNode, + publishHarperNode, + setupHarperWithFixture, + startHarper, runHarperCommand, HarperStartupError, buildHarperChildEnv, @@ -240,6 +245,106 @@ test('killHarper returns immediately when there is no process', async () => { await killHarper({} as unknown as StartedHarperTestContext); }); +test('teardownHarper rejects the node it published, and killHarper does too', async () => { + const node = markHarperNode({ hostname: '127.0.0.2', operationsAPIURL: 'http://127.0.0.2:9925' }); + await rejects( + () => teardownHarper(node as unknown as StartedHarperTestContext), + (error: Error) => { + ok(error instanceof TypeError, `expected a TypeError, got ${error.constructor.name}`); + match(error.message, /expects the test context, but received the Harper node it holds/); + match(error.message, /teardownHarper\(\{ harper: node \}\)/); + return true; + } + ); + await rejects(() => killHarper(node as unknown as StartedHarperTestContext), /killHarper\(ctx\) expects the test context/); +}); + +// `startHarper(ctx.harper)` type-checks; unguarded it claimed a second install and loopback address. +test('the entry points reject the node too, before allocating anything', async () => { + const node = markHarperNode({ hostname: '127.0.0.2', dataRootDir: '/tmp/already-installed' }); + // The remedy must NOT be "wrap it" here: doing that publishes a fresh node over the live one. + for (const start of [ + () => startHarper(node as unknown as StartedHarperTestContext), + () => setupHarperWithFixture(node as unknown as StartedHarperTestContext, fixtureDir), + ]) { + await rejects(start, (error: Error) => { + match(error.message, /expects the test context, but received the Harper node it holds/); + match(error.message, /Pass the context the node came from/); + ok(!/Wrap it:/.test(error.message), 'a start function must not advise wrapping a live node'); + return true; + }); + } +}); + +test('teardownHarper still tears down a correctly wrapped context', async () => { + const dataRootDir = mkdtempSync(join(tmpdir(), 'guard-teardown-')); + await teardownHarper({ harper: markHarperNode({ dataRootDir }) } as unknown as StartedHarperTestContext); + strictEqual(existsSync(dataRootDir), false, 'teardown should have removed the install directory'); +}); + +test('the brand is not enumerable', () => { + const node = markHarperNode({ hostname: '127.0.0.2' }); + strictEqual(Object.keys(node).length, 1); + strictEqual(JSON.stringify(node), '{"hostname":"127.0.0.2"}'); +}); + +test('killHarper rejects a live node unwrapped before signaling it, and reaps it once wrapped', async () => { + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)']); + const node = markHarperNode({ hostname: '127.0.0.2', process: child }); + try { + await rejects(() => killHarper(node as unknown as StartedHarperTestContext), TypeError); + strictEqual(child.exitCode, null, 'the guard must reject before any signal is sent'); + + await killHarper({ harper: node } as unknown as StartedHarperTestContext, { graceMs: 200 }); + ok(child.exitCode !== null || child.signalCode !== null, 'the wrapped call must reap the child'); + } finally { + child.kill('SIGKILL'); + } +}); + +test('an unbranded context is a no-op even when it carries node-like field names', async () => { + await teardownHarper({} as unknown as StartedHarperTestContext); + await teardownHarper({ name: 'suite-that-threw-in-before' } as unknown as StartedHarperTestContext); + await teardownHarper({ + httpURL: 'http://127.0.0.2:3000', + operationsAPIURL: 'http://127.0.0.2:9925', + dataRootDir: '/tmp/a-caller-owned-path', + admin: { username: 'admin' }, + } as unknown as StartedHarperTestContext); +}); + +// Every node reaches `ctx.harper` through publishHarperNode, so branding it there is what makes the +// guard apply to real nodes. Exercised directly: reaching the two call sites needs a loopback slot +// and a real install, which this suite deliberately never takes. +test('publishHarperNode brands what it assigns', async () => { + const dataRootDir = mkdtempSync(join(tmpdir(), 'guard-publish-')); + const ctx: { harper?: Partial<{ dataRootDir: string }> } = {}; + publishHarperNode(ctx, { dataRootDir }); + strictEqual(ctx.harper?.dataRootDir, dataRootDir); + await rejects( + () => teardownHarper(ctx.harper as unknown as StartedHarperTestContext), + /received the Harper node it holds/ + ); + await teardownHarper(ctx as unknown as StartedHarperTestContext); + strictEqual(existsSync(dataRootDir), false, 'the wrapped context must still tear down'); +}); + +test('the shape guard rejects a non-object argument', async () => { + await rejects( + () => teardownHarper(undefined as unknown as StartedHarperTestContext), + /requires a test context object, received undefined/ + ); + await rejects( + () => teardownHarper(null as unknown as StartedHarperTestContext), + /requires a test context object, received null/ + ); + // A suite that keeps its nodes in an array reaches the no-op this way: `[a, b].harper` is undefined. + await rejects( + () => teardownHarper([{ harper: markHarperNode({}) }] as unknown as StartedHarperTestContext), + /requires a test context object, received an array\. Pass each context separately\./ + ); +}); + test('killHarper returns immediately for an already-exited process', async () => { const child = spawn(process.execPath, ['-e', 'process.exit(0)']); await once(child, 'exit');