Skip to content

Commit

Permalink
feat(integ-runner): integ-runner enhancements (#19865)
Browse files Browse the repository at this point in the history 
This PR adds a couple of enhancements to the `integ-runner`

1. Add `--profiles` option which allows you to provide a list of
   AWS profiles to use. This will be used in combination with the
   `--parallel-regions` option to run the integration tests in parallel
   across multiple accounts + regions.
2. Adds `--from-file` option that allows you to store a list of tests to
   run (or exclude) in a file, rather than passing test names as
   arguments.
3. Adds `--exclude` option, which when provided will excluded the tests
   provided as arguments
4. Adds some additional metrics. Now each test will show the total time
   in the logs. Also, if `--verbose` option is provided a summary will
   be printed which will contain metrics for the entire test.
```bash
  --- Integration test metrics ---
Profile mb-dev + Region us-east-2 total time: 349.866
  test/integ.bundling.js: 58.75
  test/integ.lambda-insights-mapping.js: 61.012
  test/integ.assets.lit.js: 61.121
  test/integ.lambda.docker.js: 81.936
  test/integ.log-retention.js: 87.047
Profile mb-dev + Region us-west-2 total time: 359.775
  test/integ.layer-version.lit.js: 72.61
  test/integ.autoscaling.lit.js: 82.82
  test/integ.current-version.js: 84.982
  test/integ.lambda.js: 119.363
Profile mb-dev + Region us-east-1 total time: 378.713
  test/integ.assets.file.js: 67.915
  test/integ.runtime.inlinecode.js: 68.211
```

There are also a couple of small refactors.
1. Split up `workers.test.ts` into `snapshot-worker.test.ts` and `integ-worker.test.ts`
2. Refactored the `extract_workers` functions. There used to be two functions `snapshotTestBatch` and `integTestBatch` which called `singleThreadedXXX` functions. Since each request is essentially single threaded I've moved the `singleThreadedXXX` functions into `extract_worker` and renamed them to be `integTestWorker` and `snapshotTestWorker`

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@corymhall
corymhall authored Apr 12, 2022
1 parent ad249c9 commit 697fdbe
Show file tree
Hide file tree
Showing 15 changed files with 1,060 additions and 510 deletions.
6 changes: 5 additions & 1 deletion packages/@aws-cdk/integ-runner/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ to be a self contained CDK app. The runner will execute the following for each f
Search for integration tests recursively from this starting directory
- `--force` (default=`false`)
Rerun integration test even if the test passes
- `--file`
- `--profiles`
List of AWS Profiles to use when running tests in parallel
- `--exclude` (default=`false`)
If this is set to `true` then the list of tests provided will be excluded
- `--from-file`
Read the list of tests from this file

Example:
Expand Down
45 changes: 34 additions & 11 deletions packages/@aws-cdk/integ-runner/lib/cli.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// Exercise all integ stacks and if they deploy, update the expected synth files
import * as os from 'os';
import * as path from 'path';
import * as workerpool from 'workerpool';
import * as logger from './logger';
import { IntegrationTests, IntegTestConfig } from './runner/integ-tests';
import { runSnapshotTests, runIntegrationTests } from './workers';
import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics } from './workers';

// https://github.com/yargs/yargs/issues/1929
// https://github.com/evanw/esbuild/issues/1492
Expand All @@ -24,35 +23,46 @@ async function main() {
.option('parallel', { type: 'boolean', default: false, desc: 'run integration tests in parallel' })
.option('parallel-regions', { type: 'array', desc: 'if --parallel is used then these regions are used to run tests in parallel', nargs: 1, default: [] })
.options('directory', { type: 'string', default: 'test', desc: 'starting directory to discover integration tests' })
.options('profiles', { type: 'array', desc: 'list of AWS profiles to use. Tests will be run in parallel across each profile+regions', nargs: 1, default: [] })
.options('max-workers', { type: 'number', desc: 'The max number of workerpool workers to use when running integration tests in parallel', default: 16 })
.options('exclude', { type: 'boolean', desc: 'All tests should be run, except for the list of tests provided', default: false })
.options('from-file', { type: 'string', desc: 'Import tests to include or exclude from a file' })
.argv;

// Cap to a reasonable top-level limit to prevent thrash on machines with many, many cores.
const maxWorkers = parseInt(process.env.CDK_INTEG_MAX_WORKER_COUNT ?? '16');
const N = Math.min(maxWorkers, Math.max(1, Math.ceil(os.cpus().length / 2)));
const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), {
maxWorkers: N,
maxWorkers: argv['max-workers'],
});

// list of integration tests that will be executed
const testsToRun: IntegTestConfig[] = [];
const testsFromArgs: IntegTestConfig[] = [];
const parallelRegions = arrayFromYargs(argv['parallel-regions']);
const testRegions: string[] = parallelRegions ?? ['us-east-1', 'us-east-2', 'us-west-2'];
const profiles = arrayFromYargs(argv.profiles);
const runUpdateOnFailed = argv['update-on-failed'] ?? false;
const fromFile: string | undefined = argv['from-file'];
const exclude: boolean = argv.exclude;

let failedSnapshots: IntegTestConfig[] = [];
try {
if (argv['max-workers'] < testRegions.length * (profiles ?? [1]).length) {
logger.warning('You are attempting to run %s tests in parallel, but only have %s workers. Not all of your profiles+regions will be utilized', argv.profiles*argv['parallel-regions'], argv['max-workers']);
}

try {
if (argv.list) {
const tests = await new IntegrationTests(argv.directory).fromCliArgs();
process.stdout.write(tests.map(t => t.fileName).join('\n') + '\n');
return;
}

if (argv._.length === 0) {
if (argv._.length > 0 && fromFile) {
throw new Error('A list of tests cannot be provided if "--from-file" is provided');
} else if (argv._.length === 0 && !fromFile) {
testsFromArgs.push(...(await new IntegrationTests(argv.directory).fromCliArgs()));
} else if (fromFile) {
testsFromArgs.push(...(await new IntegrationTests(argv.directory).fromFile(fromFile)));
} else {
testsFromArgs.push(...(await new IntegrationTests(argv.directory).fromCliArgs(argv._.map((x: any) => x.toString()))));
testsFromArgs.push(...(await new IntegrationTests(argv.directory).fromCliArgs(argv._.map((x: any) => x.toString()), exclude)));
}

// If `--force` is not used then first validate the snapshots and gather
Expand All @@ -65,20 +75,23 @@ async function main() {
testsToRun.push(...testsFromArgs);
}


// run integration tests if `--update-on-failed` OR `--force` is used
if (runUpdateOnFailed || argv.force) {
const success = await runIntegrationTests({
const { success, metrics } = await runIntegrationTests({
pool,
tests: testsToRun,
regions: testRegions,
profiles,
clean: argv.clean,
dryRun: argv['dry-run'],
verbose: argv.verbose,
});
if (!success) {
throw new Error('Some integration tests failed!');
}
if (argv.verbose) {
printMetrics(metrics);
}

if (argv.clean === false) {
logger.warning('Not cleaning up stacks since "--no-clean" was used');
Expand All @@ -97,6 +110,16 @@ async function main() {
}
}

function printMetrics(metrics: IntegRunnerMetrics[]): void {
logger.highlight(' --- Integration test metrics ---');
const sortedMetrics = metrics.sort((a, b) => a.duration - b.duration);
sortedMetrics.forEach(metric => {
logger.print('Profile %s + Region %s total time: %s', metric.profile, metric.region, metric.duration);
const sortedTests = Object.entries(metric.tests).sort((a, b) => a[1] - b[1]);
sortedTests.forEach(test => logger.print(' %s: %s', test[0], test[1]));
});
}

/**
* Translate a Yargs input array to something that makes more sense in a programming language
* model (telling the difference between absence and an empty array)
Expand Down
84 changes: 69 additions & 15 deletions packages/@aws-cdk/integ-runner/lib/runner/integ-tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,33 @@ import * as fs from 'fs-extra';
* Represents a single integration test
*/
export interface IntegTestConfig {
readonly directory: string;
/**
* The name of the file that contains the
* integration tests. This will be in the format
* of integ.{test-name}.js
*/
readonly fileName: string;
}

/**
* The list of tests to run can be provided in a file
* instead of as command line arguments.
*/
export interface IntegrationTestFileConfig {
/**
* If this is set to true then the list of tests
* provided will be excluded
*
* @default false
*/
readonly exclude?: boolean;

/**
* List of tests to include (or exclude if `exclude=true`)
*/
readonly tests: string[];
}

/**
* Discover integration tests
*/
Expand All @@ -17,28 +40,47 @@ export class IntegrationTests {
}

/**
* Takes an optional list of tests to look for, otherwise
* it will look for all tests from the directory
* Takes a file name of a file that contains a list of test
* to either run or exclude and returns a list of Integration Tests to run
*/
public async fromCliArgs(tests?: string[]): Promise<IntegTestConfig[]> {
let allTests = await this.discover();
const all = allTests.map(x => x.fileName);
let foundAll = true;
public async fromFile(fileName: string): Promise<IntegTestConfig[]> {
const file: IntegrationTestFileConfig = JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' }));
const foundTests = await this.discover();

const allTests = this.filterTests(foundTests, file.tests, file.exclude);

if (tests && tests.length > 0) {
// Pare down found tests to filter
allTests = allTests.filter(t => {
const parts = path.parse(t.fileName);
return (tests.includes(t.fileName) || tests.includes(parts.base));
});
return allTests;
}

/**
* If the user provides a list of tests, these can either be a list of tests to include or a list of tests to exclude.
*
* - If it is a list of tests to include then we discover all available tests and check whether they have provided valid tests.
* If they have provided a test name that we don't find, then we write out that error message.
* - If it is a list of tests to exclude, then we discover all available tests and filter out the tests that were provided by the user.
*/
private filterTests(discoveredTests: IntegTestConfig[], requestedTests?: string[], exclude?: boolean): IntegTestConfig[] {
if (!requestedTests || requestedTests.length === 0) {
return discoveredTests;
}
const all = discoveredTests.map(x => x.fileName);
let foundAll = true;
// Pare down found tests to filter
const allTests = discoveredTests.filter(t => {
const parts = path.parse(t.fileName);
if (exclude) {
return (!requestedTests.includes(t.fileName) && !requestedTests.includes(parts.base));
}
return (requestedTests.includes(t.fileName) || requestedTests.includes(parts.base));
});

if (!exclude) {
const selectedNames = allTests.map(t => t.fileName);
for (const unmatched of tests.filter(t => !selectedNames.includes(t))) {
for (const unmatched of requestedTests.filter(t => !selectedNames.includes(t))) {
process.stderr.write(`No such integ test: ${unmatched}\n`);
foundAll = false;
}
}

if (!foundAll) {
process.stderr.write(`Available tests: ${all.join(' ')}\n`);
return [];
Expand All @@ -47,6 +89,18 @@ export class IntegrationTests {
return allTests;
}

/**
* Takes an optional list of tests to look for, otherwise
* it will look for all tests from the directory
*/
public async fromCliArgs(tests?: string[], exclude?: boolean): Promise<IntegTestConfig[]> {
const discoveredTests = await this.discover();

const allTests = this.filterTests(discoveredTests, tests, exclude);

return allTests;
}

private async discover(): Promise<IntegTestConfig[]> {
const files = await this.readTree();
const integs = files.filter(fileName => path.basename(fileName).startsWith('integ.') && path.basename(fileName).endsWith('.js'));
Expand Down
16 changes: 16 additions & 0 deletions packages/@aws-cdk/integ-runner/lib/runner/runners.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ export interface IntegRunnerOptions {
*/
readonly fileName: string,

/**
* The AWS profile to use when invoking the CDK CLI
*
* @default - no profile is passed, the default profile is used
*/
readonly profile?: string;

/**
* Additional environment variables that will be available
* to the CDK CLI
Expand Down Expand Up @@ -120,6 +127,8 @@ export abstract class IntegRunner {
*/
protected readonly cdkOutDir: string;

protected readonly profile?: string;

constructor(options: IntegRunnerOptions) {
const parsed = path.parse(options.fileName);
this.directory = parsed.dir;
Expand All @@ -146,6 +155,7 @@ export abstract class IntegRunner {
});
this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${testName}`;
this.cdkApp = `node ${parsed.base}`;
this.profile = options.profile;
if (this.hasSnapshot()) {
this.loadManifest();
}
Expand Down Expand Up @@ -291,6 +301,7 @@ export abstract class IntegRunner {
...this.defaultArgs,
all: true,
app: this.cdkApp,
profile: this.profile,
output: this.cdkOutDir,
})).split('\n');
if (stacks.length !== 1) {
Expand All @@ -299,6 +310,9 @@ export abstract class IntegRunner {
` ${CDK_INTEG_STACK_PRAGMA} STACK ...\n\n` +
` Available stacks: ${stacks.join(' ')} (wildcards are also supported)\n`);
}
if (stacks.length === 1 && stacks[0] === '') {
throw new Error(`No stack found for test ${this.testName}`);
}
tests.stacks.push(...stacks);
}

Expand Down Expand Up @@ -433,6 +447,7 @@ export class IntegTestRunner extends IntegRunner {
if (!options.dryRun) {
this.cdk.deploy({
...this.defaultArgs,
profile: this.profile,
stacks: options.testCase.stacks,
requireApproval: RequireApproval.NEVER,
output: this.cdkOutDir,
Expand Down Expand Up @@ -460,6 +475,7 @@ export class IntegTestRunner extends IntegRunner {
if (clean) {
this.cdk.destroy({
...this.defaultArgs,
profile: this.profile,
stacks: options.testCase.stacks,
force: true,
app: this.cdkApp,
Expand Down
64 changes: 58 additions & 6 deletions packages/@aws-cdk/integ-runner/lib/workers/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,51 @@ import * as chalk from 'chalk';
import * as logger from '../logger';
import { IntegTestConfig } from '../runner/integ-tests';

/**
* Represents integration tests metrics for a given worker
*/
export interface IntegRunnerMetrics {
/**
* The region the test was run in
*/
readonly region: string;

/**
* The total duration of the worker.
* This will be the sum of all individual test durations
*/
readonly duration: number;

/**
* Contains the duration of individual tests that the
* worker executed.
*
* Map of testName to duration.
*/
readonly tests: { [testName: string]: number };

/**
* The profile that was used to run the test
*
* @default - default profile
*/
readonly profile?: string;
}

/**
* Integration test results
*/
export interface IntegBatchResponse {
failedTests: IntegTestConfig[];
/**
* List of failed tests
*/
readonly failedTests: IntegTestConfig[];

/**
* List of Integration test metrics. Each entry in the
* list represents metrics from a single worker (account + region).
*/
readonly metrics: IntegRunnerMetrics[];
}

/**
Expand Down Expand Up @@ -91,6 +131,11 @@ export interface Diagnostic {
*/
readonly message: string;

/**
* The time it took to run the test
*/
readonly duration?: number;

/**
* The reason for the diagnostic
*/
Expand All @@ -111,18 +156,25 @@ export function printSummary(total: number, failed: number): void {
export function printResults(diagnostic: Diagnostic): void {
switch (diagnostic.reason) {
case DiagnosticReason.SNAPSHOT_SUCCESS:
logger.success(' %s No Change!', diagnostic.testName);
logger.success(' %s No Change! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
break;
case DiagnosticReason.TEST_SUCCESS:
logger.success(' %s Test Succeeded!', diagnostic.testName);
logger.success(' %s Test Succeeded! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
break;
case DiagnosticReason.NO_SNAPSHOT:
logger.error(' %s - No Snapshot!', diagnostic.testName);
logger.error(' %s - No Snapshot! %s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`));
break;
case DiagnosticReason.SNAPSHOT_FAILED:
logger.error(' %s - Snapshot changed!\n%s', diagnostic.testName, diagnostic.message);
logger.error(' %s - Snapshot changed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
break;
case DiagnosticReason.TEST_FAILED:
logger.error(' %s - Failed!\n%s', diagnostic.testName, diagnostic.message);
logger.error(' %s - Failed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
}
}

/**
* Flatten a list of lists into a list of elements
*/
export function flatten<T>(xs: T[][]): T[] {
return Array.prototype.concat.apply([], xs);
}
Loading

0 comments on commit 697fdbe

Please sign in to comment.