diff --git a/packages/@aws-cdk-testing/cli-integ/lib/index.ts b/packages/@aws-cdk-testing/cli-integ/lib/index.ts index a00964d5d..e84167973 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/index.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/index.ts @@ -2,6 +2,7 @@ export * from './aws'; export * from './corking'; export * from './integ-test'; export * from './memoize'; +export * from './platform'; export * from './resource-pool'; export * from './with-sam'; export * from './shell'; diff --git a/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts b/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts index 49832f245..0a1123b5a 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts @@ -229,12 +229,37 @@ function slugify(x: string) { return x.replace(/[^a-zA-Z0-9_,]+/g, '-'); } -async function atomicWrite(fileName: string, contents: string) { +/** + * Write a file by writing to a temp file and renaming it into place. + * + * On POSIX the final rename atomically replaces any existing destination, and + * concurrent writers of the same target harmlessly clobber each other. On + * Windows, replacing a destination that another process currently has open (or + * that is in a "delete pending" state from a concurrent replace) fails with + * EPERM/EACCES. Multiple test workers rewrite shared log files (notably + * `0-header.md`) at once, so ride out that transient window by retrying the + * rename a handful of times before giving up. + */ +export async function atomicWrite(fileName: string, contents: string) { await fs.promises.mkdir(path.dirname(fileName), { recursive: true }); const tmp = `${fileName}.${process.pid}`; await fs.promises.writeFile(tmp, contents); - await fs.promises.rename(tmp, fileName); + + const maxAttempts = 10; + for (let attempt = 1; ; attempt++) { + try { + await fs.promises.rename(tmp, fileName); + return; + } catch (e: any) { + if (!['EPERM', 'EACCES'].includes(e.code) || attempt >= maxAttempts) { + // Final failure: don't leave the temp file behind as litter. + await fs.promises.rm(tmp, { force: true }).catch(() => undefined); + throw e; + } + await new Promise(ok => setTimeout(ok, Math.floor(Math.random() * 20) + 5)); + } + } } function readSkipFile(filePath?: string): string[] { diff --git a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts index 82c96a5f8..a2a20251a 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/npm.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/npm.ts @@ -40,7 +40,8 @@ export async function npmQueryInstalledVersion(packageName: string, dir: string) * Use NPM preinstalled on the machine to look up a list of TypeScript versions */ export function typescriptVersionsSync(): string[] { - const { stdout } = spawnSync('npm', ['--silent', 'view', `typescript@>=${MINIMUM_VERSION}`, 'version', '--json'], { encoding: 'utf-8' }); + // Invoke npm through Node: on Windows `npm` is a `.cmd` file, which spawnSync cannot execute directly + const { stdout } = spawnSync(process.execPath, [require.resolve('npm'), '--silent', 'view', `typescript@>=${MINIMUM_VERSION}`, 'version', '--json'], { encoding: 'utf-8' }); const versions: string[] = JSON.parse(stdout); return Array.from(new Set(versions.map(v => v.split('.').slice(0, 2).join('.')))); @@ -50,7 +51,7 @@ export function typescriptVersionsSync(): string[] { * Use NPM preinstalled on the machine to query publish times of versions */ export function typescriptVersionsYoungerThanDaysSync(days: number, versions: string[]): string[] { - const { stdout } = spawnSync('npm', ['--silent', 'view', 'typescript', 'time', '--json'], { encoding: 'utf-8' }); + const { stdout } = spawnSync(process.execPath, [require.resolve('npm'), '--silent', 'view', 'typescript', 'time', '--json'], { encoding: 'utf-8' }); const versionTsMap: Record = JSON.parse(stdout); const cutoffDate = new Date(Date.now() - (days * 24 * 3600 * 1000)); diff --git a/packages/@aws-cdk-testing/cli-integ/lib/platform.ts b/packages/@aws-cdk-testing/cli-integ/lib/platform.ts new file mode 100644 index 000000000..e2454dbbb --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/lib/platform.ts @@ -0,0 +1,6 @@ +/** + * Whether the current process is running on Windows. + */ +export function isWindows(): boolean { + return process.platform === 'win32'; +} diff --git a/packages/@aws-cdk-testing/cli-integ/lib/process.ts b/packages/@aws-cdk-testing/cli-integ/lib/process.ts index 9b64ee585..4d961c765 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/process.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/process.ts @@ -1,6 +1,7 @@ import * as child from 'child_process'; import type { Readable, Writable } from 'stream'; import * as pty from 'node-pty'; +import { isWindows } from './platform'; /** * IProcess provides an interface to work with a subprocess. @@ -48,11 +49,23 @@ export class Process { * Spawn a process with a TTY attached. */ public static spawnTTY(command: string, args: string[], options: pty.IPtyForkOptions | pty.IWindowsPtyForkOptions = {}): IProcess { - const process = pty.spawn(command, args, { + // ConPTY resolves the spawned file with SearchPath, which only finds real + // executables — not the .cmd shims npm creates for CLI entrypoints. Route + // the command through the shell, like Process.spawn does with 'shell: true'. + if (isWindows()) { + args = ['/c', command, ...args]; + command = process.env.ComSpec ?? 'cmd.exe'; + } + const ptyProcess = pty.spawn(command, args, { name: 'xterm-color', + // Wide enough that no output line ever hits the terminal width: ConPTY + // (unlike Unix ptys) renders the screen buffer and inserts hard line + // breaks at the width, which splits long prompts across lines and + // breaks the line-based prompt matching in shell(). + cols: 512, ...options, }); - return new PtyProcess(process); + return new PtyProcess(ptyProcess); } /** diff --git a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts index 436ee7633..782614bbb 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts @@ -3,6 +3,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import type { TestContext } from './integ-test'; +import { isWindows } from './platform'; import { Process } from './process'; import type { TemporaryDirectoryContext } from './with-temporary-directory'; @@ -282,15 +283,27 @@ export class ShellHelper { export function rimraf(fsPath: string): boolean { try { let success = true; - const isDir = fs.lstatSync(fsPath).isDirectory(); - - if (isDir) { + const stat = fs.lstatSync(fsPath); + + // `lstat` describes the link itself, not its target, so a symlink is never + // reported as a directory here. That means we never recurse into a symlink's + // target — which may be shared content that other running tests still use + // (e.g. the machine-wide 'node_modules' install) — and only ever remove the + // link entry itself. + if (stat.isDirectory()) { for (const file of fs.readdirSync(fsPath)) { success &&= rimraf(path.join(fsPath, file)); } fs.rmdirSync(fsPath); } else { - fs.unlinkSync(fsPath); + // A regular file or a symlink. On POSIX, unlink removes a symlink whatever + // its target type. On Windows, a link to a directory (or a junction) must + // be removed with rmdir, while a link to a file must be removed with unlink. + if (stat.isSymbolicLink() && isWindows() && isDirectoryLink(fsPath)) { + fs.rmdirSync(fsPath); + } else { + fs.unlinkSync(fsPath); + } } return success; } catch (e: any) { @@ -309,14 +322,25 @@ export function rimraf(fsPath: string): boolean { } } +/** + * Whether a symlink resolves to a directory. + * + * `statSync` follows the link, so a directory target means a directory link. + * The link always points at the live shared install during cleanup, so a + * missing target is not expected: let it throw rather than hide the problem. + */ +function isDirectoryLink(linkPath: string): boolean { + return fs.statSync(linkPath).isDirectory(); +} + export function addToShellPath(x: string) { - const parts = process.env.PATH?.split(':') ?? []; + const parts = process.env.PATH?.split(path.delimiter) ?? []; if (!parts.includes(x)) { parts.unshift(x); } - process.env.PATH = parts.join(':'); + process.env.PATH = parts.join(path.delimiter); } /** @@ -339,7 +363,28 @@ export function addToShellPath(x: string) { class LastLine { private lastLine: string = ''; + // win32 only: the last completed line that had visible content, see below + private lastVisibleLine: string = ''; + public append(chunk: string): void { + if (isWindows()) { + // ConPTY renders the screen buffer instead of streaming plain text: + // prompts are drawn with cursor-positioning escape sequences, padded + // with spaces to the terminal width, and followed by "lines" that + // contain nothing but more escape sequences. Match against the last + // line that had visible content, so control-only lines don't erase a + // prompt that was just drawn. + const lines = stripAnsi(chunk).split(/\r?\n/); + this.lastLine += lines[0]; + for (const line of lines.slice(1)) { + if (this.lastLine.trim().length > 0) { + this.lastVisibleLine = this.lastLine; + } + this.lastLine = line; + } + return; + } + const lines = chunk.split(os.EOL); if (lines.length === 1) { // chunk doesn't contain a new line so just append @@ -351,10 +396,30 @@ class LastLine { } public get(): string { + if (isWindows() && this.lastLine.trim().length === 0) { + return this.lastVisibleLine; + } return this.lastLine; } public reset() { this.lastLine = ''; + this.lastVisibleLine = ''; } } + +const ESC = '\u001b'; +// CSI sequences (cursor movement, erase, colors) and OSC sequences (window title) +const ANSI_REGEX = new RegExp(`${ESC}\\[[0-9;?]*[@-~]|${ESC}\\][^${ESC}\\u0007]*(?:\\u0007|${ESC}\\\\)`, 'g'); + +/** + * Remove ANSI escape sequences from terminal output. + * + * Windows ConPTY renders the screen buffer rather than streaming plain text: + * once the cursor reaches the bottom of the buffer, lines arrive as absolute + * cursor-positioning sequences instead of newline-terminated text. Prompt + * matching must look at the text only. + */ +function stripAnsi(chunk: string): string { + return chunk.replace(ANSI_REGEX, ''); +} diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts index 5923a445e..5e3820df7 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts @@ -1,5 +1,6 @@ /* eslint-disable no-console */ import assert from 'assert'; +import * as crypto from 'crypto'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -11,12 +12,14 @@ import { outputFromStack, sleep } from './aws'; import type { TestContext } from './integ-test'; import type { ITestCliSource, ITestLibrarySource } from './package-sources/source'; import { testSource } from './package-sources/subprocess'; +import { isWindows } from './platform'; import { RESOURCES_DIR } from './resources'; import type { ShellOptions } from './shell'; import { shell, ShellHelper, rimraf } from './shell'; import type { AwsContext, AwsContextOptions } from './with-aws'; import { atmosphereEnabled, withAws } from './with-aws'; import { withTimeout } from './with-timeout'; +import { XpMutexPool } from './xpmutex'; import { findYarnPackages } from './yarn'; export const DEFAULT_TEST_TIMEOUT_S = 20 * 60; @@ -279,9 +282,10 @@ export interface CdkDestroyCliOptions extends CdkCliOptions { * Prepare a target dir byreplicating a source directory */ export async function cloneDirectory(source: string, target: string, output?: NodeJS.WritableStream) { - await shell(['rm', '-rf', target], { outputs: output ? [output] : [] }); - await shell(['mkdir', '-p', target], { outputs: output ? [output] : [] }); - await shell(['cp', '-R', source + '/*', target], { outputs: output ? [output] : [] }); + output?.write(`Cloning ${source} into ${target}\n`); + await fs.promises.rm(target, { recursive: true, force: true }); + await fs.promises.mkdir(target, { recursive: true }); + await fs.promises.cp(source, target, { recursive: true }); } interface CommonCdkBootstrapCommandOptions { @@ -505,15 +509,33 @@ export class TestFixture extends ShellHelper { const tokenResponse = await this.aws.ecrPublic.send(new GetAuthorizationTokenCommand({})); const authData = tokenResponse.authorizationData?.authorizationToken; - const docker = process.env.CDK_DOCKER ?? 'docker'; - if (!authData) { throw new Error('Could not retrieve ECR public auth token.'); } + if (isWindows()) { + // `docker login` on Windows stores credentials through the wincred credential + // helper (auto-detected even if `credsStore` is empty in the config file), and + // wincred cannot store ECR tokens: they exceed Windows Credential Manager's + // 2560-byte limit ('The stub received bad data'). Write the auth directly into + // the per-test Docker config file instead, which is exactly what `docker login` + // produces on the Linux runners, where no credential helper is installed. + // The plaintext `auths` entry takes precedence over any credential helper. + await fs.promises.mkdir(this.dockerConfigDir, { recursive: true }); + await fs.promises.writeFile( + path.join(this.dockerConfigDir, 'config.json'), + JSON.stringify({ auths: { 'public.ecr.aws': { auth: authData } } }), + ); + return; + } + + const docker = process.env.CDK_DOCKER ?? 'docker'; + const decoded = Buffer.from(authData, 'base64').toString('utf-8'); const [username, password] = decoded.split(':'); + // Reference the password via an environment variable so it doesn't leak into + // process listings; the shell expands it. await this.shell([docker, 'login', '--username', username, '--password', '${ECR_PASSWORD}', @@ -1011,9 +1033,13 @@ function hasJsonFlag(args: string[]): boolean { /** * Install the given NPM packages, identified by their names and versions * - * Works by writing the packages to a `package.json` file, and - * then running NPM7's "install" on it. The use of NPM7 will automatically - * install required peerDependencies. + * Works by writing the packages to a `package.json` file, and then running NPM7's + * "install" on it. The use of NPM7 will automatically install required + * peerDependencies. + * + * The install itself is shared: because every test asks for the same handful of + * packages at the same resolved versions, they are installed once per machine and + * linked into each test directory. See `sharedPackageSetInstall`. * * If we're running in REPO mode and we find the package in the set of local * packages in the repository, we'll write the directory name to `package.json` @@ -1027,6 +1053,8 @@ function hasJsonFlag(args: string[]): boolean { * for Node's dependency lookup mechanism). */ export async function installNpmPackages(fixture: TestFixture, packages: Record) { + let hasLocalPackages = false; + if (process.env.REPO_ROOT) { const monoRepo = await findYarnPackages(process.env.REPO_ROOT); @@ -1034,6 +1062,7 @@ export async function installNpmPackages(fixture: TestFixture, packages: Record< for (const key of Object.keys(packages)) { if (key in monoRepo) { packages[key] = monoRepo[key]; + hasLocalPackages = true; } } } @@ -1045,6 +1074,99 @@ export async function installNpmPackages(fixture: TestFixture, packages: Record< devDependencies: packages, }, undefined, 2), { encoding: 'utf-8' }); + if (hasLocalPackages) { + // A local package is referenced by directory, so the package set no longer + // identifies its own contents: rebuilding changes what is on disk without + // changing the requested version. Install per test, so that the dev cycle + // of 'rebuild, rerun the test' keeps working. + await npmInstallWithRetry(fixture, fixture.integTestDir); + return; + } + + // Every test installs the same small set of packages, and `aws-cdk-lib` alone is + // tens of thousands of files, so installing per test is pure duplicated work: it + // is very slow on Windows (minutes instead of seconds), and on every platform it + // means many concurrent `npm install` processes, which is a source of ECONNRESET + // failures. Install each distinct package set once per machine and link it into + // the test directory instead. + const sharedNodeModules = await sharedPackageSetInstall(fixture, packages); + fs.symlinkSync( + sharedNodeModules, + path.join(fixture.integTestDir, 'node_modules'), + // Ignored on POSIX. On Windows a 'junction' works for unprivileged users, + // where a 'dir' symlink needs elevation. + isWindows() ? 'junction' : 'dir', + ); + + // `npm` writes the lock file next to the `package.json` it installed, which is now + // the shared directory, so copy it back into the test directory. Constructs that + // bundle (`NodejsFunction`) find their project root by searching upwards from the + // app for a lock file, and bundle-mount that directory into Docker; without a lock + // file here the search escapes the test directory and synth fails. + fs.copyFileSync( + path.join(sharedNodeModules, '..', 'package-lock.json'), + path.join(fixture.integTestDir, 'package-lock.json'), + ); +} + +/** + * Mutex pool guarding the shared installs, created on first use. + * + * Constructing a pool starts an `fs.watch`, so don't do it for test runs that + * never install anything. + */ +let installMutexPool: XpMutexPool | undefined; + +/** + * Install the given package set into a machine-shared directory, once. + * + * Concurrent callers (jest workers are separate processes) coordinate through a + * cross-process mutex: whoever holds it installs, and everyone else waits and then + * finds the completion marker already there. A worker that dies while installing + * holds a lock nobody would ever release, so `XpMutex` reclaims it once the owning + * pid is gone. + * + * The shared directory is keyed on the requested package set. Those versions are + * always fully resolved by the time they get here (see `requestedVersion()` on the + * library sources), so the key identifies the contents and the directory can be + * reused across runs on the same machine. + * + * @returns the path of the installed `node_modules` directory. + */ +async function sharedPackageSetInstall(fixture: TestFixture, packages: Record): Promise { + const hash = crypto.createHash('sha256').update(JSON.stringify(packages)).digest('hex').slice(0, 16); + const sharedDir = path.join(os.tmpdir(), `cdk-integ-shared-${hash}`); + const nodeModules = path.join(sharedDir, 'node_modules'); + + // Only ever written after a successful install, so a half-installed directory + // (from a worker that was killed) is never handed out. + const completeMarker = path.join(sharedDir, '.install-complete'); + + if (fs.existsSync(completeMarker)) { + return nodeModules; + } + + if (!installMutexPool) { + installMutexPool = XpMutexPool.fromName('cdk-integ-shared-install'); + } + const lock = await installMutexPool.mutex(hash).acquire(); + try { + if (fs.existsSync(completeMarker)) { + return nodeModules; + } + + fixture.log(`Installing shared package set into '${sharedDir}'`); + fs.mkdirSync(sharedDir, { recursive: true }); + fs.copyFileSync(path.join(fixture.integTestDir, 'package.json'), path.join(sharedDir, 'package.json')); + await npmInstallWithRetry(fixture, sharedDir); + fs.writeFileSync(completeMarker, ''); + return nodeModules; + } finally { + await lock.release(); + } +} + +async function npmInstallWithRetry(fixture: TestFixture, cwd: string) { // we often ECONNRESET from NPM so lets retry. this might be because of high concurrency // which overwhelmes system resources. const timeoutMinutes = 10; @@ -1054,7 +1176,10 @@ export async function installNpmPackages(fixture: TestFixture, packages: Record< while (true) { try { // Now install that `package.json` using NPM7 - await fixture.shell(['node', require.resolve('npm'), 'install']); + await shell(['node', require.resolve('npm'), 'install'], { + cwd, + outputs: [fixture.output], + }); break; } catch (e: any) { if (Date.now() < timeoutDate.getTime() && fixture.output.toString().includes('ECONNRESET' )) { diff --git a/packages/@aws-cdk-testing/cli-integ/lib/xpmutex.ts b/packages/@aws-cdk-testing/cli-integ/lib/xpmutex.ts index 372395272..2787c1010 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/xpmutex.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/xpmutex.ts @@ -2,6 +2,30 @@ import { watch, promises as fs, mkdirSync } from 'fs'; import * as os from 'os'; import * as path from 'path'; +/** + * Error codes that mean "the lock file is currently held or in the middle of a + * transition", i.e. we could not create it right now and should back off. + * + * On POSIX the only such signal is `EEXIST` (the exclusive create found the + * file already there). On Windows a file that another process still has open, + * or that was just unlinked, enters a "delete pending" state: it lingers in the + * directory but `open()` against it fails with `EPERM`/`EACCES` instead of + * `EEXIST`. Under contention (many workers racing for the same lock) this is a + * routine, transient condition, not a fatal error, so we treat it the same as + * `EEXIST` and retry. + */ +const CONTENDED_CODES = ['EEXIST', 'EPERM', 'EACCES']; + +/** + * Error codes that mean "the lock file is not readable right now", which we + * treat as "it isn't there" and retry. + * + * `ENOENT` is the file being gone; on Windows `EPERM`/`EACCES` additionally + * cover the delete-pending window, where the name still exists but cannot be + * opened for reading. + */ +const UNREADABLE_CODES = ['ENOENT', 'EPERM', 'EACCES']; + export class XpMutexPool { public static fromDirectory(directory: string) { mkdirSync(directory, { recursive: true }); @@ -96,7 +120,9 @@ export class XpMutex { try { return await this.writePidFile('wx'); // Fails if the file already exists } catch (e: any) { - if (e.code !== 'EEXIST') { + // EEXIST: the lock is held. On Windows a delete-pending lock file + // surfaces as EPERM/EACCES instead; treat those the same way and retry. + if (!CONTENDED_CODES.includes(e.code)) { throw e; } } @@ -104,7 +130,9 @@ export class XpMutex { // File already exists. Read the contents, see if it's an existent PID (if so, the lock is taken) const ownerPid = await this.readPidFile(); if (ownerPid === undefined) { - // File got deleted just now, maybe we can acquire it again + // File got deleted just now (or is mid-transition on Windows). Pause + // briefly so we don't spin on a delete-pending file, then try again. + await randomSleep(10); continue; } if (processExists(ownerPid)) { @@ -164,7 +192,9 @@ export class XpMutex { try { contents = await fs.readFile(this.fileName, { encoding: 'utf-8' }); } catch (e: any) { - if (e.code === 'ENOENT') { + // ENOENT: the file is gone. On Windows a delete-pending file is still + // named but unreadable (EPERM/EACCES); treat it as gone and retry. + if (UNREADABLE_CODES.includes(e.code)) { return undefined; } throw e; diff --git a/packages/@aws-cdk-testing/cli-integ/test/integ-test.test.ts b/packages/@aws-cdk-testing/cli-integ/test/integ-test.test.ts new file mode 100644 index 000000000..7369ec7c7 --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/test/integ-test.test.ts @@ -0,0 +1,64 @@ +import { promises as fs } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { atomicWrite } from '../lib/integ-test'; + +let dir: string; + +beforeEach(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'atomic-write-test-')); +}); + +afterEach(async () => { + await fs.rm(dir, { recursive: true, force: true }); +}); + +test('atomicWrite writes the file contents', async () => { + const target = path.join(dir, 'out.txt'); + await atomicWrite(target, 'hello'); + expect(await fs.readFile(target, 'utf-8')).toBe('hello'); +}); + +test('atomicWrite retries a Windows-style EPERM on rename and still writes the file', async () => { + // On Windows, renaming onto a destination another worker has open fails with + // EPERM. When several workers rewrite the same shared log file concurrently + // this is transient, so atomicWrite must retry rather than propagate it (which + // is what failed the migrate test on the Windows integ runner). + const target = path.join(dir, 'shared.md'); + + const realRename = fs.rename.bind(fs); + let epermInjected = 0; + const spy = jest.spyOn(fs, 'rename').mockImplementation((async (...args: any[]) => { + // Fail the first rename attempt once, as Windows would under contention. + if (epermInjected < 1) { + epermInjected++; + const e: any = new Error('EPERM: operation not permitted, rename'); + e.code = 'EPERM'; + throw e; + } + return realRename(...(args as Parameters)); + }) as unknown as typeof fs.rename); + + try { + await atomicWrite(target, 'body'); // would throw before the fix + expect(epermInjected).toBe(1); + expect(await fs.readFile(target, 'utf-8')).toBe('body'); + } finally { + spy.mockRestore(); + } +}); + +test('atomicWrite rethrows a non-retryable error', async () => { + const target = path.join(dir, 'nope.txt'); + const spy = jest.spyOn(fs, 'rename').mockImplementation((async () => { + const e: any = new Error('ENOSPC: no space left on device'); + e.code = 'ENOSPC'; + throw e; + }) as unknown as typeof fs.rename); + + try { + await expect(atomicWrite(target, 'x')).rejects.toThrow('ENOSPC'); + } finally { + spy.mockRestore(); + } +}); diff --git a/packages/@aws-cdk-testing/cli-integ/test/xpmutex.test.ts b/packages/@aws-cdk-testing/cli-integ/test/xpmutex.test.ts index 73e0d9140..7d10f49a9 100644 --- a/packages/@aws-cdk-testing/cli-integ/test/xpmutex.test.ts +++ b/packages/@aws-cdk-testing/cli-integ/test/xpmutex.test.ts @@ -1,3 +1,4 @@ +import { promises as fs } from 'fs'; import { XpMutexPool } from '../lib/xpmutex'; const POOL = XpMutexPool.fromName('test-pool'); @@ -30,6 +31,36 @@ test('acquire waits', async () => { await secondProcess; }); +test('a Windows delete-pending EPERM on create is treated as contention, not a fatal error', async () => { + // On Windows, creating the lock file can transiently fail with EPERM while a + // just-unlinked file is in "delete pending" state. The mutex must swallow + // that and retry rather than throwing it up to the caller (which is what + // took down the shared-install lock on the Windows integ runner). + const mux = POOL.mutex('windowsEperm'); + + const realOpen = fs.open.bind(fs); + let epermInjected = 0; + const spy = jest.spyOn(fs, 'open').mockImplementation((async (...args: any[]) => { + // Fail the first exclusive-create attempt exactly once, as Windows would. + if (args[1] === 'wx' && epermInjected < 1) { + epermInjected++; + const e: any = new Error("EPERM: operation not permitted, open ''"); + e.code = 'EPERM'; + throw e; + } + return realOpen(...(args as Parameters)); + }) as unknown as typeof fs.open); + + try { + // Would reject with EPERM before the fix; now it retries and succeeds. + const lock = await mux.acquire(); + expect(epermInjected).toBe(1); + await lock.release(); + } finally { + spy.mockRestore(); + } +}); + /** * Poll for some condition every 10ms */ diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-generating-and-loading-assembly.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-generating-and-loading-assembly.integtest.ts index b4106c500..893afa16f 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-generating-and-loading-assembly.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/deploy/cdk-generating-and-loading-assembly.integtest.ts @@ -7,14 +7,14 @@ integTest( 'generating and loading assembly', withDefaultFixture(async (fixture) => { const asmOutputDir = `${fixture.integTestDir}-cdk-integ-asm`; - await fixture.shell(['rm', '-rf', asmOutputDir]); + await fs.rm(asmOutputDir, { recursive: true, force: true }); // Synthesize a Cloud Assembly tothe default directory (cdk.out) and a specific directory. await fixture.cdk(['synth']); await fixture.cdk(['synth', '--output', asmOutputDir]); // cdk.out in the current directory and the indicated --output should be the same - await fixture.shell(['diff', 'cdk.out', asmOutputDir]); + await assertDirsEqual(path.join(fixture.integTestDir, 'cdk.out'), asmOutputDir); // Check that we can 'ls' the synthesized asm. // Change to some random directory to make sure we're not accidentally loading cdk.json @@ -48,3 +48,28 @@ integTest( }), ); +/** + * Assert that two directories have the same files with the same contents (like `diff -r`) + */ +async function assertDirsEqual(dirA: string, dirB: string) { + const filesA = await relativeFiles(dirA); + const filesB = await relativeFiles(dirB); + expect(filesB).toEqual(filesA); + + for (const file of filesA) { + const contentsA = await fs.readFile(path.join(dirA, file), 'utf-8'); + const contentsB = await fs.readFile(path.join(dirB, file), 'utf-8'); + if (contentsA !== contentsB) { + throw new Error(`File ${file} differs between ${dirA} and ${dirB}`); + } + } +} + +async function relativeFiles(root: string): Promise { + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + return entries + .filter((e) => e.isFile()) + .map((e) => path.join(path.relative(root, e.parentPath), e.name)) + .sort(); +} + diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/synth/cdk-templates-on-disk-contain-metadata-resource.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/synth/cdk-templates-on-disk-contain-metadata-resource.integtest.ts index 8587a51ab..082d1db5f 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/synth/cdk-templates-on-disk-contain-metadata-resource.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/synth/cdk-templates-on-disk-contain-metadata-resource.integtest.ts @@ -1,3 +1,5 @@ +import { promises as fs } from 'fs'; +import * as path from 'path'; import { integTest, withDefaultFixture } from '../../../lib'; integTest( @@ -7,17 +9,34 @@ integTest( await fixture.cdk(['synth', '--version-reporting=true']); // Load template from disk from root assembly - const templateContents = await fixture.shell(['cat', 'cdk.out/*-lambda.template.json']); + const templateContents = await readMatchingFile(path.join(fixture.integTestDir, 'cdk.out'), /^[^\\/]*-lambda\.template\.json$/); expect(JSON.parse(templateContents).Resources.CDKMetadata).toBeTruthy(); - // Load template from nested assembly - const nestedTemplateContents = await fixture.shell([ - 'cat', - 'cdk.out/assembly-*-stage/*StackInStage*.template.json', - ]); + // Load template from nested assembly (multiple stage assemblies exist; find the one holding StackInStage) + const nestedTemplate = await findMatchingFile( + path.join(fixture.integTestDir, 'cdk.out'), + /^assembly-.*-stage[\\/].*StackInStage.*\.template\.json$/, + ); + const nestedTemplateContents = await fs.readFile(nestedTemplate, 'utf-8'); expect(JSON.parse(nestedTemplateContents).Resources.CDKMetadata).toBeTruthy(); }), ); +/** + * Find a file whose path relative to `root` matches `pattern`, searching recursively (like a shell glob) + */ +async function findMatchingFile(root: string, pattern: RegExp): Promise { + const entries = await fs.readdir(root, { recursive: true, withFileTypes: true }); + const match = entries.find((e) => e.isFile() && pattern.test(path.join(path.relative(root, e.parentPath), e.name))); + if (!match) { + throw new Error(`No file matching ${pattern} found in ${root}`); + } + return path.join(match.parentPath, match.name); +} + +async function readMatchingFile(root: string, pattern: RegExp): Promise { + return fs.readFile(await findMatchingFile(root, pattern), 'utf-8'); +} + diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-directory-scoped-glob.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-directory-scoped-glob.integtest.ts index 62f2d6303..a3d226ede 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-directory-scoped-glob.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-directory-scoped-glob.integtest.ts @@ -1,7 +1,6 @@ -import * as child_process from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { waitForOutput, waitForCondition, safeKillProcess } from './watch-helpers'; +import { waitForOutput, waitForCondition, safeKillProcess, spawnWatch } from './watch-helpers'; import { integTest, withDefaultFixture } from '../../../lib'; jest.setTimeout(5 * 60 * 1000); // 5 minutes for watch tests @@ -34,11 +33,10 @@ integTest( let output = ''; // Start cdk watch - const watchProcess = child_process.spawn('cdk', [ + const watchProcess = spawnWatch([ 'watch', '--hotswap', '-v', fixture.fullStackName('test-1'), ], { cwd: fixture.integTestDir, - stdio: 'pipe', env: { ...process.env, ...fixture.cdkShellEnv() }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns-negative.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns-negative.integtest.ts index 1dcab2a1a..a95f23482 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns-negative.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns-negative.integtest.ts @@ -1,7 +1,6 @@ -import * as child_process from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { waitForOutput, safeKillProcess } from './watch-helpers'; +import { waitForOutput, safeKillProcess, spawnWatch } from './watch-helpers'; import { integTest, withDefaultFixture, sleep } from '../../../lib'; jest.setTimeout(5 * 60 * 1000); // 5 minutes for watch tests @@ -27,11 +26,10 @@ integTest( let output = ''; // Start cdk watch - const watchProcess = child_process.spawn('cdk', [ + const watchProcess = spawnWatch([ 'watch', '--hotswap', '-v', fixture.fullStackName('test-1'), ], { cwd: fixture.integTestDir, - stdio: 'pipe', env: { ...process.env, ...fixture.cdkShellEnv() }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns.integtest.ts index 815a595fa..7b5f92bb8 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/cdk-watch-detects-file-changes-with-glob-patterns.integtest.ts @@ -1,7 +1,6 @@ -import * as child_process from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { waitForOutput, waitForCondition, safeKillProcess } from './watch-helpers'; +import { waitForOutput, waitForCondition, safeKillProcess, spawnWatch } from './watch-helpers'; import { integTest, withDefaultFixture } from '../../../lib'; jest.setTimeout(5 * 60 * 1000); // 5 minutes for watch tests @@ -26,11 +25,10 @@ integTest( let output = ''; // Start cdk watch - const watchProcess = child_process.spawn('cdk', [ + const watchProcess = spawnWatch([ 'watch', '--hotswap', '-v', fixture.fullStackName('test-1'), ], { cwd: fixture.integTestDir, - stdio: 'pipe', env: { ...process.env, ...fixture.cdkShellEnv() }, }); @@ -51,7 +49,8 @@ integTest( fixture.log('✓ Initial deployment completed'); // Update the test file timestamp to trigger a watch event - child_process.spawnSync('touch', [testFile]); + const now = new Date(); + fs.utimesSync(testFile, now, now); await waitForOutput(() => output, 'Detected change to'); fixture.log('✓ Watch detected file change'); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/watch-helpers.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/watch-helpers.ts index bbe2a918d..b47d259b9 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/watch-helpers.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/watch/watch-helpers.ts @@ -1,4 +1,6 @@ -import type { ChildProcess } from 'node:child_process'; +import * as child_process from 'node:child_process'; +import type { ChildProcess, SpawnOptions } from 'node:child_process'; +import { isWindows } from '../../../lib'; const DEFAULT_POLL_TIMEOUT = 120_000; // 2 minutes @@ -33,12 +35,32 @@ export async function waitForCondition(condition: () => boolean): Promise expect(condition()).toBe(true); } +/** + * Spawn a long-running `cdk watch` process. + * + * On Windows the CLI is an npm .cmd shim, which `spawn` can only start + * through a shell ('spawn cdk ENOENT' otherwise). + */ +export function spawnWatch(args: string[], options: SpawnOptions): ChildProcess { + return child_process.spawn('cdk', args, { + stdio: 'pipe', + shell: isWindows(), + ...options, + }); +} + /** * Kill a spawned process. */ export function safeKillProcess(proc: ChildProcess): void { try { - proc.kill('SIGKILL'); + if (isWindows() && proc.pid !== undefined) { + // Kill the whole tree: the process was spawned through a shell, + // so proc.pid is the shell and 'cdk watch' is its child. + child_process.spawnSync('taskkill', ['/pid', proc.pid.toString(), '/T', '/F']); + } else { + proc.kill('SIGKILL'); + } } catch { // process may have already exited } diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-csharp/init-csharp.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-csharp/init-csharp.integtest.ts index 98fc4da23..617b2c3f2 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-csharp/init-csharp.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-csharp/init-csharp.integtest.ts @@ -10,6 +10,6 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['cdk', 'init', '--lib-version', context.library.requestedVersion(), '-l', 'csharp', template]); await context.library.initializeDotnetPackages(context.integTestDir); await shell.shell(['cdk', 'synth']); - }))); + })), 180_000); }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-fsharp/init-fsharp.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-fsharp/init-fsharp.integtest.ts index b53b28a91..81ac0d32c 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-fsharp/init-fsharp.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-fsharp/init-fsharp.integtest.ts @@ -10,6 +10,6 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['cdk', 'init', '--lib-version', context.library.requestedVersion(), '-l', 'fsharp', template]); await context.library.initializeDotnetPackages(context.integTestDir); await shell.shell(['cdk', 'synth']); - }))); + })), 240_000); }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-go/init-go.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-go/init-go.integtest.ts index cd256f723..8890b481e 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-go/init-go.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-go/init-go.integtest.ts @@ -25,5 +25,5 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['go', 'test']); await shell.shell(['cdk', 'synth']); - }))); + })), 240_000); }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-java/init-java.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-java/init-java.integtest.ts index dbeedda4e..60acc7f42 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-java/init-java.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-java/init-java.integtest.ts @@ -10,5 +10,5 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['cdk', 'init', '--lib-version', context.library.requestedVersion(), '-l', 'java', template]); await shell.shell(['mvn', 'package']); await shell.shell(['cdk', 'synth']); - }))); + })), 180_000); }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-javascript/init-javascript.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-javascript/init-javascript.integtest.ts index 1e01e9767..499fa7e55 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-javascript/init-javascript.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-javascript/init-javascript.integtest.ts @@ -13,7 +13,7 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['npm', 'run', 'test']); await shell.shell(['cdk', 'synth']); - }))); + })), 180_000); }); integTest('Test importing CDK from ESM', withTemporaryDirectory(withPackages(async (context) => { @@ -55,4 +55,4 @@ new TestjsStack(app, 'TestjsStack'); await fs.writeJson(path.join(context.integTestDir, 'cdk.json'), cdkJson); await shell.shell(['cdk', 'synth']); -}))); +})), 180_000); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-python/init-python.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-python/init-python.integtest.ts index 4e4a89b22..2face443f 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-python/init-python.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-python/init-python.integtest.ts @@ -1,5 +1,5 @@ import * as path from 'path'; -import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '../../lib'; +import { integTest, withTemporaryDirectory, ShellHelper, withPackages, isWindows } from '../../lib'; ['app', 'sample-app'].forEach(template => { integTest(`init python ${template}`, withTemporaryDirectory(withPackages(async (context) => { @@ -10,11 +10,13 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. await shell.shell(['cdk', 'init', '--lib-version', context.library.requestedVersion(), '-l', 'python', template]); const venvPath = path.resolve(context.integTestDir, '.venv'); - const venv = { PATH: `${venvPath}/bin:${process.env.PATH}`, VIRTUAL_ENV: venvPath }; + // Virtualenvs put binaries in 'Scripts' on Windows and 'bin' elsewhere + const venvBin = path.join(venvPath, isWindows() ? 'Scripts' : 'bin'); + const venv = { PATH: `${venvBin}${path.delimiter}${process.env.PATH}`, VIRTUAL_ENV: venvPath }; - await shell.shell([`${venvPath}/bin/pip`, 'install', '-r', 'requirements.txt'], { modEnv: venv }); - await shell.shell([`${venvPath}/bin/pip`, 'install', '-r', 'requirements-dev.txt'], { modEnv: venv }); - await shell.shell([`${venvPath}/bin/pytest`], { modEnv: venv }); + await shell.shell([path.join(venvBin, 'pip'), 'install', '-r', 'requirements.txt'], { modEnv: venv }); + await shell.shell([path.join(venvBin, 'pip'), 'install', '-r', 'requirements-dev.txt'], { modEnv: venv }); + await shell.shell([path.join(venvBin, 'pytest')], { modEnv: venv }); await shell.shell(['cdk', 'synth'], { modEnv: venv }); - }))); + })), 240_000); }); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts index 83025e56f..5f8796336 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts @@ -19,7 +19,7 @@ import { typescriptVersionsSync, typescriptVersionsYoungerThanDaysSync } from '. await shell.shell(['npm', 'run', 'test']); await shell.shell(['cdk', 'synth']); - })), 300_000); + })), 600_000); }); // Same as https://github.com/DefinitelyTyped/DefinitelyTyped?tab=readme-ov-file#support-window @@ -55,11 +55,11 @@ TYPESCRIPT_VERSIONS.forEach(tsVersion => { await shell.shell(['npm', 'ls']); // this will fail if we have unmet peer dependencies // We just removed the 'jest' dependency so remove the tests as well because they won't compile - await shell.shell(['rm', '-rf', 'test/']); + await fs.rm(path.join(context.integTestDir, 'test'), { recursive: true, force: true }); await shell.shell(['npm', 'run', 'build']); await shell.shell(['cdk', 'synth']); - }))); + })), 300_000); }); async function removeDevDependencies(context: TemporaryDirectoryContext) { diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/init-typescript-lib.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/init-typescript-lib.integtest.ts index 57d7adfdf..2f73b06ed 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/init-typescript-lib.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/init-typescript-lib.integtest.ts @@ -10,4 +10,4 @@ integTest('typescript init lib', withTemporaryDirectory(withPackages(async (cont await shell.shell(['npm', 'ls']); // this will fail if we have unmet peer dependencies await shell.shell(['npm', 'run', 'build']); await shell.shell(['npm', 'run', 'test']); -}))); +})), 300_000); diff --git a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/use-lib-as-bundled-dependency.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/use-lib-as-bundled-dependency.integtest.ts index c757fa7e7..9b22aab91 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/use-lib-as-bundled-dependency.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/init-typescript-lib/use-lib-as-bundled-dependency.integtest.ts @@ -22,4 +22,4 @@ integTest('using aws-cdk-lib as a bundled dependency', withTemporaryDirectory(wi await fs.writeFile(packageJsonPath, JSON.stringify(packageJson, undefined, 2), 'utf-8'); await shell.shell(['npm', 'install']); -}))); +})), 300_000);