Skip to content

chore(cli-integ): cleanup unnecessary AwsClients creation #33051

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 22, 2025
Merged

chore(cli-integ): cleanup unnecessary AwsClients creation #33051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ffa95a6
mid work
iliapolo Jan 22, 2025
3b763f6
mid work
iliapolo Jan 22, 2025
a6c0d24
Merge branch 'main' into epolon/fixtures-cleanup
iliapolo Jan 22, 2025
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
4 changes: 0 additions & 4 deletions packages/@aws-cdk-testing/cli-integ/lib/aws.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ interface ClientConfig {
}

export class AwsClients {
public static async default(output: NodeJS.WritableStream) {
Copy link
Contributor Author

@iliapolo iliapolo Jan 22, 2025

Choose a reason for hiding this comment

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

No one is using this now - which is great, we really shouldn't have been calling this function at all because it bypasses the region pool.

const region = process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION ?? 'us-east-1';
return AwsClients.forRegion(region, output);
}

public static async forRegion(region: string, output: NodeJS.WritableStream) {
return new AwsClients(region, output);
Expand Down
56 changes: 7 additions & 49 deletions packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,23 @@ export function withCdkApp(
return withSpecificCdkApp('app', block);
}

export function withCdkMigrateApp<A extends TestContext>(language: string, block: (context: TestFixture) => Promise<void>) {
return async (context: A) => {
export function withCdkMigrateApp(
language: string,
block: (context: TestFixture) => Promise<void>,
): (context: TestContext & AwsContext & DisableBootstrapContext) => Promise<void> {
return async (context: TestContext & AwsContext & DisableBootstrapContext) => {
const stackName = `cdk-migrate-${language}-integ-${context.randomString}`;
const integTestDir = path.join(os.tmpdir(), `cdk-migrate-${language}-integ-${context.randomString}`);

context.output.write(` Stack name: ${stackName}\n`);
context.output.write(` Test directory: ${integTestDir}\n`);

const awsClients = await AwsClients.default(context.output);
fs.mkdirSync(integTestDir);
const fixture = new TestFixture(
integTestDir,
stackName,
context.output,
awsClients,
context.aws,
context.randomString,
);

Expand All @@ -125,7 +127,7 @@ export function withCdkMigrateApp<A extends TestContext>(language: string, block
path.join(integTestDir, stackName),
stackName,
context.output,
awsClients,
context.aws,
context.randomString,
);

Expand All @@ -145,50 +147,6 @@ export function withCdkMigrateApp<A extends TestContext>(language: string, block
};
}

export function withMonolithicCfnIncludeCdkApp<A extends TestContext>(block: (context: TestFixture) => Promise<void>) {
return async (context: A) => {
const uberPackage = process.env.UBERPACKAGE;
if (!uberPackage) {
throw new Error('The UBERPACKAGE environment variable is required for running this test!');
}

const randy = context.randomString;
const stackNamePrefix = `cdk-uber-cfn-include-${randy}`;
const integTestDir = path.join(os.tmpdir(), `cdk-uber-cfn-include-${randy}`);

context.output.write(` Stack prefix: ${stackNamePrefix}\n`);
context.output.write(` Test directory: ${integTestDir}\n`);

const awsClients = await AwsClients.default(context.output);
await cloneDirectory(path.join(RESOURCES_DIR, 'cdk-apps', 'cfn-include-app'), integTestDir, context.output);
const fixture = new TestFixture(
integTestDir,
stackNamePrefix,
context.output,
awsClients,
context.randomString,
);

let success = true;
try {
await installNpmPackages(fixture, {
[uberPackage]: fixture.packages.requestedFrameworkVersion(),
});

await block(fixture);
} catch (e) {
success = false;
throw e;
} finally {
if (process.env.INTEG_NO_CLEAN) {
context.log(`Left test directory in '${integTestDir}' ($INTEG_NO_CLEAN)`);
} else {
await fixture.dispose(success);
}
}
};
}

/**
* Default test fixture for most (all?) integ tests
*
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk-testing/cli-integ/tests/uberpackage/uberpackage.integtest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { integTest, withMonolithicCfnIncludeCdkApp } from '../../lib';
import { integTest, withSpecificFixture } from '../../lib';

jest.setTimeout(600_000);

describe('uberpackage', () => {
integTest('works with cloudformation-include', withMonolithicCfnIncludeCdkApp(async (fixture) => {
integTest('works with cloudformation-include', withSpecificFixture('cfn-include-app', async (fixture) => {
fixture.log('Starting test of cfn-include with monolithic CDK');

await fixture.cdkSynth();
Expand Down
Loading