From bd48f455793d74604f787a737e6b9dba1e8a459e Mon Sep 17 00:00:00 2001 From: Jimmy Ho Date: Thu, 1 Oct 2026 15:51:47 -0500 Subject: [PATCH] test(npm): one bounded, diagnosable way to run npm synchronously (#1130) The Windows Node 24 failure in #1130 (test/prepare.test.ts, npm pack --ignore-scripts --json) printed its complete pack JSON and was then killed by spawnSync's 30 s timeout, reported only as `null !== 0`. scripts/npm-command.ts gains runNpmSync/npmSpawn/describeNpmRun: npm's JavaScript entry under this Node (no npm.cmd, no shell), stdin ignored, windowsHide, and npm_config_update_notifier/fund/audit/progress forced off, so nothing beyond the command is left for the child to finish. A failure names status, signal, whether the timeout fired, elapsed time and the tails of stderr and stdout. prepare.test.ts and docs-search-package.test.ts use it and log each npm call's elapsed time as a test diagnostic; the timeouts are unchanged. The remaining npm.cmd-through-a-shell fallbacks (addons, private-requests and authjs-provider integration tests, scripts/workspaces.ts run) now go through npmCommand, so every npm the repository spawns is started the same way. docs/CI.md records the helper and how to repeat the test. Refs #1130 Co-Authored-By: Claude Opus 5.5 --- docs/CI.md | 12 +++++ scripts/npm-command.ts | 78 ++++++++++++++++++++++++++++ scripts/workspaces.ts | 5 +- test/addons.integration.ts | 9 ++-- test/authjs-provider.integration.ts | 8 +-- test/docs-search-package.test.ts | 22 +++----- test/npm-command.test.ts | 48 ++++++++++++++++- test/prepare.test.ts | 11 ++-- test/private-requests.integration.ts | 9 ++-- 9 files changed, 164 insertions(+), 38 deletions(-) diff --git a/docs/CI.md b/docs/CI.md index 86e9dbc0..740763d6 100644 --- a/docs/CI.md +++ b/docs/CI.md @@ -205,6 +205,18 @@ stack: Node runs separate `t.after()` hooks in registration order, so an earlier directory-removal hook would run while those later resources are still open. Filesystem retries cannot repair that ordering. +Tests and scripts start npm through `scripts/npm-command.ts`, never through +`npm.cmd` and a shell: `npmCommand` runs npm's JavaScript entry under the +current Node, and `runNpmSync` adds a bounded synchronous run with no stdin, +no update check, funding or audit request, and a failure message that names +the exit status, signal, whether the timeout fired, the elapsed time and the +tail of both streams +([#1130](https://github.com/jimhoyd-com/urlcode/issues/1130)). To repeat a +suspected Windows npm hang, rerun the failing job, or run the test file in a +loop on a Windows machine, for example +`for /L %i in (1,1,50) do npm run test:shard -- test/prepare.test.ts`; each +npm call logs its elapsed time as a test diagnostic. + The store suite uses a two-minute test-file timeout. Keep large suites split into focused files so a file can finish within its budget on supported CI runners, with fixtures isolated between files. Platform-sensitive Windows coverage is Node 24 diff --git a/scripts/npm-command.ts b/scripts/npm-command.ts index e1ba4ced..e0cc7239 100644 --- a/scripts/npm-command.ts +++ b/scripts/npm-command.ts @@ -1,4 +1,6 @@ import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import type { SpawnSyncOptionsWithStringEncoding } from 'node:child_process'; import { existsSync } from 'node:fs'; import { dirname, join } from 'node:path'; @@ -17,3 +19,79 @@ export function npmCommand(args: string[], options: { assert((options.exists ?? existsSync)(adjacent), 'Cannot locate npm-cli.js. Run this helper through npm run so npm_execpath is available.'); return { command: execPath, args: [adjacent, ...args] }; } + +/** + * Settings forced on every npm a test or script runs synchronously (#1130): no update check, funding or audit request + * and no progress output, so nothing but the command itself is left for the child to finish before it exits. + */ +export const quietNpmEnv: Readonly> = { + npm_config_update_notifier: 'false', + npm_config_fund: 'false', + npm_config_audit: 'false', + npm_config_progress: 'false', +}; + +export interface NpmSyncOptions { + cwd: string; + /** The child's whole environment before the quiet settings are layered over it; defaults to this process's. */ + env?: NodeJS.ProcessEnv; + timeoutMs: number; +} + +/** + * The exact spawn of one synchronous npm run: npm's JavaScript entry under this Node (never npm.cmd through a shell, + * whose cmd.exe wrapper can outlive a timeout kill), no stdin to hold open or read, and no console window on Windows. + */ +export function npmSpawn(args: string[], options: NpmSyncOptions & Parameters[1]): { + command: string; args: string[]; options: SpawnSyncOptionsWithStringEncoding; +} { + const { cwd, env = process.env, timeoutMs, ...locate } = options; + const command = npmCommand(args, locate); + return { + ...command, + options: { + cwd, encoding: 'utf8', timeout: timeoutMs, maxBuffer: 64 * 1024 * 1024, windowsHide: true, + stdio: ['ignore', 'pipe', 'pipe'], env: { ...env, ...quietNpmEnv }, + }, + }; +} + +export interface NpmRun { + command: string; args: string[]; cwd: string; + status: number | null; signal: NodeJS.Signals | null; timedOut: boolean; + elapsedMs: number; timeoutMs: number; stdout: string; stderr: string; error?: string; +} + +/** Runs npm once, synchronously, and records what a failure needs to be diagnosed: signal, timeout and elapsed time. */ +export function runNpmSync(args: string[], options: NpmSyncOptions): NpmRun { + const spawn = npmSpawn(args, options); + const started = performance.now(); + const result = spawnSync(spawn.command, spawn.args, spawn.options); + const elapsedMs = Math.round(performance.now() - started); + const code = (result.error as NodeJS.ErrnoException | undefined)?.code; + return { + command: spawn.command, args: spawn.args, cwd: options.cwd, status: result.status, signal: result.signal, + timedOut: code === 'ETIMEDOUT', elapsedMs, timeoutMs: options.timeoutMs, + stdout: result.stdout ?? '', stderr: result.stderr ?? '', ...(result.error ? { error: result.error.message } : {}), + }; +} + +const tail = (text: string, lines = 20): string => { + const kept = text.trimEnd().split('\n'); + return kept.length > lines ? [`... [${kept.length - lines} earlier lines]`, ...kept.slice(-lines)].join('\n') : kept.join('\n'); +}; + +/** One message naming the command, how it ended, how long it took against its limit, and the end of both streams. */ +export function describeNpmRun(run: NpmRun): string { + return [ + `${JSON.stringify([run.command, ...run.args])} in ${run.cwd}`, + `exited with status ${run.status}, signal ${run.signal}, timedOut ${run.timedOut} after ${run.elapsedMs} ms (timeout ${run.timeoutMs} ms)`, + ...(run.error ? [`spawn error: ${run.error}`] : []), + `stderr (tail):\n${tail(run.stderr) || '(empty)'}`, + `stdout (tail):\n${tail(run.stdout) || '(empty)'}`, + ].join('\n'); +} + +export function assertNpmSucceeded(run: NpmRun): asserts run is NpmRun & { status: 0 } { + assert.equal(run.status, 0, describeNpmRun(run)); +} diff --git a/scripts/workspaces.ts b/scripts/workspaces.ts index 0b58eecb..0a8bcad6 100644 --- a/scripts/workspaces.ts +++ b/scripts/workspaces.ts @@ -10,6 +10,7 @@ import { readFile, readdir } from 'node:fs/promises'; import { spawnSync } from 'node:child_process'; import { join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; +import { npmCommand } from './npm-command.ts'; export interface Addon { name: string; kind: 'extension' | 'artifact'; directory: string; packageName: string; version: string; description: string; requires: string[]; uses: string[]; testsWith: string[]; scripts: Record } export const repositoryRoot = resolve(fileURLToPath(new URL('..', import.meta.url))); @@ -44,10 +45,10 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) const list = await addons(); if (command === 'list') process.stdout.write(JSON.stringify(list.map(({ scripts: _scripts, ...addon }) => addon), null, 2) + '\n'); else if (command === 'run' && script) { - const npm = process.platform === 'win32' ? 'npm.cmd' : 'npm'; for (const addon of list) { if (!addon.scripts[script]) continue; - const result = spawnSync(npm, ['run', script, '--workspace', addon.packageName], { stdio: 'inherit', shell: process.platform === 'win32' }); + const npm = npmCommand(['run', script, '--workspace', addon.packageName]); + const result = spawnSync(npm.command, npm.args, { stdio: 'inherit' }); if (result.status !== 0) process.exit(result.status ?? 1); } } else { process.stderr.write('Use: node scripts/workspaces.ts list | run