chore(cli-integ): make it possible to run on GitHub Actions (#33175)

Migrate some changes back from the new testing repo. These changes are necessary to make the tests run on GitHub Actions. If we keep them here, in the future we can do a `cp -R` on the test directory. If not, we'll have to do manual sorting on every copy over, which is annoying and easy to make mistakes in. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Jan 28, 2025 · c8de5be · c8de5be
1 parent 8c13cf2
commit c8de5be
Show file tree

Hide file tree

Showing 12 changed files with 111 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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<string, string>;
+  readonly modEnv?: Record<string, string | undefined>;
 
   /**
    * 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
@@ -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
@@ -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
@@ -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<ActionOutput>((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,20 +248,22 @@ export async function shellWithAction(
     child.stdout!.on('data', chunk => {
       writeToOutputs(chunk);
       stdout.push(chunk);
-      executeAction(chunk);
+      void maybeExecuteAction(chunk);
     });
 
     child.stderr!.on('data', chunk => {
       writeToOutputs(chunk);
       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<string>();
@@ -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
@@ -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
@@ -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
@@ -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/...dk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js b/...dk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js
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
@@ -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/...ges/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts b/...ges/@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']);