Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(integ-runner): support config file #22937

Merged
merged 5 commits into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions packages/@aws-cdk/integ-runner/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ If you are providing a list of tests to execute, either as CLI arguments or from
For example, if there is a test `aws-iam/test/integ.policy.js` and the current working directory is `aws-iam` you would provide `integ.policy.js`

```bash
yarn integ integ.policy.js
integ-runner integ.policy.js
```

### Common Workflow
Expand Down Expand Up @@ -121,7 +121,7 @@ Snapshot Results:

Tests: 1 failed, 9 total
Error: Some snapshot tests failed!
To re-run failed tests run: yarn integ-runner --update-on-failed
To re-run failed tests run: integ-runner --update-on-failed
at main (packages/@aws-cdk/integ-runner/lib/cli.js:90:15)
error Command failed with exit code 1.
```
Expand Down Expand Up @@ -197,7 +197,7 @@ If you are adding a new test which creates a new snapshot then you should run th
For example, if you are working on a new test `integ.new-test.js` then you would run:

```bash
yarn integ --update-on-failed --disable-update-workflow integ.new-test.js
integ-runner --update-on-failed --disable-update-workflow integ.new-test.js
```

This is because for a new test we do not need to test the update workflow (there is nothing to update).
Expand All @@ -210,3 +210,25 @@ See [@aws-cdk/cloud-assembly-schema/lib/integ-tests/schema.ts](../cloud-assembly

See the `@aws-cdk/integ-tests` module for information on how to define
integration tests for the runner to exercise.

### Config file

All options can be configured via the `integ.config.json` configuration file in the current working directory.

```json
{
"maxWorkers": 10,
"parallelRegions": [
"eu-west-1",
"ap-southeast-2"
]
}
```

Available options can be listed by running the following command:

```sh
integ-runner --help
```

To use a different config file, provide the `--config` command-line option.
135 changes: 83 additions & 52 deletions packages/@aws-cdk/integ-runner/lib/cli.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
// Exercise all integ stacks and if they deploy, update the expected synth files
import { promises as fs } from 'fs';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no point in having the test list file being async

import * as fs from 'fs';
import * as path from 'path';
import * as chalk from 'chalk';
import * as workerpool from 'workerpool';
import * as logger from './logger';
import { IntegrationTests, IntegTestInfo, IntegTest } from './runner/integration-tests';
import { IntegrationTests, IntegTestInfo } from './runner/integration-tests';
import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics, IntegTestWorkerConfig, DestructiveChange } from './workers';

// https://github.com/yargs/yargs/issues/1929
// https://github.com/evanw/esbuild/issues/1492
// eslint-disable-next-line @typescript-eslint/no-require-imports
const yargs = require('yargs');


export async function main(args: string[]) {
export function parseCliArgs(args: string[] = []) {
const argv = yargs
.usage('Usage: integ-runner [TEST...]')
.option('config', {
config: true,
configParser: IntegrationTests.configFromFile,
default: 'integ.config.json',
desc: 'Load options from a JSON config file. Options provided as CLI arguments take precedent.',
})
.option('list', { type: 'boolean', default: false, desc: 'List tests instead of running them' })
.option('clean', { type: 'boolean', default: true, desc: 'Skips stack clean up after test is completed (use --no-clean to negate)' })
.option('verbose', { type: 'boolean', default: false, alias: 'v', count: true, desc: 'Verbose logs and metrics on integration tests durations (specify multiple times to increase verbosity)' })
Expand All @@ -35,61 +40,87 @@ export async function main(args: string[]) {
.strict()
.parse(args);

const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), {
maxWorkers: argv['max-workers'],
});

// list of integration tests that will be executed
const testRegex = arrayFromYargs(argv['test-regex']);
const testsToRun: IntegTestWorkerConfig[] = [];
const destructiveChanges: DestructiveChange[] = [];
const testsFromArgs: IntegTest[] = [];
const tests: string[] = argv._;
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;
const app: string | undefined = argv.app;
const maxWorkers: number = argv['max-workers'];
const verbosity: number = argv.verbose;
const verbose: boolean = verbosity >= 1;

let failedSnapshots: IntegTestWorkerConfig[] = [];
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']);
const numTests = testRegions.length * (profiles ?? [1]).length;
if (maxWorkers < numTests) {
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', numTests, maxWorkers);
}

let testsSucceeded = false;
try {
if (argv.list) {
const tests = await new IntegrationTests(argv.directory).fromCliArgs({ testRegex, app });
process.stdout.write(tests.map(t => t.discoveryRelativeFileName).join('\n') + '\n');
return;
}
if (tests.length > 0 && fromFile) {
throw new Error('A list of tests cannot be provided if "--from-file" is provided');
}
const requestedTests = fromFile
? (fs.readFileSync(fromFile, { encoding: 'utf8' })).split('\n').filter(x => x)
: (tests.length > 0 ? tests : undefined); // 'undefined' means no request

return {
tests: requestedTests,
app: argv.app as (string | undefined),
testRegex: arrayFromYargs(argv['test-regex']),
testRegions,
profiles,
runUpdateOnFailed: (argv['update-on-failed'] ?? false) as boolean,
fromFile,
exclude: argv.exclude as boolean,
maxWorkers,
list: argv.list as boolean,
directory: argv.directory as string,
inspectFailures: argv['inspect-failures'] as boolean,
verbosity,
verbose,
clean: argv.clean as boolean,
force: argv.force as boolean,
dryRun: argv['dry-run'] as boolean,
disableUpdateWorkflow: argv['disable-update-workflow'] as boolean,
};
}

if (argv._.length > 0 && fromFile) {
throw new Error('A list of tests cannot be provided if "--from-file" is provided');
}
const requestedTests = fromFile
? (await fs.readFile(fromFile, { encoding: 'utf8' })).split('\n').filter(x => x)
: (argv._.length > 0 ? argv._ : undefined); // 'undefined' means no request

testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs({
app,
testRegex,
tests: requestedTests,
exclude,
})));
export async function main(args: string[]) {
const options = parseCliArgs(args);

const testsFromArgs = await new IntegrationTests(path.resolve(options.directory)).fromCliArgs({
app: options.app,
testRegex: options.testRegex,
tests: options.tests,
exclude: options.exclude,
});

// List only prints the discoverd tests
if (options.list) {
process.stdout.write(testsFromArgs.map(t => t.discoveryRelativeFileName).join('\n') + '\n');
return;
}

const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), {
maxWorkers: options.maxWorkers,
});

const testsToRun: IntegTestWorkerConfig[] = [];
const destructiveChanges: DestructiveChange[] = [];
let failedSnapshots: IntegTestWorkerConfig[] = [];
let testsSucceeded = false;

try {
// always run snapshot tests, but if '--force' is passed then
// run integration tests on all failed tests, not just those that
// failed snapshot tests
failedSnapshots = await runSnapshotTests(pool, testsFromArgs, {
retain: argv['inspect-failures'],
verbose: Boolean(argv.verbose),
retain: options.inspectFailures,
verbose: options.verbose,
});
for (const failure of failedSnapshots) {
destructiveChanges.push(...failure.destructiveChanges ?? []);
}
if (!argv.force) {
if (!options.force) {
testsToRun.push(...failedSnapshots);
} else {
// if any of the test failed snapshot tests, keep those results
Expand All @@ -98,25 +129,25 @@ export async function main(args: string[]) {
}

// run integration tests if `--update-on-failed` OR `--force` is used
if (runUpdateOnFailed || argv.force) {
if (options.runUpdateOnFailed || options.force) {
const { success, metrics } = await runIntegrationTests({
pool,
tests: testsToRun,
regions: testRegions,
profiles,
clean: argv.clean,
dryRun: argv['dry-run'],
verbosity: argv.verbose,
updateWorkflow: !argv['disable-update-workflow'],
regions: options.testRegions,
profiles: options.profiles,
clean: options.clean,
dryRun: options.dryRun,
verbosity: options.verbosity,
updateWorkflow: !options.disableUpdateWorkflow,
});
testsSucceeded = success;


if (argv.clean === false) {
if (options.clean === false) {
logger.warning('Not cleaning up stacks since "--no-clean" was used');
}

if (Boolean(argv.verbose)) {
if (Boolean(options.verbose)) {
printMetrics(metrics);
}

Expand All @@ -134,8 +165,8 @@ export async function main(args: string[]) {
}
if (failedSnapshots.length > 0) {
let message = '';
if (!runUpdateOnFailed) {
message = 'To re-run failed tests run: yarn integ-runner --update-on-failed';
if (!options.runUpdateOnFailed) {
message = 'To re-run failed tests run: integ-runner --update-on-failed';
}
if (!testsSucceeded) {
throw new Error(`Some tests failed!\n${message}`);
Expand Down
32 changes: 13 additions & 19 deletions packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,32 +175,26 @@ export interface IntegrationTestsDiscoveryOptions {
}


/**
* The list of tests to run can be provided in a file
* instead of as command line arguments.
*/
export interface IntegrationTestFileConfig extends IntegrationTestsDiscoveryOptions {
/**
* List of tests to include (or exclude if `exclude=true`)
*/
readonly tests: string[];
}

/**
* Discover integration tests
*/
export class IntegrationTests {
constructor(private readonly directory: string) {
}

/**
* 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
* Return configuration options from a file
*/
public async fromFile(fileName: string): Promise<IntegTest[]> {
const file: IntegrationTestFileConfig = JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' }));
public static configFromFile(fileName?: string): Record<string, any> {
if (!fileName) {
return {};
}

return this.discover(file);
try {
return JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' }));
} catch {
return {};
}
}

constructor(private readonly directory: string) {
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/integ-runner/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export class WorkList<A> {

public done() {
this.remaining.clear();
this.stopTimer();
Copy link
Contributor Author

@mrgrain mrgrain Nov 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug currently where the runner would not finish immediately if no tests are found.
(It was only completing after the 60s timeout)

}

private stopTimer() {
Expand Down
Loading