Skip to content

Commit cba729c

Browse files
authored
Merge branch 'master' into cognito-idp-oidc
2 parents 5957e37 + cedfde8 commit cba729c

16 files changed

+453
-282
lines changed

packages/@aws-cdk/integ-runner/lib/cli.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
// Exercise all integ stacks and if they deploy, update the expected synth files
2+
import { promises as fs } from 'fs';
23
import * as path from 'path';
34
import * as chalk from 'chalk';
45
import * as workerpool from 'workerpool';
56
import * as logger from './logger';
6-
import { IntegrationTests, IntegTestConfig } from './runner/integration-tests';
7+
import { IntegrationTests, IntegTestInfo, IntegTest } from './runner/integration-tests';
78
import { runSnapshotTests, runIntegrationTests, IntegRunnerMetrics, IntegTestWorkerConfig, DestructiveChange } from './workers';
89

910
// https://github.com/yargs/yargs/issues/1929
@@ -25,8 +26,8 @@ async function main() {
2526
.options('directory', { type: 'string', default: 'test', desc: 'starting directory to discover integration tests. Tests will be discovered recursively from this directory' })
2627
.options('profiles', { type: 'array', desc: 'list of AWS profiles to use. Tests will be run in parallel across each profile+regions', nargs: 1, default: [] })
2728
.options('max-workers', { type: 'number', desc: 'The max number of workerpool workers to use when running integration tests in parallel', default: 16 })
28-
.options('exclude', { type: 'boolean', desc: 'All tests should be run, except for the list of tests provided', default: false })
29-
.options('from-file', { type: 'string', desc: 'Import tests to include or exclude from a file' })
29+
.options('exclude', { type: 'boolean', desc: 'Run all tests in the directory, except the specified TESTs', default: false })
30+
.options('from-file', { type: 'string', desc: 'Read TEST names from a file (one TEST per line)' })
3031
.option('inspect-failures', { type: 'boolean', desc: 'Keep the integ test cloud assembly if a failure occurs for inspection', default: false })
3132
.option('disable-update-workflow', { type: 'boolean', default: false, desc: 'If this is "true" then the stack update workflow will be disabled' })
3233
.strict()
@@ -39,7 +40,7 @@ async function main() {
3940
// list of integration tests that will be executed
4041
const testsToRun: IntegTestWorkerConfig[] = [];
4142
const destructiveChanges: DestructiveChange[] = [];
42-
const testsFromArgs: IntegTestConfig[] = [];
43+
const testsFromArgs: IntegTest[] = [];
4344
const parallelRegions = arrayFromYargs(argv['parallel-regions']);
4445
const testRegions: string[] = parallelRegions ?? ['us-east-1', 'us-east-2', 'us-west-2'];
4546
const profiles = arrayFromYargs(argv.profiles);
@@ -56,19 +57,18 @@ async function main() {
5657
try {
5758
if (argv.list) {
5859
const tests = await new IntegrationTests(argv.directory).fromCliArgs();
59-
process.stdout.write(tests.map(t => t.fileName).join('\n') + '\n');
60+
process.stdout.write(tests.map(t => t.discoveryRelativeFileName).join('\n') + '\n');
6061
return;
6162
}
6263

6364
if (argv._.length > 0 && fromFile) {
6465
throw new Error('A list of tests cannot be provided if "--from-file" is provided');
65-
} else if (argv._.length === 0 && !fromFile) {
66-
testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs()));
67-
} else if (fromFile) {
68-
testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromFile(path.resolve(fromFile))));
69-
} else {
70-
testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs(argv._.map((x: any) => x.toString()), exclude)));
7166
}
67+
const requestedTests = fromFile
68+
? (await fs.readFile(fromFile, { encoding: 'utf8' })).split('\n').filter(x => x)
69+
: (argv._.length > 0 ? argv._ : undefined); // 'undefined' means no request
70+
71+
testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs(requestedTests, exclude)));
7272

7373
// always run snapshot tests, but if '--force' is passed then
7474
// run integration tests on all failed tests, not just those that
@@ -85,7 +85,7 @@ async function main() {
8585
} else {
8686
// if any of the test failed snapshot tests, keep those results
8787
// and merge with the rest of the tests from args
88-
testsToRun.push(...mergeTests(testsFromArgs, failedSnapshots));
88+
testsToRun.push(...mergeTests(testsFromArgs.map(t => t.info), failedSnapshots));
8989
}
9090

9191
// run integration tests if `--update-on-failed` OR `--force` is used
@@ -174,7 +174,7 @@ function arrayFromYargs(xs: string[]): string[] | undefined {
174174
* tests that failed snapshot tests. The failed snapshot tests have additional
175175
* information that we want to keep so this should override any test from args
176176
*/
177-
function mergeTests(testFromArgs: IntegTestConfig[], failedSnapshotTests: IntegTestWorkerConfig[]): IntegTestWorkerConfig[] {
177+
function mergeTests(testFromArgs: IntegTestInfo[], failedSnapshotTests: IntegTestWorkerConfig[]): IntegTestWorkerConfig[] {
178178
const failedTestNames = new Set(failedSnapshotTests.map(test => test.fileName));
179179
const final: IntegTestWorkerConfig[] = failedSnapshotTests;
180180
final.push(...testFromArgs.filter(test => !failedTestNames.has(test.fileName)));

packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ export class IntegTestRunner extends IntegRunner {
7272
* all branches and we then search for one that starts with `HEAD branch: `
7373
*/
7474
private checkoutSnapshot(): void {
75-
const cwd = path.dirname(this.snapshotDir);
75+
const cwd = this.directory;
76+
7677
// https://git-scm.com/docs/git-merge-base
7778
let baseBranch: string | undefined = undefined;
7879
// try to find the base branch that the working branch was created from
@@ -98,17 +99,19 @@ export class IntegTestRunner extends IntegRunner {
9899
// if we found the base branch then get the merge-base (most recent common commit)
99100
// and checkout the snapshot using that commit
100101
if (baseBranch) {
102+
const relativeSnapshotDir = path.relative(this.directory, this.snapshotDir);
103+
101104
try {
102105
const base = exec(['git', 'merge-base', 'HEAD', baseBranch], {
103106
cwd,
104107
});
105-
exec(['git', 'checkout', base, '--', this.relativeSnapshotDir], {
108+
exec(['git', 'checkout', base, '--', relativeSnapshotDir], {
106109
cwd,
107110
});
108111
} catch (e) {
109112
logger.warning('%s\n%s',
110113
`Could not checkout snapshot directory ${this.snapshotDir} using these commands: `,
111-
`git merge-base HEAD ${baseBranch} && git checkout {merge-base} -- ${this.relativeSnapshotDir}`,
114+
`git merge-base HEAD ${baseBranch} && git checkout {merge-base} -- ${relativeSnapshotDir}`,
112115
);
113116
logger.warning('error: %s', e);
114117
}
@@ -129,6 +132,9 @@ export class IntegTestRunner extends IntegRunner {
129132
public runIntegTestCase(options: RunOptions): AssertionResults | undefined {
130133
let assertionResults: AssertionResults | undefined;
131134
const actualTestCase = this.actualTestSuite.testSuite[options.testCaseName];
135+
if (!actualTestCase) {
136+
throw new Error(`Did not find test case name '${options.testCaseName}' in '${Object.keys(this.actualTestSuite.testSuite)}'`);
137+
}
132138
const clean = options.clean ?? true;
133139
const updateWorkflowEnabled = (options.updateWorkflow ?? true)
134140
&& (actualTestCase.stackUpdateWorkflow ?? true);
@@ -151,7 +157,7 @@ export class IntegTestRunner extends IntegRunner {
151157
this.cdk.synthFast({
152158
execCmd: this.cdkApp.split(' '),
153159
env,
154-
output: this.cdkOutDir,
160+
output: path.relative(this.directory, this.cdkOutDir),
155161
});
156162
}
157163
// only create the snapshot if there are no assertion assertion results
@@ -170,7 +176,7 @@ export class IntegTestRunner extends IntegRunner {
170176
all: true,
171177
force: true,
172178
app: this.cdkApp,
173-
output: this.cdkOutDir,
179+
output: path.relative(this.directory, this.cdkOutDir),
174180
...actualTestCase.cdkCommandOptions?.destroy?.args,
175181
context: this.getContext(actualTestCase.cdkCommandOptions?.destroy?.args?.context),
176182
});
@@ -241,7 +247,7 @@ export class IntegTestRunner extends IntegRunner {
241247
stacks: expectedTestCase.stacks,
242248
...expectedTestCase?.cdkCommandOptions?.deploy?.args,
243249
context: this.getContext(expectedTestCase?.cdkCommandOptions?.deploy?.args?.context),
244-
app: this.relativeSnapshotDir,
250+
app: path.relative(this.directory, this.snapshotDir),
245251
lookups: this.expectedTestSuite?.enableLookups,
246252
});
247253
}
@@ -255,9 +261,9 @@ export class IntegTestRunner extends IntegRunner {
255261
...actualTestCase.assertionStack ? [actualTestCase.assertionStack] : [],
256262
],
257263
rollback: false,
258-
output: this.cdkOutDir,
264+
output: path.relative(this.directory, this.cdkOutDir),
259265
...actualTestCase?.cdkCommandOptions?.deploy?.args,
260-
...actualTestCase.assertionStack ? { outputsFile: path.join(this.cdkOutDir, 'assertion-results.json') } : undefined,
266+
...actualTestCase.assertionStack ? { outputsFile: path.relative(this.directory, path.join(this.cdkOutDir, 'assertion-results.json')) } : undefined,
261267
context: this.getContext(actualTestCase?.cdkCommandOptions?.deploy?.args?.context),
262268
app: this.cdkApp,
263269
});
@@ -270,7 +276,7 @@ export class IntegTestRunner extends IntegRunner {
270276

271277
if (actualTestCase.assertionStack) {
272278
return this.processAssertionResults(
273-
path.join(this.directory, this.cdkOutDir, 'assertion-results.json'),
279+
path.join(this.cdkOutDir, 'assertion-results.json'),
274280
actualTestCase.assertionStack,
275281
);
276282
}

packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts

Lines changed: 124 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,118 @@
11
import * as path from 'path';
22
import * as fs from 'fs-extra';
33

4+
const CDK_OUTDIR_PREFIX = 'cdk-integ.out';
5+
46
/**
57
* Represents a single integration test
8+
*
9+
* This type is a data-only structure, so it can trivially be passed to workers.
10+
* Derived attributes are calculated using the `IntegTest` class.
611
*/
7-
export interface IntegTestConfig {
12+
export interface IntegTestInfo {
813
/**
9-
* The name of the file that contains the
10-
* integration tests. This will be in the format
11-
* of integ.{test-name}.js
14+
* Path to the file to run
15+
*
16+
* Path is relative to the current working directory.
1217
*/
1318
readonly fileName: string;
1419

1520
/**
16-
* The base directory where the tests are
17-
* discovered from
21+
* The root directory we discovered this test from
22+
*
23+
* Path is relative to the current working directory.
1824
*/
19-
readonly directory: string;
25+
readonly discoveryRoot: string;
26+
}
27+
28+
/**
29+
* Derived information for IntegTests
30+
*/
31+
export class IntegTest {
32+
/**
33+
* The name of the file to run
34+
*
35+
* Path is relative to the current working directory.
36+
*/
37+
public readonly fileName: string;
38+
39+
/**
40+
* Relative path to the file to run
41+
*
42+
* Relative from the "discovery root".
43+
*/
44+
public readonly discoveryRelativeFileName: string;
45+
46+
/**
47+
* The absolute path to the file
48+
*/
49+
public readonly absoluteFileName: string;
50+
51+
/**
52+
* Directory the test is in
53+
*/
54+
public readonly directory: string;
55+
56+
/**
57+
* Display name for the test
58+
*
59+
* Depends on the discovery directory.
60+
*
61+
* Looks like `integ.mytest` or `package/test/integ.mytest`.
62+
*/
63+
public readonly testName: string;
64+
65+
/**
66+
* Path of the snapshot directory for this test
67+
*/
68+
public readonly snapshotDir: string;
69+
70+
/**
71+
* Path to the temporary output directory for this test
72+
*/
73+
public readonly temporaryOutputDir: string;
74+
75+
constructor(public readonly info: IntegTestInfo) {
76+
this.absoluteFileName = path.resolve(info.fileName);
77+
this.fileName = path.relative(process.cwd(), info.fileName);
78+
79+
const parsed = path.parse(this.fileName);
80+
this.discoveryRelativeFileName = path.relative(info.discoveryRoot, info.fileName);
81+
this.directory = parsed.dir;
82+
83+
// if we are running in a package directory then just use the fileName
84+
// as the testname, but if we are running in a parent directory with
85+
// multiple packages then use the directory/filename as the testname
86+
//
87+
// Looks either like `integ.mytest` or `package/test/integ.mytest`.
88+
const relDiscoveryRoot = path.relative(process.cwd(), info.discoveryRoot);
89+
this.testName = this.directory === path.join(relDiscoveryRoot, 'test') || this.directory === path.join(relDiscoveryRoot)
90+
? parsed.name
91+
: path.join(path.relative(this.info.discoveryRoot, parsed.dir), parsed.name);
92+
93+
const nakedTestName = parsed.name.slice(6); // Leave name without 'integ.' and '.ts'
94+
this.snapshotDir = path.join(this.directory, `${nakedTestName}.integ.snapshot`);
95+
this.temporaryOutputDir = path.join(this.directory, `${CDK_OUTDIR_PREFIX}.${nakedTestName}`);
96+
}
97+
98+
/**
99+
* Whether this test matches the user-given name
100+
*
101+
* We are very lenient here. A name matches if it matches:
102+
*
103+
* - The CWD-relative filename
104+
* - The discovery root-relative filename
105+
* - The suite name
106+
* - The absolute filename
107+
*/
108+
public matches(name: string) {
109+
return [
110+
this.fileName,
111+
this.discoveryRelativeFileName,
112+
this.testName,
113+
this.absoluteFileName,
114+
].includes(name);
115+
}
20116
}
21117

22118
/**
@@ -49,7 +145,7 @@ export class IntegrationTests {
49145
* Takes a file name of a file that contains a list of test
50146
* to either run or exclude and returns a list of Integration Tests to run
51147
*/
52-
public async fromFile(fileName: string): Promise<IntegTestConfig[]> {
148+
public async fromFile(fileName: string): Promise<IntegTest[]> {
53149
const file: IntegrationTestFileConfig = JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' }));
54150
const foundTests = await this.discover();
55151

@@ -65,32 +161,27 @@ export class IntegrationTests {
65161
* If they have provided a test name that we don't find, then we write out that error message.
66162
* - 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.
67163
*/
68-
private filterTests(discoveredTests: IntegTestConfig[], requestedTests?: string[], exclude?: boolean): IntegTestConfig[] {
69-
if (!requestedTests || requestedTests.length === 0) {
164+
private filterTests(discoveredTests: IntegTest[], requestedTests?: string[], exclude?: boolean): IntegTest[] {
165+
if (!requestedTests) {
70166
return discoveredTests;
71167
}
72-
const all = discoveredTests.map(x => {
73-
return path.relative(x.directory, x.fileName);
74-
});
75-
let foundAll = true;
76-
// Pare down found tests to filter
168+
169+
77170
const allTests = discoveredTests.filter(t => {
78-
if (exclude) {
79-
return (!requestedTests.includes(path.relative(t.directory, t.fileName)));
80-
}
81-
return (requestedTests.includes(path.relative(t.directory, t.fileName)));
171+
const matches = requestedTests.some(pattern => t.matches(pattern));
172+
return matches !== !!exclude; // Looks weird but is equal to (matches && !exclude) || (!matches && exclude)
82173
});
83174

175+
// If not excluding, all patterns must have matched at least one test
84176
if (!exclude) {
85-
const selectedNames = allTests.map(t => path.relative(t.directory, t.fileName));
86-
for (const unmatched of requestedTests.filter(t => !selectedNames.includes(t))) {
177+
const unmatchedPatterns = requestedTests.filter(pattern => !discoveredTests.some(t => t.matches(pattern)));
178+
for (const unmatched of unmatchedPatterns) {
87179
process.stderr.write(`No such integ test: ${unmatched}\n`);
88-
foundAll = false;
89180
}
90-
}
91-
if (!foundAll) {
92-
process.stderr.write(`Available tests: ${all.join(' ')}\n`);
93-
return [];
181+
if (unmatchedPatterns.length > 0) {
182+
process.stderr.write(`Available tests: ${discoveredTests.map(t => t.discoveryRelativeFileName).join(' ')}\n`);
183+
return [];
184+
}
94185
}
95186

96187
return allTests;
@@ -99,23 +190,26 @@ export class IntegrationTests {
99190
/**
100191
* Takes an optional list of tests to look for, otherwise
101192
* it will look for all tests from the directory
193+
*
194+
* @param tests Tests to include or exclude, undefined means include all tests.
195+
* @param exclude Whether the 'tests' list is inclusive or exclusive (inclusive by default).
102196
*/
103-
public async fromCliArgs(tests?: string[], exclude?: boolean): Promise<IntegTestConfig[]> {
197+
public async fromCliArgs(tests?: string[], exclude?: boolean): Promise<IntegTest[]> {
104198
const discoveredTests = await this.discover();
105199

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

108202
return allTests;
109203
}
110204

111-
private async discover(): Promise<IntegTestConfig[]> {
205+
private async discover(): Promise<IntegTest[]> {
112206
const files = await this.readTree();
113207
const integs = files.filter(fileName => path.basename(fileName).startsWith('integ.') && path.basename(fileName).endsWith('.js'));
114208
return this.request(integs);
115209
}
116210

117-
private request(files: string[]): IntegTestConfig[] {
118-
return files.map(fileName => { return { directory: this.directory, fileName }; });
211+
private request(files: string[]): IntegTest[] {
212+
return files.map(fileName => new IntegTest({ discoveryRoot: this.directory, fileName }));
119213
}
120214

121215
private async readTree(): Promise<string[]> {

0 commit comments

Comments
 (0)