From c4ac5740695903f3768a7f1f6516457d4508f850 Mon Sep 17 00:00:00 2001 From: dgandhi62 Date: Fri, 25 Sep 2026 10:36:11 -0400 Subject: [PATCH 1/2] chore: add install time check for cli test integ harness --- .../@aws-cdk-testing/cli-integ/lib/npm.ts | 67 +++++++++++++++++++ .../lib/package-sources/cli-npm-source.ts | 36 ++++++++-- 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts index 857240a0e..af1a0cab8 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts @@ -1,5 +1,6 @@ // eslint-disable-next-line no-restricted-imports -- cli-integ is a test harness that spawns processes to exercise the CLI as a user would; it is test infrastructure, not shipped runtime. import { spawnSync } from 'child_process'; +import * as path from 'path'; import * as semver from 'semver'; import { shell } from './shell'; @@ -26,6 +27,72 @@ export async function npmMostRecentMatching(packageName: string, range: string) return output[output.length - 1]; } +/** + * `npm install ` into `dir`, retrying a bounded number of times on failure. + * + * npm registry degradations are transient: a single install may fail or land + * incomplete (e.g. a package's version gets recorded but its `bin` never makes + * it into `node_modules/.bin`). A short bounded retry lets a brief blip + * self-heal instead of failing a canary. + */ +export async function npmInstallWithRetry(spec: string, dir: string, attempts: number = 3) { + let lastErr: unknown; + for (let attempt = 1; attempt <= attempts; attempt++) { + try { + await shell(['node', require.resolve('npm'), 'install', spec], { + cwd: dir, + show: 'error', + outputs: [process.stderr], + }); + return; + } catch (e) { + lastErr = e; + if (attempt < attempts) { + // Linear backoff; keep it short so we don't stall a canary for long. + const delayMs = attempt * 2000; + process.stderr.write(`npm install ${spec} failed (attempt ${attempt}/${attempts}), retrying in ${delayMs}ms...\n`); + await new Promise((res) => setTimeout(res, delayMs)); + } + } + } + throw new Error(`npm install ${spec} failed after ${attempts} attempts: ${lastErr}`); +} + +/** + * Verify that the CLI binary installed under `installRoot` is actually runnable. + * + * `npm install` recording a version (see `npmQueryInstalledVersion`) does not + * guarantee the package's `bin` landed in `node_modules/.bin` or that it runs. + * During an npm registry degradation an install can be incomplete: the version + * is recorded but the `cdk` binary is missing, which only surfaces much later + * as `cdk: not found` (exit 127) from inside a test's `cdk synth` — pointing + * investigators at CDK/synth instead of at the install. + * + * Invoking ` --version` through the same install root the tests will use + * turns that downstream failure into a clear, install-time diagnosis. + * + * @param binName - the executable to run (e.g. `cdk`) + * @param installRoot - the directory that contains `node_modules/.bin` + * @param installSpec - the `@` that was installed, for the error message + */ +export async function verifyCliRunnable(binName: string, installRoot: string, installSpec: string) { + const binPath = path.join(installRoot, 'node_modules', '.bin', binName); + try { + await shell([binPath, '--version'], { + cwd: installRoot, + show: 'error', + captureStderr: true, + outputs: [process.stderr], + }); + } catch (e) { + throw new Error( + `CLI install verification failed: '${binName} --version' did not run after installing ${installSpec}. ` + + 'This usually indicates an incomplete or degraded npm install rather than a CDK defect. ' + + `(underlying error: ${e})`, + ); + } +} + export async function npmQueryInstalledVersion(packageName: string, dir: string) { const reportStr = await shell(['node', require.resolve('npm'), 'list', '--json', '--depth', '0', packageName], { cwd: dir, diff --git a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts index 8c548b9a8..abbdb6b7f 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts @@ -2,8 +2,23 @@ import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs-extra'; import type { IRunnerSource, ITestCliSource, IPreparedRunnerSource } from './source'; -import { npmQueryInstalledVersion } from '../npm'; -import { addToShellPath, rimraf, shell } from '../shell'; +import { npmInstallWithRetry, npmQueryInstalledVersion, verifyCliRunnable } from '../npm'; +import { addToShellPath, rimraf } from '../shell'; + +/** + * The executable that a given CLI package installs into `node_modules/.bin`. + * + * npm names the bin after the `bin` key in the package's `package.json`, which + * does not always equal the package name (e.g. `aws-cdk` installs `cdk`). + */ +const CLI_BIN_NAMES: Record = { + 'aws-cdk': 'cdk', + 'cdk-assets': 'cdk-assets', +}; + +function cliBinName(packageName: string): string { + return CLI_BIN_NAMES[packageName] ?? packageName; +} export class RunnerCliNpmSource implements IRunnerSource { public readonly sourceDescription: string; @@ -16,13 +31,20 @@ export class RunnerCliNpmSource implements IRunnerSource { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tmpcdk')); fs.mkdirSync(tempDir, { recursive: true }); - await shell(['node', require.resolve('npm'), 'install', `${this.packageName}@${this.range}`], { - cwd: tempDir, - show: 'error', - outputs: [process.stderr], - }); + const installSpec = `${this.packageName}@${this.range}`; + + // Bounded retry: npm registry degradations are transient, so a brief blip + // shouldn't cut a canary ticket. + await npmInstallWithRetry(installSpec, tempDir); + const installedVersion = await npmQueryInstalledVersion(this.packageName, tempDir); + // Fail fast, with an npm-attributable message, if the bin didn't land or + // isn't runnable. Recording a version does not prove the CLI is usable; + // an incomplete install otherwise surfaces much later as `cdk: not found` + // (exit 127) from inside a test's `cdk synth`, blaming the wrong component. + await verifyCliRunnable(cliBinName(this.packageName), tempDir, installSpec); + return { version: installedVersion, async dispose() { From 1d1af9f3a214510b0583beaf920766859c21bffe Mon Sep 17 00:00:00 2001 From: dgandhi62 Date: Fri, 25 Sep 2026 10:42:33 -0400 Subject: [PATCH 2/2] chore: remove retry mechanism --- .../@aws-cdk-testing/cli-integ/lib/npm.ts | 31 ------------------- .../lib/package-sources/cli-npm-source.ts | 12 ++++--- 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts index af1a0cab8..ea5d11a90 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts @@ -27,37 +27,6 @@ export async function npmMostRecentMatching(packageName: string, range: string) return output[output.length - 1]; } -/** - * `npm install ` into `dir`, retrying a bounded number of times on failure. - * - * npm registry degradations are transient: a single install may fail or land - * incomplete (e.g. a package's version gets recorded but its `bin` never makes - * it into `node_modules/.bin`). A short bounded retry lets a brief blip - * self-heal instead of failing a canary. - */ -export async function npmInstallWithRetry(spec: string, dir: string, attempts: number = 3) { - let lastErr: unknown; - for (let attempt = 1; attempt <= attempts; attempt++) { - try { - await shell(['node', require.resolve('npm'), 'install', spec], { - cwd: dir, - show: 'error', - outputs: [process.stderr], - }); - return; - } catch (e) { - lastErr = e; - if (attempt < attempts) { - // Linear backoff; keep it short so we don't stall a canary for long. - const delayMs = attempt * 2000; - process.stderr.write(`npm install ${spec} failed (attempt ${attempt}/${attempts}), retrying in ${delayMs}ms...\n`); - await new Promise((res) => setTimeout(res, delayMs)); - } - } - } - throw new Error(`npm install ${spec} failed after ${attempts} attempts: ${lastErr}`); -} - /** * Verify that the CLI binary installed under `installRoot` is actually runnable. * diff --git a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts index abbdb6b7f..cb8da3c37 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/package-sources/cli-npm-source.ts @@ -2,8 +2,8 @@ import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs-extra'; import type { IRunnerSource, ITestCliSource, IPreparedRunnerSource } from './source'; -import { npmInstallWithRetry, npmQueryInstalledVersion, verifyCliRunnable } from '../npm'; -import { addToShellPath, rimraf } from '../shell'; +import { npmQueryInstalledVersion, verifyCliRunnable } from '../npm'; +import { addToShellPath, rimraf, shell } from '../shell'; /** * The executable that a given CLI package installs into `node_modules/.bin`. @@ -33,9 +33,11 @@ export class RunnerCliNpmSource implements IRunnerSource { const installSpec = `${this.packageName}@${this.range}`; - // Bounded retry: npm registry degradations are transient, so a brief blip - // shouldn't cut a canary ticket. - await npmInstallWithRetry(installSpec, tempDir); + await shell(['node', require.resolve('npm'), 'install', installSpec], { + cwd: tempDir, + show: 'error', + outputs: [process.stderr], + }); const installedVersion = await npmQueryInstalledVersion(this.packageName, tempDir);