diff --git a/README.md b/README.md index aaecbcb..494af54 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,8 @@ The Harper binary is resolved in the following order: 1. `harperBinPath` option passed directly to `startHarper()` 2. `HARPER_INTEGRATION_TEST_INSTALL_SCRIPT` environment variable (path to `dist/bin/harper.js`) -3. Auto-resolved from a `harper` package installed as a project dependency +3. The nearest `dist/bin/harper.js` in the working directory or its ancestors — the Harper build (harper, harper-pro) your tests run inside, which takes precedence over the `harper` package npm auto-installs for this package's peer dependency +4. A `harper` package installed as a project dependency: the one `require('harper')` resolves to from the working directory, located through its `bin` field **Options:** diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index 65a656c..f8bff2d 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -1,5 +1,5 @@ import { spawn, ChildProcess } from 'node:child_process'; -import { createWriteStream, existsSync, type WriteStream } from 'node:fs'; +import { createWriteStream, existsSync, readFileSync, type WriteStream } from 'node:fs'; import { join, basename, dirname } from 'node:path'; import { tmpdir } from 'node:os'; import { mkdtemp, mkdir, rm, cp } from 'node:fs/promises'; @@ -143,7 +143,8 @@ export interface StartHarperOptions { * If not provided, resolution order is: * 1. This option * 2. HARPER_INTEGRATION_TEST_INSTALL_SCRIPT environment variable - * 3. Auto-resolved from 'harper' package in node_modules + * 3. Auto-resolved from current directory or ancestors ('dist/bin/harper.js') + * 4. Auto-resolved from the 'harper' package the project (cwd) resolves */ harperBinPath?: string; } @@ -213,8 +214,11 @@ export interface ContextWithHarper extends SuiteContext, TestContext { * Resolution order: * 1. `harperBinPath` argument * 2. `HARPER_INTEGRATION_TEST_INSTALL_SCRIPT` environment variable - * 3. Auto-resolved from 'harper' package in node_modules - * 4. Auto-resolved from current directory or ancestors ('dist/bin/harper.js') + * 3. Auto-resolved from current directory or ancestors ('dist/bin/harper.js') + * 4. Auto-resolved from the 'harper' package the project (cwd) resolves, via its `bin` field + * + * A Harper build enclosing the working directory (harper, harper-pro) is the product under test, + * so it precedes the `harper` package, which npm auto-installs there to satisfy our peer dependency. * * @returns The absolute path to the Harper CLI entry script * @throws {AssertionError} If the script cannot be found @@ -236,18 +240,7 @@ function getHarperScript(harperBinPath?: string): string { return logResolvedHarperScript(envPath, 'HARPER_INTEGRATION_TEST_INSTALL_SCRIPT'); } - // 3. Auto-resolve from node_modules - try { - const require = createRequire(import.meta.url); - const resolved = require.resolve('harper/dist/bin/harper.js'); - if (existsSync(resolved)) { - return logResolvedHarperScript(resolved, 'node_modules'); - } - } catch { - // harper package not found in node_modules - } - - // 4. Auto-resolve from current directory or ancestors + // 3. Auto-resolve from current directory or ancestors let currentDir = process.cwd(); while (true) { const potentialPath = join(currentDir, 'dist/bin/harper.js'); @@ -261,6 +254,12 @@ function getHarperScript(harperBinPath?: string): string { currentDir = parentDir; } + // 4. Auto-resolve from the project's 'harper' dependency + const packageScript = resolveHarperPackageScript(process.cwd()); + if (packageScript && existsSync(packageScript)) { + return logResolvedHarperScript(packageScript, "'harper' package"); + } + throw new Error( `Harper CLI script not found. Provide the path via:\n` + ` - harperBinPath option: startHarper(ctx, { harperBinPath: '/path/to/dist/bin/harper.js' })\n` + @@ -270,6 +269,41 @@ function getHarperScript(harperBinPath?: string): string { ); } +/** + * Locates the CLI script of the `harper` package that `require('harper')` resolves to from + * `projectDir`, via the package's `bin` field: harper's `exports` map exposes only `"."`, so + * `dist/bin/harper.js` cannot be resolved as a subpath. Resolved from the project rather than from + * this module, so it is the project's dependency even when this package is linked from elsewhere. + * + * Returns `undefined` when no `harper` is resolvable. Exported for tests, not from `index.ts`. + */ +export function resolveHarperPackageScript(projectDir: string): string | undefined { + try { + const entryPath = createRequire(join(projectDir, 'noop.js')).resolve('harper'); + let packageDir = dirname(entryPath); + while (true) { + const packageJsonPath = join(packageDir, 'package.json'); + if (existsSync(packageJsonPath)) { + const { name, bin } = JSON.parse(readFileSync(packageJsonPath, 'utf8')); + if (name === 'harper') { + const binPath = typeof bin === 'string' ? bin : bin?.harper; + return binPath ? join(packageDir, binPath) : undefined; + } + } + const parentDir = dirname(packageDir); + if (parentDir === packageDir) return undefined; + packageDir = parentDir; + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'MODULE_NOT_FOUND') return undefined; + throw new Error( + `Could not locate the Harper CLI script of the 'harper' package resolved from ${projectDir}. ` + + `Provide it via the harperBinPath option or HARPER_INTEGRATION_TEST_INSTALL_SCRIPT.`, + { cause: error } + ); + } +} + const loggedHarperScripts = new Set(); /** @@ -278,31 +312,12 @@ const loggedHarperScripts = new Set(); * Which binary runs is the single most consequential (and previously invisible) * decision the harness makes: resolving a stale `harper` package from * node_modules while the caller expected their local build produces confusing - * wholesale test failures with no hint of the cause. When the `node_modules` - * path wins while `./dist/bin/harper.js` also exists, warn with the override. + * wholesale test failures with no hint of the cause. */ function logResolvedHarperScript(scriptPath: string, source: string): string { if (loggedHarperScripts.has(scriptPath)) return scriptPath; loggedHarperScripts.add(scriptPath); console.log(`[integration-testing] Using Harper at ${scriptPath} (via ${source})`); - if (source === 'node_modules') { - // Mirror resolution step 4's ancestor walk so the warning fires even when - // tests run from a subdirectory of the repo that holds the local build. - let currentDir = process.cwd(); - while (true) { - const localDist = join(currentDir, 'dist/bin/harper.js'); - if (existsSync(localDist) && localDist !== scriptPath) { - console.warn( - `[integration-testing] Warning: resolved the 'harper' package from node_modules, but ${localDist} also exists. ` + - `If you meant to test the local build, set HARPER_INTEGRATION_TEST_INSTALL_SCRIPT=${localDist}.` - ); - break; - } - const parentDir = dirname(currentDir); - if (parentDir === currentDir) break; - currentDir = parentDir; - } - } return scriptPath; } diff --git a/test/harperLifecycle.test.ts b/test/harperLifecycle.test.ts index 4e7a70a..74335f5 100644 --- a/test/harperLifecycle.test.ts +++ b/test/harperLifecycle.test.ts @@ -1,11 +1,11 @@ import { test, before, after } from 'node:test'; -import { ok, strictEqual, match, doesNotMatch, rejects } from 'node:assert'; +import { ok, strictEqual, match, doesNotMatch, rejects, throws } from 'node:assert'; import { spawn, type ChildProcess } from 'node:child_process'; import { once } from 'node:events'; -import { mkdtempSync, writeFileSync, rmSync, existsSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, realpathSync, writeFileSync, rmSync, existsSync } from 'node:fs'; import { readFile, writeFile as writeFileAsync } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { setTimeout as sleep } from 'node:timers/promises'; import { fileURLToPath } from 'node:url'; import { @@ -18,6 +18,7 @@ import { runHarperCommand, HarperStartupError, buildHarperChildEnv, + resolveHarperPackageScript, type StartedHarperTestContext, } from '../src/harperLifecycle.ts'; import { @@ -143,6 +144,11 @@ for (const index of [0, 1]) { }); } setInterval(() => {}, 1000); +`, + // Starts Harper with no harperBinPath, so the binary comes from auto-resolution in this cwd. + 'resolve-runner.mjs': ` +const { runHarperCommand } = await import(process.env.HARPER_LIFECYCLE_URL); +await runHarperCommand({ args: [], env: {} }); `, 'worker-harper.mjs': ` import { parentPort, workerData } from 'node:worker_threads'; @@ -1085,3 +1091,98 @@ test('buildHarperChildEnv: isolates HOME/USERPROFILE to dataRootDir by default', strictEqual(env.HOME, '/tmp/data-root'); strictEqual(env.USERPROFILE, '/tmp/data-root'); }); + +// The shape every published harper 5.x has: `exports` maps only ".", so dist/bin/harper.js is not a +// resolvable subpath. +const HARPER_PACKAGE_JSON = { + name: 'harper', + exports: { '.': { types: './dist/index.d.ts', default: './dist/index.js' } }, + bin: { harper: 'dist/bin/harper.js' }, +}; +const BUILT_HARPER_FILES = ['dist/index.js', 'dist/bin/harper.js']; + +function writePackage(dir: string, packageJson: object, files: string[] = []): string { + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify(packageJson)); + for (const file of files) { + mkdirSync(dirname(join(dir, file)), { recursive: true }); + writeFileSync(join(dir, file), ''); + } + return dir; +} + +/** Realpath, since that is what module resolution returns and the assertions compare against. */ +function makeResolutionRoot(): string { + return realpathSync(mkdtempSync(join(fixtureDir, 'resolution-'))); +} + +/** Starts Harper through `runHarperCommand` from `cwd`, with nothing but auto-resolution to pick the binary. */ +async function runAutoResolution(cwd: string): Promise<{ code: number | null; stdout: string; stderr: string }> { + const runner = spawn(process.execPath, [fixtures['resolve-runner.mjs']], { + cwd, + env: { + ...process.env, + HARPER_LIFECYCLE_URL: new URL('../src/harperLifecycle.ts', import.meta.url).href, + HARPER_INTEGRATION_TEST_INSTALL_SCRIPT: undefined, + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + runner.stdout.on('data', (chunk: Buffer) => (stdout += chunk)); + runner.stderr.on('data', (chunk: Buffer) => (stderr += chunk)); + const [code] = await once(runner, 'close'); + return { code, stdout, stderr }; +} + +test("auto-resolution finds the CLI of a 'harper' dependency whose exports map only '.'", async () => { + const projectDir = writePackage(makeResolutionRoot(), { name: 'consumer' }); + const harperDir = writePackage(join(projectDir, 'node_modules', 'harper'), HARPER_PACKAGE_JSON, BUILT_HARPER_FILES); + + const { code, stdout, stderr } = await runAutoResolution(projectDir); + + strictEqual(code, 0, stderr); + ok( + stdout.includes(`Using Harper at ${join(harperDir, 'dist', 'bin', 'harper.js')} (via 'harper' package)`), + stdout + ); +}); + +test('a Harper build enclosing the working directory wins over node_modules/harper', async () => { + // harper-pro's shape: its own build, the registry harper npm auto-installs for this package's peer + // dependency, and tests run from a subdirectory whose package.json only sets the module type. + const productDir = writePackage(makeResolutionRoot(), { name: '@harperfast/harper-pro', bin: { harper: 'dist/bin/harper.js' } }, ['dist/bin/harper.js']); + writePackage(join(productDir, 'node_modules', 'harper'), HARPER_PACKAGE_JSON, BUILT_HARPER_FILES); + const testsDir = writePackage(join(productDir, 'integrationTests'), { type: 'module' }); + + const { code, stdout, stderr } = await runAutoResolution(testsDir); + + strictEqual(code, 0, stderr); + ok(stdout.includes(`Using Harper at ${join(productDir, 'dist', 'bin', 'harper.js')} (via ancestor dist)`), stdout); +}); + +test('resolveHarperPackageScript falls through when the resolved harper is not built', () => { + const projectDir = makeResolutionRoot(); + writePackage(join(projectDir, 'node_modules', 'harper'), HARPER_PACKAGE_JSON); + + strictEqual(resolveHarperPackageScript(projectDir), undefined); +}); + +test("resolveHarperPackageScript reads npm's string form of bin", () => { + const projectDir = makeResolutionRoot(); + const harperDir = writePackage(join(projectDir, 'node_modules', 'harper'), { ...HARPER_PACKAGE_JSON, bin: 'dist/bin/harper.js' }, BUILT_HARPER_FILES); + + strictEqual(resolveHarperPackageScript(projectDir), join(harperDir, 'dist', 'bin', 'harper.js')); +}); + +test('resolveHarperPackageScript reports an installed harper it cannot resolve instead of skipping it', () => { + const projectDir = makeResolutionRoot(); + writePackage(join(projectDir, 'node_modules', 'harper'), { ...HARPER_PACKAGE_JSON, exports: { './cli': './dist/bin/harper.js' } }, BUILT_HARPER_FILES); + + throws( + () => resolveHarperPackageScript(projectDir), + (error: Error) => + /Could not locate the Harper CLI script/.test(error.message) && + (error.cause as NodeJS.ErrnoException).code === 'ERR_PACKAGE_PATH_NOT_EXPORTED' + ); +});