Merge branch 'main' into auto-add-bug-to-board

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.

packages/@aws-cdk-testing/cli-integ/lib/cli/run-suite.ts

@@ -127,7 +127,6 @@ async function main() {
       ...passWithNoTests ? ['--passWithNoTests'] : [],
       ...args['test-file'] ? [args['test-file']] : [],
     ], path.resolve(__dirname, '..', '..', 'resources', 'integ.jest.config.js'));
-
   } finally {
     await packageSource.cleanup();
   }

packages/@aws-cdk-testing/cli-integ/lib/cli/stage-distribution.ts

@@ -52,7 +52,6 @@ async function main() {
         type: 'string',
         requiresArg: true,
       }), async (args) => {
-
       await validateDirectory(args);
       const repo = await (args.name ? TestRepository.newWithName(args.name) : TestRepository.newRandom());
       const usageDir = UsageDir.default();
@@ -71,7 +70,6 @@ async function main() {
         requiresArg: true,
         demandOption: true,
       }), async (args) => {
-
       const repo = TestRepository.existing(args.name);
       const usageDir = UsageDir.default();
 
@@ -99,7 +97,6 @@ async function main() {
         default: true,
         requiresArg: false,
       }), async (args) => {
-
       await validateDirectory(args);
       const repo = await TestRepository.newRandom();
       const usageDir = UsageDir.default();
@@ -114,7 +111,6 @@ async function main() {
           shell: true,
           show: 'always',
         });
-
       } finally {
         if (args.cleanup) {
           await repo.delete();
@@ -128,7 +124,6 @@ async function main() {
         type: 'string',
         requiresArg: true,
       }), async (args) => {
-
       const usageDir = UsageDir.default();
 
       let repositoryName = args.name;

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;

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;
   }
 }
 

packages/@aws-cdk-testing/cli-integ/lib/with-aws.ts

@@ -37,7 +37,6 @@ export function withAws<A extends TestContext>(
   disableBootstrap: boolean = false,
 ): (context: A) => Promise<void> {
   return async (context: A) => {
-
     if (atmosphereEnabled()) {
       const atmosphere = new AtmosphereClient(atmosphereEndpoint());
       const allocation = await atmosphere.acquire({ pool: atmospherePool(), requester: context.name });
@@ -59,7 +58,6 @@ export function withAws<A extends TestContext>(
       } finally {
         await atmosphere.release(allocation.id, outcome);
       }
-
     } else {
       return regionPool().using(async (region) => {
         const aws = await AwsClients.forRegion(region, context.output);
@@ -68,7 +66,6 @@ export function withAws<A extends TestContext>(
         return block({ ...context, disableBootstrap, aws });
       });
     }
-
   };
 }
 

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -331,7 +331,6 @@ export class TestFixture extends ShellHelper {
     public readonly output: NodeJS.WritableStream,
     public readonly aws: AwsClients,
     public readonly randomString: string) {
-
     super(integTestDir, output);
 
     this.packages = packageSourceInSubprocess();
@@ -513,6 +512,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,
       },
@@ -564,11 +565,9 @@ export class TestFixture extends ShellHelper {
    * Cleanup leftover stacks and bootstrapped resources
    */
   public async dispose(success: boolean) {
-
     // when using the atmosphere service, it does resource cleanup on our behalf
     // so we don't have to wait for it.
     if (!atmosphereEnabled()) {
-
       const stacksToDelete = await this.deleteableStacks(this.stackNamePrefix);
 
       this.sortBootstrapStacksToTheEnd(stacksToDelete);
@@ -601,13 +600,15 @@ export class TestFixture extends ShellHelper {
       for (const bucket of this.bucketsToDelete) {
         await this.aws.deleteBucket(bucket);
       }
-
     }
 
     // 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?)`);
+      }
     }
   }
 
@@ -639,7 +640,6 @@ export class TestFixture extends ShellHelper {
 
   private sortBootstrapStacksToTheEnd(stacks: Stack[]) {
     stacks.sort((a, b) => {
-
       if (!a.StackName || !b.StackName) {
         throw new Error('Stack names do not exists. These are required for sorting the bootstrap stacks.');
       }

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,
       },
     });

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`);
 }

packages/@aws-cdk-testing/cli-integ/package.json

@@ -87,4 +87,4 @@
   "publishConfig": {
     "tag": "latest"
   }
-}
+}