Skip to content

Commit 498495a

Browse files
authored
chore(cli-integ): cleanup unnecessary AwsClients creation (#33051)
Some fixture cleanup: - `withCdkMigrateApp` was needlessly creating its own instance of `AwsClients` instead of reusing the one created by the `withAws` fixture. - `withMonolithicCfnIncludeCdkApp` is no longer necessary because we now have `withSpecificFixture` which can use a non default app. This will make it easier to introduce the allocation service logic as there is now only a single place that creates `AwsClients` ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7452462 commit 498495a

File tree

3 files changed

+9
-55
lines changed

3 files changed

+9
-55
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ interface ClientConfig {
2929
}
3030

3131
export class AwsClients {
32-
public static async default(output: NodeJS.WritableStream) {
33-
const region = process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION ?? 'us-east-1';
34-
return AwsClients.forRegion(region, output);
35-
}
3632

3733
public static async forRegion(region: string, output: NodeJS.WritableStream) {
3834
return new AwsClients(region, output);

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

Lines changed: 7 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -99,21 +99,23 @@ export function withCdkApp(
9999
return withSpecificCdkApp('app', block);
100100
}
101101

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

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

110-
const awsClients = await AwsClients.default(context.output);
111113
fs.mkdirSync(integTestDir);
112114
const fixture = new TestFixture(
113115
integTestDir,
114116
stackName,
115117
context.output,
116-
awsClients,
118+
context.aws,
117119
context.randomString,
118120
);
119121

@@ -125,7 +127,7 @@ export function withCdkMigrateApp<A extends TestContext>(language: string, block
125127
path.join(integTestDir, stackName),
126128
stackName,
127129
context.output,
128-
awsClients,
130+
context.aws,
129131
context.randomString,
130132
);
131133

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

148-
export function withMonolithicCfnIncludeCdkApp<A extends TestContext>(block: (context: TestFixture) => Promise<void>) {
149-
return async (context: A) => {
150-
const uberPackage = process.env.UBERPACKAGE;
151-
if (!uberPackage) {
152-
throw new Error('The UBERPACKAGE environment variable is required for running this test!');
153-
}
154-
155-
const randy = context.randomString;
156-
const stackNamePrefix = `cdk-uber-cfn-include-${randy}`;
157-
const integTestDir = path.join(os.tmpdir(), `cdk-uber-cfn-include-${randy}`);
158-
159-
context.output.write(` Stack prefix: ${stackNamePrefix}\n`);
160-
context.output.write(` Test directory: ${integTestDir}\n`);
161-
162-
const awsClients = await AwsClients.default(context.output);
163-
await cloneDirectory(path.join(RESOURCES_DIR, 'cdk-apps', 'cfn-include-app'), integTestDir, context.output);
164-
const fixture = new TestFixture(
165-
integTestDir,
166-
stackNamePrefix,
167-
context.output,
168-
awsClients,
169-
context.randomString,
170-
);
171-
172-
let success = true;
173-
try {
174-
await installNpmPackages(fixture, {
175-
[uberPackage]: fixture.packages.requestedFrameworkVersion(),
176-
});
177-
178-
await block(fixture);
179-
} catch (e) {
180-
success = false;
181-
throw e;
182-
} finally {
183-
if (process.env.INTEG_NO_CLEAN) {
184-
context.log(`Left test directory in '${integTestDir}' ($INTEG_NO_CLEAN)`);
185-
} else {
186-
await fixture.dispose(success);
187-
}
188-
}
189-
};
190-
}
191-
192150
/**
193151
* Default test fixture for most (all?) integ tests
194152
*

packages/@aws-cdk-testing/cli-integ/tests/uberpackage/uberpackage.integtest.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { integTest, withMonolithicCfnIncludeCdkApp } from '../../lib';
1+
import { integTest, withSpecificFixture } from '../../lib';
22

33
jest.setTimeout(600_000);
44

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

99
await fixture.cdkSynth();

0 commit comments

Comments
 (0)