refactor(toolkit): various test organization improvements

.projenrc.ts

@@ -691,6 +691,7 @@ const tmpToolkitHelpers = configureProject(
       'wrap-ansi@^7', // Last non-ESM version
       'yaml@^1',
     ],
+
     tsconfig: {
       compilerOptions: {
         ...defaultTsOptions,
@@ -699,6 +700,24 @@ const tmpToolkitHelpers = configureProject(
         module: 'NodeNext',
       },
     },
+
+    jestOptions: jestOptionsForProject({
+      jestConfig: {
+        coverageThreshold: {
+          // We want to improve our test coverage
+          // DO NOT LOWER THESE VALUES!
+          // If you need to break glass, open an issue to re-up the values with additional test coverage
+          statements: 80,
+          branches: 80,
+          functions: 80,
+          lines: 80,
+        },
+        // We have many tests here that commonly time out
+        testTimeout: 30_000,
+        testEnvironment: './test/_helpers/jest-bufferedconsole.ts',
+        setupFilesAfterEnv: ['<rootDir>/test/_helpers/jest-setup-after-env.ts'],
+      },
+    }),
   }),
 );
 
@@ -713,6 +732,12 @@ tmpToolkitHelpers.package.addField('exports', {
 tmpToolkitHelpers.eslint?.addRules({
   '@cdklabs/no-throw-default-error': 'error',
 });
+tmpToolkitHelpers.eslint?.addOverride({
+  files: ['./test/**'],
+  rules: {
+    '@cdklabs/no-throw-default-error': 'off',
+  },
+});
 
 tmpToolkitHelpers.gitignore.addPatterns('test/**/*.map');
 
@@ -849,9 +874,9 @@ const cli = configureProject(
           '<rootDir>/lib/api/aws-auth/sdk.ts',
 
           // Files generated by cli-args-gen
-          '<rootDir>/lib/parse-command-line-arguments.ts',
-          '<rootDir>/lib/user-input.ts',
-          '<rootDir>/lib/convert-to-user-input.ts',
+          '<rootDir>/lib/cli/parse-command-line-arguments.ts',
+          '<rootDir>/lib/cli/user-input.ts',
+          '<rootDir>/lib/cli/convert-to-user-input.ts',
         ],
         testEnvironment: './test/_helpers/jest-bufferedconsole.ts',
         setupFilesAfterEnv: ['<rootDir>/test/_helpers/jest-setup-after-env.ts'],
@@ -867,7 +892,7 @@ const cli = configureProject(
 
 // Eslint rules
 cli.eslint?.addRules({
-  '@cdklabs/no-throw-default-error': ['error'],
+  '@cdklabs/no-throw-default-error': 'error',
 });
 cli.eslint?.addOverride({
   files: ['./test/**'],
@@ -1177,14 +1202,15 @@ const toolkitLib = configureProject(
     },
     jestOptions: jestOptionsForProject({
       jestConfig: {
-        testEnvironment: './test/_helpers/jest-bufferedconsole.ts',
         coverageThreshold: {
           // this is very sad but we will get better
           statements: 85,
           branches: 76,
           functions: 77,
           lines: 85,
         },
+        testEnvironment: './test/_helpers/jest-bufferedconsole.ts',
+        setupFilesAfterEnv: ['<rootDir>/test/_helpers/jest-setup-after-env.ts'],
       },
     }),
     tsconfig: {

packages/@aws-cdk/tmp-toolkit-helpers/src/api/io/private/index.ts

@@ -5,4 +5,3 @@ export * from './message-maker';
 export * from './messages';
 export * from './types';
 export * from './io-default-messages';
-export * from './testing';

packages/@aws-cdk/tmp-toolkit-helpers/test/_helpers/jest-bufferedconsole.ts

@@ -0,0 +1,111 @@
+/* eslint-disable import/no-extraneous-dependencies,@typescript-eslint/unbound-method */
+/**
+ * A Jest environment that buffers outputs to `console.log()` and only shows it for failing tests.
+ */
+import type { EnvironmentContext, JestEnvironment, JestEnvironmentConfig } from '@jest/environment';
+import type { Circus } from '@jest/types';
+import { TestEnvironment as NodeEnvironment } from 'jest-environment-node';
+
+interface ConsoleMessage {
+  type: 'log' | 'error' | 'warn' | 'info' | 'debug';
+  args: any[];
+}
+
+export default class TestEnvironment extends NodeEnvironment implements JestEnvironment<unknown> {
+  private log = new Array<ConsoleMessage>();
+
+  private originalConsole!: typeof console;
+  private originalStdoutWrite!: typeof process.stdout.write;
+  private originalStderrWrite!: typeof process.stderr.write;
+
+  constructor(config: JestEnvironmentConfig, context: EnvironmentContext) {
+    super(config, context);
+
+    // We need to set the event handler by assignment in the constructor,
+    // because if we declare it as an async member TypeScript's type derivation
+    // doesn't work properly.
+    (this as JestEnvironment<unknown>).handleTestEvent = (async (event, _state) => {
+      if (event.name === 'test_done' && event.test.errors.length > 0 && this.log.length > 0) {
+        this.stopCapture();
+
+        this.originalConsole.log(`[Console output] ${fullTestName(event.test)}\n`);
+        for (const item of this.log) {
+          this.originalConsole[item.type].apply(this.originalConsole, ['    ', ...item.args]);
+        }
+        this.originalConsole.log('\n');
+
+        this.startCapture();
+      }
+
+      if (event.name === 'test_done') {
+        this.log = [];
+      }
+    }) satisfies Circus.EventHandler;
+  }
+
+  async setup() {
+    await super.setup();
+
+    this.log = [];
+    this.startCapture();
+  }
+
+  async teardown() {
+    this.stopCapture();
+    await super.teardown();
+  }
+
+  private startCapture() {
+    this.originalConsole = console;
+    this.originalStdoutWrite = process.stdout.write;
+    this.originalStderrWrite = process.stderr.write;
+
+    this.global.console = {
+      ...console,
+      log: (...args) => this.log.push({ type: 'log', args }),
+      error: (...args) => this.log.push({ type: 'error', args }),
+      warn: (...args) => this.log.push({ type: 'warn', args }),
+      info: (...args) => this.log.push({ type: 'info', args }),
+      debug: (...args) => this.log.push({ type: 'debug', args }),
+    };
+
+    const self = this;
+    process.stdout.write = function (chunk: Buffer | string, enccb?: BufferEncoding | ((error?: Error | null) => void)): void {
+      const encoding = typeof enccb === 'string' ? enccb : 'utf-8';
+      const message = Buffer.isBuffer(chunk) ? chunk.toString(encoding) : chunk;
+      self.log.push({ type: 'log', args: [message.replace(/\n$/, '')] });
+      if (typeof enccb === 'function') {
+        enccb();
+      }
+    } as any;
+    process.stderr.write = function (chunk: Buffer | string, enccb?: BufferEncoding | ((error?: Error | null) => void)): void {
+      const encoding = typeof enccb === 'string' ? enccb : 'utf-8';
+      const message = Buffer.isBuffer(chunk) ? chunk.toString(encoding) : chunk;
+      self.log.push({ type: 'error', args: [message.replace(/\n$/, '')] });
+      if (typeof enccb === 'function') {
+        enccb();
+      }
+    } as any;
+  }
+
+  private stopCapture() {
+    this.global.console = this.originalConsole;
+    process.stdout.write = this.originalStdoutWrite;
+    process.stderr.write = this.originalStderrWrite;
+  }
+}
+
+// DescribeBlock is not exported from `@jest/types`, so we need to build the parts we are interested in
+type TestDescription = PartialBy<Pick<Circus.TestEntry, 'name' | 'parent'>, 'parent'>;
+
+// Utility type to make specific fields optional
+type PartialBy<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>
+
+function fullTestName(test: TestDescription) {
+  let ret = test.name;
+  while (test.parent != null && test.parent.name !== 'ROOT_DESCRIBE_BLOCK') {
+    ret = test.parent.name + ' › ' + ret;
+    test = test.parent;
+  }
+  return ret;
+}

packages/@aws-cdk/tmp-toolkit-helpers/test/_helpers/jest-setup-after-env.ts

@@ -0,0 +1,83 @@
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { isPromise } from 'util/types';
+
+/**
+ * Global test setup for Jest tests
+ *
+ * It's easy to accidentally write tests that interfere with each other by
+ * writing files to disk in the "current directory". To prevent this, the global
+ * test setup creates a directory in the temporary directory and chmods it to
+ * being non-writable. That way, whenever a test tries to write to the current
+ * directory, it will produce an error and we'll be able to find and fix the
+ * test.
+ *
+ * If you see `EACCES: permission denied`, you have a test that creates files
+ * in the current directory, and you should be sure to do it in a temporary
+ * directory that you clean up afterwards.
+ *
+ * ## Alternate approach
+ *
+ * I tried an approach where I would automatically try to create and clean up
+ * temp directories for every test, but it was introducing too many conflicts
+ * with existing test behavior (around specific ordering of temp directory
+ * creation and cleanup tasks that are already present) in many places that I
+ * didn't want to go and chase down.
+ *
+ */
+
+let tmpDir: string;
+let oldDir: string;
+
+beforeAll(() => {
+  tmpDir = path.join(os.tmpdir(), 'cdk-nonwritable-on-purpose');
+  fs.mkdirSync(tmpDir, { recursive: true });
+  fs.chmodSync(tmpDir, 0o500);
+  oldDir = process.cwd();
+  process.chdir(tmpDir);
+  tmpDir = process.cwd(); // This will have resolved symlinks
+});
+
+const reverseAfterAll: Array<jest.ProvidesHookCallback> = [];
+
+/**
+ * We need a cleanup here
+ *
+ * 99% of the time, Jest runs the tests in a subprocess and this isn't
+ * necessary because we would have `chdir`ed in the subprocess.
+ *
+ * But sometimes we ask Jest with `-i` to run the tests in the main process,
+ * or if you only ask for a single test suite Jest runs the tests in the main
+ * process, and then we `chdir`ed the main process away.
+ *
+ * Jest will then try to write the `coverage` directory to the readonly directory,
+ * and fail. Chdir back to the original dir.
+ *
+ * If the test file has an `afterAll()` hook it installed as well, we need to run
+ * it before our cleanup, otherwise the wrong thing will happen (by default,
+ * all `afterAll()`s run in call order, but they should be run in reverse).
+ */
+afterAll(async () => {
+  for (const aft of reverseAfterAll.reverse()) {
+    await new Promise<void>((resolve, reject) => {
+      const response = aft(resolve as any);
+      if (isPromise(response)) {
+        response.then(() => {
+          return resolve();
+        }, reject);
+      } else {
+        resolve();
+      }
+    });
+  }
+
+  if (process.cwd() === tmpDir) {
+    process.chdir(oldDir);
+  }
+});
+
+// Patch afterAll to make later-provided afterAll's run before us (in reverse order even).
+afterAll = (after: jest.ProvidesHookCallback) => {
+  reverseAfterAll.push(after);
+};

packages/@aws-cdk/toolkit-lib/test/_helpers/index.ts

@@ -4,8 +4,8 @@ import * as path from 'node:path';
 import type { AssemblyDirectoryProps, Toolkit } from '../../lib';
 import { ToolkitError } from '../../lib';
 
-export * from '../../lib/api/shared-private';
 export * from './test-cloud-assembly-source';
+export * from './test-io-host';
 
 function fixturePath(...parts: string[]): string {
   return path.normalize(path.join(__dirname, '..', '_fixtures', ...parts));
@@ -18,6 +18,7 @@ export async function appFixture(toolkit: Toolkit, name: string, context?: { [ke
   }
   const app = `cat ${appPath} | node --input-type=module`;
   return toolkit.fromCdkApp(app, {
+    workingDirectory: path.join(__dirname, '..', '..'),
     outdir: tmpOutdir(),
     context,
   });