Skip to content

Commit

Permalink
chore(integ-runner): refactor to support fromFile
Browse files Browse the repository at this point in the history
  • Loading branch information
mrgrain committed Nov 7, 2022
1 parent e91d69a commit fd350bb
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 46 deletions.
12 changes: 8 additions & 4 deletions packages/@aws-cdk/integ-runner/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export async function main(args: string[]) {
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;

let failedSnapshots: IntegTestWorkerConfig[] = [];
if (argv['max-workers'] < testRegions.length * (profiles ?? [1]).length) {
Expand All @@ -59,7 +60,7 @@ export async function main(args: string[]) {
let testsSucceeded = false;
try {
if (argv.list) {
const tests = await new IntegrationTests(argv.directory).fromCliArgs({ testRegex });
const tests = await new IntegrationTests(argv.directory).fromCliArgs({ testRegex, app });
process.stdout.write(tests.map(t => t.discoveryRelativeFileName).join('\n') + '\n');
return;
}
Expand All @@ -71,15 +72,19 @@ export async function main(args: string[]) {
? (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({ testRegex, tests: requestedTests, exclude })));
testsFromArgs.push(...(await new IntegrationTests(path.resolve(argv.directory)).fromCliArgs({
app,
testRegex,
tests: requestedTests,
exclude,
})));

// 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),
appCommand: argv.app,
});
for (const failure of failedSnapshots) {
destructiveChanges.push(...failure.destructiveChanges ?? []);
Expand All @@ -103,7 +108,6 @@ export async function main(args: string[]) {
dryRun: argv['dry-run'],
verbosity: argv.verbose,
updateWorkflow: !argv['disable-update-workflow'],
appCommand: argv.app,
});
testsSucceeded = success;

Expand Down
54 changes: 41 additions & 13 deletions packages/@aws-cdk/integ-runner/lib/runner/integration-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ export interface IntegTestInfo {
* Path is relative to the current working directory.
*/
readonly discoveryRoot: string;

/**
* The CLI command used to run this test.
* If it contains {filePath}, the test file names will be substituted at that place in the command for each run.
*
* @default - test run command will be `node {filePath}`
*/
readonly appCommand?: string;
}

/**
Expand Down Expand Up @@ -79,7 +87,16 @@ export class IntegTest {
*/
public readonly temporaryOutputDir: string;

/**
* The CLI command used to run this test.
* If it contains {filePath}, the test file names will be substituted at that place in the command for each run.
*
* @default - test run command will be `node {filePath}`
*/
readonly appCommand: string;

constructor(public readonly info: IntegTestInfo) {
this.appCommand = info.appCommand ?? 'node {filePath}';
this.absoluteFileName = path.resolve(info.fileName);
this.fileName = path.relative(process.cwd(), info.fileName);

Expand Down Expand Up @@ -147,6 +164,14 @@ export interface IntegrationTestsDiscoveryOptions {
* @default
*/
readonly testRegex?: string[];

/**
* The CLI command used to run this test.
* If it contains {filePath}, the test file names will be substituted at that place in the command for each run.
*
* @default - test run command will be `node {filePath}`
*/
readonly app?: string;
}


Expand Down Expand Up @@ -174,11 +199,8 @@ export class IntegrationTests {
*/
public async fromFile(fileName: string): Promise<IntegTest[]> {
const file: IntegrationTestFileConfig = JSON.parse(fs.readFileSync(fileName, { encoding: 'utf-8' }));
const foundTests = await this.discover(file.testRegex);

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

return allTests;
return this.discover(file);
}

/**
Expand Down Expand Up @@ -222,24 +244,30 @@ export class IntegrationTests {
* @param exclude Whether the 'tests' list is inclusive or exclusive (inclusive by default).
*/
public async fromCliArgs(options: IntegrationTestsDiscoveryOptions = {}): Promise<IntegTest[]> {
const discoveredTests = await this.discover(options.testRegex);

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

return allTests;
return this.discover(options);
}

private async discover(patterns: string[] = ['^integ\\..*\\.js$']): Promise<IntegTest[]> {
private async discover(options: IntegrationTestsDiscoveryOptions): Promise<IntegTest[]> {
const patterns = options.testRegex ?? ['^integ\\..*\\.js$'];

const files = await this.readTree();
const integs = files.filter(fileName => patterns.some((p) => {
const regex = new RegExp(p);
return regex.test(fileName) || regex.test(path.basename(fileName));
}));
return this.request(integs);

return this.request(integs, options);
}

private request(files: string[]): IntegTest[] {
return files.map(fileName => new IntegTest({ discoveryRoot: this.directory, fileName }));
private request(files: string[], options: IntegrationTestsDiscoveryOptions): IntegTest[] {
const discoveredTests = files.map(fileName => new IntegTest({
discoveryRoot: this.directory,
fileName,
appCommand: options.app,
}));


return this.filterTests(discoveredTests, options.tests, options.exclude);
}

private async readTree(): Promise<string[]> {
Expand Down
10 changes: 1 addition & 9 deletions packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ export interface IntegRunnerOptions {
*/
readonly cdk?: ICdk;

/**
* You can specify a custom run command, and it will be applied to all test files.
* If it contains {filePath}, the test file names will be substituted at that place in the command for each run.
*
* @default - test run command will be `node {filePath}`
*/
readonly appCommand?: string;

/**
* Show output from running integration tests
*
Expand Down Expand Up @@ -159,7 +151,7 @@ export abstract class IntegRunner {
});
this.cdkOutDir = options.integOutDir ?? this.test.temporaryOutputDir;

const testRunCommand = options.appCommand ?? 'node {filePath}';
const testRunCommand = this.test.appCommand;
this.cdkApp = testRunCommand.replace('{filePath}', path.relative(this.directory, this.test.fileName));

this.profile = options.profile;
Expand Down
14 changes: 0 additions & 14 deletions packages/@aws-cdk/integ-runner/lib/workers/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,6 @@ export interface SnapshotVerificationOptions {
* @default false
*/
readonly verbose?: boolean;

/**
* The CLI command used to run the test files.
*
* @default - test run command will be `node {filePath}`
*/
readonly appCommand?: string;
}

/**
Expand Down Expand Up @@ -169,13 +162,6 @@ export interface IntegTestOptions {
* @default true
*/
readonly updateWorkflow?: boolean;

/**
* The CLI command used to run the test files.
*
* @default - test run command will be `node {filePath}`
*/
readonly appCommand?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker
env: {
AWS_REGION: request.region,
},
appCommand: request.appCommand,
showOutput: verbosity >= 2,
}, testInfo.destructiveChanges);

Expand Down Expand Up @@ -106,7 +105,7 @@ export function snapshotTestWorker(testInfo: IntegTestInfo, options: SnapshotVer
}, 60_000);

try {
const runner = new IntegSnapshotRunner({ test, appCommand: options.appCommand });
const runner = new IntegSnapshotRunner({ test });
if (!runner.hasSnapshot()) {
workerpool.workerEmit({
reason: DiagnosticReason.NO_SNAPSHOT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ export async function runIntegrationTestsInParallel(
dryRun: options.dryRun,
verbosity: options.verbosity,
updateWorkflow: options.updateWorkflow,
appCommand: options.appCommand,
}], {
on: printResults,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,8 @@ describe('IntegTest runIntegTests', () => {
test: new IntegTest({
fileName: 'test/test-data/xxxxx.test-with-snapshot.js',
discoveryRoot: 'test/test-data',
appCommand: 'node --no-warnings {filePath}',
}),
appCommand: 'node --no-warnings {filePath}',
});
integTest.runIntegTestCase({
testCaseName: 'xxxxx.test-with-snapshot',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { writeFileSync } from 'fs';
import * as mockfs from 'mock-fs';
import { IntegrationTests } from '../../lib/runner/integration-tests';
import { IntegrationTests, IntegrationTestsDiscoveryOptions } from '../../lib/runner/integration-tests';

describe('IntegrationTests', () => {
const tests = new IntegrationTests('test');
Expand Down Expand Up @@ -81,11 +81,19 @@ describe('IntegrationTests', () => {
expect(integTests.length).toEqual(1);
expect(integTests[0].fileName).toEqual(expect.stringMatching(/integ.other-test1.js$/));
});

test('can set app command', async () => {
const otherTestDir = new IntegrationTests('test');
const integTests = await otherTestDir.fromCliArgs({ app: 'node --no-warnings {filePath}' });

expect(integTests.length).toEqual(3);
expect(integTests[0].appCommand).toEqual('node --no-warnings {filePath}');
});
});

describe('from file', () => {
const configFile = 'integ.config.json';
const writeConfig = (settings: any, fileName = configFile) => {
const writeConfig = (settings: IntegrationTestsDiscoveryOptions, fileName = configFile) => {
writeFileSync(fileName, JSON.stringify(settings, null, 2), { encoding: 'utf-8' });
};

Expand Down Expand Up @@ -150,5 +158,13 @@ describe('IntegrationTests', () => {
expect(integTests.length).toEqual(1);
expect(integTests[0].fileName).toEqual(expect.stringMatching(/integ.other-test1.js$/));
});

test('can set app command', async () => {
writeConfig({ app: 'node --no-warnings {filePath}' });
const integTests = await tests.fromFile(configFile);

expect(integTests.length).toEqual(3);
expect(integTests[0].appCommand).toEqual('node --no-warnings {filePath}');
});
});
});

0 comments on commit fd350bb

Please sign in to comment.