diff --git a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts index 64c5b7c2f38fe..ab8006572eff5 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts @@ -252,7 +252,7 @@ export async function sleep(ms: number) { } function chainableCredentials(region: string): AwsCredentialIdentityProvider | undefined { - if (process.env.CODEBUILD_BUILD_ARN && process.env.AWS_PROFILE) { + if ((process.env.CODEBUILD_BUILD_ARN || process.env.GITHUB_RUN_ID) && process.env.AWS_PROFILE) { // in codebuild we must assume the role that the cdk uses // otherwise credentials will just be picked up by the normal sdk // heuristics and expire after an hour. 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 85650d932ef5d..df9d37d134ede 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/integ-test.ts @@ -11,6 +11,12 @@ if (SKIP_TESTS) { process.stderr.write(`ℹ️ Skipping tests: ${JSON.stringify(SKIP_TESTS)}\n`); } +// Whether we want to stop after the first failure, for quicker debugging (hopefully). +const FAIL_FAST = process.env.FAIL_FAST === 'true'; + +// Keep track of whether the suite has failed. If so, we stop running. +let failed = false; + export interface TestContext { readonly randomString: string; readonly name: string; @@ -49,6 +55,10 @@ export function integTest( const now = Date.now(); process.stderr.write(`[INTEG TEST::${name}] Starting (pid ${process.pid})...\n`); try { + if (FAIL_FAST && failed) { + throw new Error('FAIL_FAST requested and currently failing. Stopping test early.'); + } + return await callback({ output, randomString: randomString(), @@ -58,13 +68,41 @@ export function integTest( }, }); } catch (e: any) { - process.stderr.write(`[INTEG TEST::${name}] Failed: ${e}\n`); + failed = true; + + // Print the buffered output, only if the test fails. output.write(e.message); output.write(e.stack); - // Print output only if the test fails. Use 'console.log' so the output is buffered by - // jest and prints without a stack trace (if verbose: false). - // eslint-disable-next-line no-console - console.log(output.buffer().toString()); + process.stderr.write(`[INTEG TEST::${name}] Failed: ${e}\n`); + + const isGitHub = !!process.env.GITHUB_RUN_ID; + + if (isGitHub) { + // GitHub Actions compatible output formatting + // https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-an-error-message + let written = process.stderr.write(`::error title=Failed ${name}::${e.message}\n`); + if (!written) { + // Wait for drain + await new Promise((ok) => process.stderr.once('drain', ok)); + } + + // Print output only if the test fails. Use 'console.log' so the output is buffered by + // jest and prints without a stack trace (if verbose: false). + written = process.stdout.write([ + `::group::Failure details: ${name} (click to expand)\n`, + `${output.buffer().toString()}\n`, + '::endgroup::\n', + ].join('')); + if (!written) { + // Wait for drain + await new Promise((ok) => process.stdout.once('drain', ok)); + } + } else { + // Use 'console.log' so the output is buffered by + // jest and prints without a stack trace (if verbose: false). + // eslint-disable-next-line no-console + console.log(output.buffer().toString()); + } throw e; } finally { const duration = Date.now() - now; diff --git a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts index 4d376b636024f..c462676604ce4 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/shell.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/shell.ts @@ -25,10 +25,6 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom writeToOutputs(`💻 ${command.join(' ')}\n`); const show = options.show ?? 'always'; - if (process.env.VERBOSE) { - outputs.add(process.stdout); - } - const env = options.env ?? (options.modEnv ? { ...process.env, ...options.modEnv } : process.env); const child = child_process.spawn(command[0], command.slice(1), { @@ -81,7 +77,7 @@ export interface ShellOptions extends child_process.SpawnOptions { /** * Properties to add to 'env' */ - readonly modEnv?: Record; + readonly modEnv?: Record; /** * Don't fail when exiting with an error @@ -138,22 +134,33 @@ export class ShellHelper { /** * rm -rf reimplementation, don't want to depend on an NPM package for this + * + * Returns `true` if everything got deleted, or `false` if some files could + * not be deleted due to permissions issues. */ -export function rimraf(fsPath: string) { +export function rimraf(fsPath: string): boolean { try { + let success = true; const isDir = fs.lstatSync(fsPath).isDirectory(); if (isDir) { for (const file of fs.readdirSync(fsPath)) { - rimraf(path.join(fsPath, file)); + success &&= rimraf(path.join(fsPath, file)); } fs.rmdirSync(fsPath); } else { fs.unlinkSync(fsPath); } + return success; } catch (e: any) { - // We will survive ENOENT - if (e.code !== 'ENOENT') { throw e; } + // Can happen if some files got generated inside a Docker container and are now inadvertently owned by `root`. + // We can't ever clean those up anymore, but since it only happens inside GitHub Actions containers we also don't care too much. + if (e.code === 'EACCES' || e.code === 'ENOTEMPTY') { return false; } + + // Already gone + if (e.code === 'ENOENT') { return true; } + + throw e; } } 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 03325ad771ed8..966becf2c2f64 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 @@ -513,6 +513,8 @@ export class TestFixture extends ShellHelper { AWS_DEFAULT_REGION: this.aws.region, STACK_NAME_PREFIX: this.stackNamePrefix, PACKAGE_LAYOUT_VERSION: this.packages.majorVersion(), + // CI must be unset, because we're trying to capture stdout in a bunch of tests + CI: 'false', ...awsCreds, ...options.modEnv, }, @@ -607,7 +609,10 @@ export class TestFixture extends ShellHelper { // If the tests completed successfully, happily delete the fixture // (otherwise leave it for humans to inspect) if (success) { - rimraf(this.integTestDir); + const cleaned = rimraf(this.integTestDir); + if (!cleaned) { + console.error(`Failed to clean up ${this.integTestDir} due to permissions issues (Docker running as root?)`); + } } } diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-cli-lib.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-cli-lib.ts index 8b319b9ed13a3..7bc98bc0b9735 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-cli-lib.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-cli-lib.ts @@ -131,6 +131,9 @@ __EOS__`], { AWS_DEFAULT_REGION: this.aws.region, STACK_NAME_PREFIX: this.stackNamePrefix, PACKAGE_LAYOUT_VERSION: this.packages.majorVersion(), + // Unset CI because we need to distinguish stdout/stderr and this variable + // makes everything go to stdout + CI: 'false', ...options.modEnv, }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts index 9f71386d77028..5e09762f21a6f 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts @@ -1,5 +1,4 @@ import * as child_process from 'child_process'; -import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import axios from 'axios'; @@ -139,6 +138,7 @@ export class SamIntegrationTestFixture extends TestFixture { args.push('--port'); args.push(port.toString()); + // "Press Ctrl+C to quit" looks to be printed by a Flask server built into SAM CLI. return this.samShell(['sam', 'local', 'start-api', ...args], 'Press CTRL+C to quit', ()=>{ return new Promise((resolve, reject) => { axios.get(`http://127.0.0.1:${port}${apiPath}`).then( resp => { @@ -157,7 +157,11 @@ export class SamIntegrationTestFixture extends TestFixture { // If the tests completed successfully, happily delete the fixture // (otherwise leave it for humans to inspect) if (success) { - rimraf(this.integTestDir); + const cleaned = rimraf(this.integTestDir); + if (!cleaned) { + // eslint-disable-next-line no-console + console.error(`Failed to clean up ${this.integTestDir} due to permissions issues (Docker running as root?)`); + } } } } @@ -207,23 +211,24 @@ export async function shellWithAction( let actionOutput: any; let actionExecuted = false; - function executeAction(chunk: any) { + async function maybeExecuteAction(chunk: any) { out.push(Buffer.from(chunk)); if (!actionExecuted && typeof filter === 'string' && Buffer.concat(out).toString('utf-8').includes(filter) && typeof action === 'function') { actionExecuted = true; - writeToOutputs('before executing action'); - action().then((output) => { - writeToOutputs(`action output is ${output}`); + writeToOutputs('before executing action\n'); + try { + const output = await action(); + writeToOutputs(`action output is ${JSON.stringify(output)}\n`); actionOutput = output; actionSucceeded = true; - }).catch((error) => { - writeToOutputs(`action error is ${error}`); + } catch (error: any) { + writeToOutputs(`action error is ${error}\n`); actionSucceeded = false; actionOutput = error; - }).finally(() => { - writeToOutputs('terminate sam sub process'); + } finally { + writeToOutputs('terminate sam sub process\n'); killSubProcess(child, command.join(' ')); - }); + } } } @@ -234,6 +239,7 @@ export async function shellWithAction( () => { if (!actionExecuted) { reject(new Error(`Timed out waiting for filter ${JSON.stringify(filter)} to appear in command output after ${actionTimeoutSeconds} seconds\nOutput so far:\n${Buffer.concat(out).toString('utf-8')}`)); + killSubProcess(child, command.join(' ')); } }, actionTimeoutSeconds * 1_000, ).unref(); @@ -242,7 +248,7 @@ export async function shellWithAction( child.stdout!.on('data', chunk => { writeToOutputs(chunk); stdout.push(chunk); - executeAction(chunk); + void maybeExecuteAction(chunk); }); child.stderr!.on('data', chunk => { @@ -250,12 +256,14 @@ export async function shellWithAction( if (options.captureStderr ?? true) { stderr.push(chunk); } - executeAction(chunk); + void maybeExecuteAction(chunk); }); child.once('error', reject); - child.once('close', code => { + // Wait for 'exit' instead of close, don't care about reading the streams all the way to the end + child.once('exit', (code, signal) => { + writeToOutputs(`Subprocess has exited with code ${code}, signal ${signal}\n`); const output = (Buffer.concat(stdout).toString('utf-8') + Buffer.concat(stderr).toString('utf-8')).trim(); if (code == null || code === 0 || options.allowErrExit) { let result = new Array(); @@ -270,7 +278,6 @@ export async function shellWithAction( reject(new Error(`'${command.join(' ')}' exited with error code ${code}. Output: \n${output}`)); } }); - }); } @@ -279,10 +286,6 @@ function killSubProcess(child: child_process.ChildProcess, command: string) { * Check if the sub process is running in container, so child_process.spawn will * create multiple processes, and to kill all of them we need to run different logic */ - if (fs.existsSync('/.dockerenv')) { - child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`); - } else { - child.kill('SIGINT'); - } - + child.kill('SIGINT'); + child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`); } diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index 61e58871ed2c1..c0ce0ed383886 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -748,7 +748,7 @@ class NotificationArnsStack extends cdk.Stack { const arnsFromEnv = process.env.INTEG_NOTIFICATION_ARNS; super(parent, id, { ...props, - // comma separated list of arns. + // comma separated list of arns. // empty string means empty list. // undefined means undefined notificationArns: arnsFromEnv == '' ? [] : (arnsFromEnv ? arnsFromEnv.split(',') : undefined) @@ -793,12 +793,12 @@ class MetadataStack extends cdk.Stack { const handle = new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle'); handle.addMetadata('Key', process.env.INTEG_METADATA_VALUE ?? 'default') - } + } } const app = new cdk.App({ context: { - '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build + '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID ?? process.env.GITHUB_RUN_ID, // Force all assets to be unique, but consistent in one build }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js index dd117b62a9dd9..413bcbd1972c9 100644 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/rollback-test-app/app.js @@ -91,7 +91,7 @@ class RollbacktestStack extends cdk.Stack { const app = new cdk.App({ context: { - '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build + '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID ?? process.env.GITHUB_RUN_ID, // Force all assets to be unique, but consistent in one build }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/sam_cdk_integ_app/lib/test-stack.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/sam_cdk_integ_app/lib/test-stack.js index 3e9a071de8ff1..935edcf15bf7c 100644 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/sam_cdk_integ_app/lib/test-stack.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/sam_cdk_integ_app/lib/test-stack.js @@ -21,6 +21,8 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { } const isRunningOnCodeBuild = !!process.env.CODEBUILD_BUILD_ID; +const isRunningOnGitHubActions = !!process.env.GITHUB_RUN_ID; +const isRunningOnCi = isRunningOnCodeBuild || isRunningOnGitHubActions; class CDKSupportDemoRootStack extends Stack{ constructor(scope, id, props) { @@ -102,7 +104,7 @@ class CDKSupportDemoRootStack extends Stack{ bundling: { forcedDockerBundling: true, // Only use Google proxy in the CI tests, as it is blocked on workstations - goProxies: isRunningOnCodeBuild ? [GoFunction.GOOGLE_GOPROXY, 'direct'] : undefined, + goProxies: isRunningOnCi ? [GoFunction.GOOGLE_GOPROXY, 'direct'] : undefined, }, }); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js b/packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js index 5895d49ff1ad9..33881210be70c 100644 --- a/packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js @@ -4,7 +4,7 @@ const fs = require("fs"); const path = require("path"); const cdk_1 = require("../helpers/cdk"); const test_helpers_1 = require("../helpers/test-helpers"); -const timeout = process.env.CODEBUILD_BUILD_ID ? // if the process is running in CodeBuild +const timeout = (process.env.CODEBUILD_BUILD_ID ?? process.env.GITHUB_RUN_ID) ? // if the process is running in CodeBuild 3600000 : // 1 hour 600000; // 10 minutes jest.setTimeout(timeout); 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 56d8b9486c6b2..ee5d9418fab37 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 @@ -13,7 +13,12 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '.. // Canaries will use the generated go.mod as is // For pipeline tests we replace the source with the locally build one if (!isCanary) { - await shell.shell(['go', 'mod', 'edit', '-replace', 'github.com/aws/aws-cdk-go/awscdk/v2=$CODEBUILD_SRC_DIR/go/awscdk']); + const dir = process.env.CODEBUILD_SRC_DIR ?? process.env.GITHUB_WORKSPACE; + if (!dir) { + throw new Error('Cannot figure out CI system root directory'); + } + + await shell.shell(['go', 'mod', 'edit', '-replace', `github.com/aws/aws-cdk-go/awscdk/v2=${dir}/go/awscdk`]); } await shell.shell(['go', 'mod', 'tidy']); 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 df1dde2e7b36b..a462a67cce8af 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 @@ -24,9 +24,6 @@ const TYPESCRIPT_VERSION_AGE_DAYS = 2 * 365; const TYPESCRIPT_VERSIONS = typescriptVersionsYoungerThanDaysSync(TYPESCRIPT_VERSION_AGE_DAYS, typescriptVersionsSync()); -// eslint-disable-next-line no-console -console.log(TYPESCRIPT_VERSIONS); - /** * Test our generated code with various versions of TypeScript */ @@ -45,6 +42,10 @@ TYPESCRIPT_VERSIONS.forEach(tsVersion => { await removeDevDependencies(context); await shell.shell(['npm', 'install', '--save-dev', `typescript@${tsVersion}`]); + + // After we've removed devDependencies we need to re-install ts-node because it's necessary for `cdk synth` + await shell.shell(['npm', 'install', '--save-dev', 'ts-node@^10']); + await shell.shell(['npm', 'install']); // Older versions of npm require this to be a separate step from the one above await shell.shell(['npx', 'tsc', '--version']); await shell.shell(['npm', 'prune']);