Skip to content

Commit c4ad3ac

Browse files
corymhallrix0rrr
andauthored
chore(cli-integ): try to fix some integ test failures (#24091)
While investigating cli test failures I found some things that need to be cleaned up. 1. Ensure that the bootstrap tests do not also run `ensureBootstrapped` since the test itself is responsible for bootstrapping. 2. Ensure that the correct context is passed to each test --------- Co-authored-by: Rico Hermans <[email protected]>
1 parent 68f1ff8 commit c4ad3ac

File tree

7 files changed

+90
-32
lines changed

7 files changed

+90
-32
lines changed

packages/@aws-cdk-testing/cli-integ/bin/run-suite.ts

+6
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ async function main() {
2222
type: 'string',
2323
requiresArg: true,
2424
})
25+
.option('test-file', {
26+
description: 'The specific test file to run',
27+
type: 'string',
28+
requiresArg: true,
29+
})
2530
.option('use-source', {
2631
descripton: 'Use TypeScript packages from the given source repository (or "auto")',
2732
alias: 's',
@@ -121,6 +126,7 @@ async function main() {
121126
...args.test ? ['-t', args.test] : [],
122127
...args.verbose ? ['--verbose'] : [],
123128
...passWithNoTests ? ['--passWithNoTests'] : [],
129+
...args['test-file'] ? [args['test-file']] : [],
124130
], path.resolve(__dirname, '..', 'resources', 'integ.jest.config.js'));
125131

126132
} finally {

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export function integTest(
1717
name: string,
1818
callback: (context: TestContext) => Promise<void>,
1919
timeoutMillis?: number,
20-
) {
20+
): void {
2121

2222
// Integ tests can run concurrently, and are responsible for blocking
2323
// themselves if they cannot. Because `test.concurrent` executes the test
@@ -62,4 +62,4 @@ function shouldSkip(testName: string) {
6262
export function randomString() {
6363
// Crazy
6464
return Math.random().toString(36).replace(/[^a-z0-9]+/g, '');
65-
}
65+
}

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { AwsClients } from './aws';
22
import { TestContext } from './integ-test';
33
import { ResourcePool } from './resource-pool';
4+
import { DisableBootstrapContext } from './with-cdk-app';
45

56
export type AwsContext = { readonly aws: AwsClients };
67

@@ -9,12 +10,15 @@ export type AwsContext = { readonly aws: AwsClients };
910
*
1011
* Allocate the next region from the REGION pool and dispose it afterwards.
1112
*/
12-
export function withAws<A extends TestContext>(block: (context: A & AwsContext) => Promise<void>) {
13-
return (context: A) => regionPool().using(async (region) => {
13+
export function withAws(
14+
block: (context: TestContext & AwsContext & DisableBootstrapContext) => Promise<void>,
15+
disableBootstrap: boolean = false,
16+
): (context: TestContext) => Promise<void> {
17+
return (context: TestContext) => regionPool().using(async (region) => {
1418
const aws = await AwsClients.forRegion(region, context.output);
1519
await sanityCheck(aws);
1620

17-
return block({ ...context, aws });
21+
return block({ ...context, disableBootstrap, aws });
1822
});
1923
}
2024

@@ -60,4 +64,4 @@ async function sanityCheck(aws: AwsClients) {
6064
throw new Error('AWS credentials probably not configured, see previous error');
6165
}
6266
}
63-
let sanityChecked: boolean | undefined;
67+
let sanityChecked: boolean | undefined;

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

+31-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ import { AwsContext, withAws } from './with-aws';
1818
* For backwards compatibility with existing tests (so we don't have to change
1919
* too much) the inner block is expected to take a `TestFixture` object.
2020
*/
21-
export function withCdkApp<A extends TestContext & AwsContext>(block: (context: TestFixture) => Promise<void>) {
22-
return async (context: A) => {
21+
export function withCdkApp(
22+
block: (context: TestFixture) => Promise<void>,
23+
): (context: TestContext & AwsContext & DisableBootstrapContext) => Promise<void> {
24+
return async (context: TestContext & AwsContext & DisableBootstrapContext) => {
2325
const randy = context.randomString;
2426
const stackNamePrefix = `cdktest-${randy}`;
2527
const integTestDir = path.join(os.tmpdir(), `cdk-integ-${randy}`);
@@ -61,7 +63,9 @@ export function withCdkApp<A extends TestContext & AwsContext>(block: (context:
6163
});
6264
}
6365

64-
await ensureBootstrapped(fixture);
66+
if (!context.disableBootstrap) {
67+
await ensureBootstrapped(fixture);
68+
}
6569

6670
await block(fixture);
6771
} catch (e) {
@@ -131,8 +135,28 @@ export function withMonolithicCfnIncludeCdkApp<A extends TestContext>(block: (co
131135
* test declaration but centralizing it is going to make it convenient to modify in the future.
132136
*/
133137
export function withDefaultFixture(block: (context: TestFixture) => Promise<void>) {
134-
return withAws<TestContext>(withCdkApp(block));
135-
// ^~~~~~ this is disappointing TypeScript! Feels like you should have been able to derive this.
138+
return withAws(withCdkApp(block));
139+
}
140+
141+
export interface DisableBootstrapContext {
142+
/**
143+
* Whether to disable creating the default bootstrap
144+
* stack prior to running the test
145+
*
146+
* This should be set to true when running tests that
147+
* explicitly create a bootstrap stack
148+
*
149+
* @default false
150+
*/
151+
readonly disableBootstrap?: boolean;
152+
}
153+
154+
/**
155+
* To be used in place of `withDefaultFixture` when the test
156+
* should not create the default bootstrap stack
157+
*/
158+
export function withoutBootstrap(block: (context: TestFixture) => Promise<void>) {
159+
return withAws(withCdkApp(block), true);
136160
}
137161

138162
export interface CdkCliOptions extends ShellOptions {
@@ -246,6 +270,8 @@ export class TestFixture extends ShellHelper {
246270
return this.cdk(['deploy',
247271
...(neverRequireApproval ? ['--require-approval=never'] : []), // Default to no approval in an unattended test
248272
...(options.options ?? []),
273+
// use events because bar renders bad in tests
274+
'--progress', 'events',
249275
...this.fullStackName(stackNames)], options);
250276
}
251277

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ function errorCausedByGoPkg(error: string) {
113113
* SAM Integration test fixture for CDK - SAM integration test cases
114114
*/
115115
export function withSamIntegrationFixture(block: (context: SamIntegrationTestFixture) => Promise<void>) {
116-
return withAws<TestContext>(withSamIntegrationCdkApp(block));
116+
return withAws(withSamIntegrationCdkApp(block));
117117
}
118118

119119
export class SamIntegrationTestFixture extends TestFixture {
@@ -263,4 +263,4 @@ function killSubProcess(child: child_process.ChildProcess, command: string) {
263263
child.kill('SIGINT');
264264
}
265265

266-
}
266+
}

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js

+15-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
1414
} else {
1515
var cdk = require('aws-cdk-lib');
1616
var {
17+
DefaultStackSynthesizer,
18+
LegacyStackSynthesizer,
1719
aws_ec2: ec2,
1820
aws_s3: s3,
1921
aws_ssm: ssm,
@@ -216,7 +218,19 @@ class MissingSSMParameterStack extends cdk.Stack {
216218

217219
class LambdaStack extends cdk.Stack {
218220
constructor(parent, id, props) {
219-
super(parent, id, props);
221+
// sometimes we need to specify the custom bootstrap bucket to use
222+
// see the 'upgrade legacy bootstrap stack' test
223+
const synthesizer = parent.node.tryGetContext('legacySynth') === 'true' ?
224+
new LegacyStackSynthesizer({
225+
fileAssetsBucketName: parent.node.tryGetContext('bootstrapBucket'),
226+
})
227+
: new DefaultStackSynthesizer({
228+
fileAssetsBucketName: parent.node.tryGetContext('bootstrapBucket'),
229+
})
230+
super(parent, id, {
231+
...props,
232+
synthesizer: synthesizer,
233+
});
220234

221235
const fn = new lambda.Function(this, 'my-function', {
222236
code: lambda.Code.asset(path.join(__dirname, 'lambda')),

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

+26-18
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import * as fs from 'fs';
22
import * as path from 'path';
3-
import { integTest, randomString, withDefaultFixture } from '../../lib';
3+
import { integTest, randomString, withoutBootstrap } from '../../lib';
44

55
jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
66

7-
integTest('can bootstrap without execution', withDefaultFixture(async (fixture) => {
7+
integTest('can bootstrap without execution', withoutBootstrap(async (fixture) => {
88
const bootstrapStackName = fixture.bootstrapStackName;
99

1010
await fixture.cdkBootstrapLegacy({
@@ -19,7 +19,7 @@ integTest('can bootstrap without execution', withDefaultFixture(async (fixture)
1919
expect(resp.Stacks?.[0].StackStatus).toEqual('REVIEW_IN_PROGRESS');
2020
}));
2121

22-
integTest('upgrade legacy bootstrap stack to new bootstrap stack while in use', withDefaultFixture(async (fixture) => {
22+
integTest('upgrade legacy bootstrap stack to new bootstrap stack while in use', withoutBootstrap(async (fixture) => {
2323
const bootstrapStackName = fixture.bootstrapStackName;
2424

2525
const legacyBootstrapBucketName = `aws-cdk-bootstrap-integ-test-legacy-bckt-${randomString()}`;
@@ -35,7 +35,12 @@ integTest('upgrade legacy bootstrap stack to new bootstrap stack while in use',
3535

3636
// Deploy stack that uses file assets
3737
await fixture.cdkDeploy('lambda', {
38-
options: ['--toolkit-stack-name', bootstrapStackName],
38+
options: [
39+
'--context', `bootstrapBucket=${legacyBootstrapBucketName}`,
40+
'--context', 'legacySynth=true',
41+
'--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`,
42+
'--toolkit-stack-name', bootstrapStackName,
43+
],
3944
});
4045

4146
// Upgrade bootstrap stack to "new" style
@@ -49,13 +54,15 @@ integTest('upgrade legacy bootstrap stack to new bootstrap stack while in use',
4954
// --force to bypass the check which says that the template hasn't changed.
5055
await fixture.cdkDeploy('lambda', {
5156
options: [
57+
'--context', `bootstrapBucket=${newBootstrapBucketName}`,
58+
'--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`,
5259
'--toolkit-stack-name', bootstrapStackName,
5360
'--force',
5461
],
5562
});
5663
}));
5764

58-
integTest('can and deploy if omitting execution policies', withDefaultFixture(async (fixture) => {
65+
integTest('can and deploy if omitting execution policies', withoutBootstrap(async (fixture) => {
5966
const bootstrapStackName = fixture.bootstrapStackName;
6067

6168
await fixture.cdkBootstrapModern({
@@ -72,7 +79,7 @@ integTest('can and deploy if omitting execution policies', withDefaultFixture(as
7279
});
7380
}));
7481

75-
integTest('deploy new style synthesis to new style bootstrap', withDefaultFixture(async (fixture) => {
82+
integTest('deploy new style synthesis to new style bootstrap', withoutBootstrap(async (fixture) => {
7683
const bootstrapStackName = fixture.bootstrapStackName;
7784

7885
await fixture.cdkBootstrapModern({
@@ -90,7 +97,7 @@ integTest('deploy new style synthesis to new style bootstrap', withDefaultFixtur
9097
});
9198
}));
9299

93-
integTest('deploy new style synthesis to new style bootstrap (with docker image)', withDefaultFixture(async (fixture) => {
100+
integTest('deploy new style synthesis to new style bootstrap (with docker image)', withoutBootstrap(async (fixture) => {
94101
const bootstrapStackName = fixture.bootstrapStackName;
95102

96103
await fixture.cdkBootstrapModern({
@@ -108,7 +115,7 @@ integTest('deploy new style synthesis to new style bootstrap (with docker image)
108115
});
109116
}));
110117

111-
integTest('deploy old style synthesis to new style bootstrap', withDefaultFixture(async (fixture) => {
118+
integTest('deploy old style synthesis to new style bootstrap', withoutBootstrap(async (fixture) => {
112119
const bootstrapStackName = fixture.bootstrapStackName;
113120

114121
await fixture.cdkBootstrapModern({
@@ -119,12 +126,13 @@ integTest('deploy old style synthesis to new style bootstrap', withDefaultFixtur
119126
// Deploy stack that uses file assets
120127
await fixture.cdkDeploy('lambda', {
121128
options: [
129+
'--context', `@aws-cdk/core:bootstrapQualifier=${fixture.qualifier}`,
122130
'--toolkit-stack-name', bootstrapStackName,
123131
],
124132
});
125133
}));
126134

127-
integTest('can create a legacy bootstrap stack with --public-access-block-configuration=false', withDefaultFixture(async (fixture) => {
135+
integTest('can create a legacy bootstrap stack with --public-access-block-configuration=false', withoutBootstrap(async (fixture) => {
128136
const bootstrapStackName = fixture.bootstrapStackName;
129137

130138
await fixture.cdkBootstrapLegacy({
@@ -140,7 +148,7 @@ integTest('can create a legacy bootstrap stack with --public-access-block-config
140148
]);
141149
}));
142150

143-
integTest('can create multiple legacy bootstrap stacks', withDefaultFixture(async (fixture) => {
151+
integTest('can create multiple legacy bootstrap stacks', withoutBootstrap(async (fixture) => {
144152
const bootstrapStackName1 = `${fixture.bootstrapStackName}-1`;
145153
const bootstrapStackName2 = `${fixture.bootstrapStackName}-2`;
146154

@@ -162,7 +170,7 @@ integTest('can create multiple legacy bootstrap stacks', withDefaultFixture(asyn
162170
]);
163171
}));
164172

165-
integTest('can dump the template, modify and use it to deploy a custom bootstrap stack', withDefaultFixture(async (fixture) => {
173+
integTest('can dump the template, modify and use it to deploy a custom bootstrap stack', withoutBootstrap(async (fixture) => {
166174
let template = await fixture.cdkBootstrapModern({
167175
// toolkitStackName doesn't matter for this particular invocation
168176
toolkitStackName: fixture.bootstrapStackName,
@@ -188,7 +196,7 @@ integTest('can dump the template, modify and use it to deploy a custom bootstrap
188196
});
189197
}));
190198

191-
integTest('can use the default permissions boundary to bootstrap', withDefaultFixture(async (fixture) => {
199+
integTest('can use the default permissions boundary to bootstrap', withoutBootstrap(async (fixture) => {
192200
let template = await fixture.cdkBootstrapModern({
193201
// toolkitStackName doesn't matter for this particular invocation
194202
toolkitStackName: fixture.bootstrapStackName,
@@ -199,7 +207,7 @@ integTest('can use the default permissions boundary to bootstrap', withDefaultFi
199207
expect(template).toContain('PermissionsBoundary');
200208
}));
201209

202-
integTest('can use the custom permissions boundary to bootstrap', withDefaultFixture(async (fixture) => {
210+
integTest('can use the custom permissions boundary to bootstrap', withoutBootstrap(async (fixture) => {
203211
let template = await fixture.cdkBootstrapModern({
204212
// toolkitStackName doesn't matter for this particular invocation
205213
toolkitStackName: fixture.bootstrapStackName,
@@ -210,7 +218,7 @@ integTest('can use the custom permissions boundary to bootstrap', withDefaultFix
210218
expect(template).toContain('permission-boundary-name');
211219
}));
212220

213-
integTest('switch on termination protection, switch is left alone on re-bootstrap', withDefaultFixture(async (fixture) => {
221+
integTest('switch on termination protection, switch is left alone on re-bootstrap', withoutBootstrap(async (fixture) => {
214222
const bootstrapStackName = fixture.bootstrapStackName;
215223

216224
await fixture.cdkBootstrapModern({
@@ -229,7 +237,7 @@ integTest('switch on termination protection, switch is left alone on re-bootstra
229237
expect(response.Stacks?.[0].EnableTerminationProtection).toEqual(true);
230238
}));
231239

232-
integTest('add tags, left alone on re-bootstrap', withDefaultFixture(async (fixture) => {
240+
integTest('add tags, left alone on re-bootstrap', withoutBootstrap(async (fixture) => {
233241
const bootstrapStackName = fixture.bootstrapStackName;
234242

235243
await fixture.cdkBootstrapModern({
@@ -250,7 +258,7 @@ integTest('add tags, left alone on re-bootstrap', withDefaultFixture(async (fixt
250258
]);
251259
}));
252260

253-
integTest('can add tags then update tags during re-bootstrap', withDefaultFixture(async (fixture) => {
261+
integTest('can add tags then update tags during re-bootstrap', withoutBootstrap(async (fixture) => {
254262
const bootstrapStackName = fixture.bootstrapStackName;
255263

256264
await fixture.cdkBootstrapModern({
@@ -273,7 +281,7 @@ integTest('can add tags then update tags during re-bootstrap', withDefaultFixtur
273281
]);
274282
}));
275283

276-
integTest('can deploy modern-synthesized stack even if bootstrap stack name is unknown', withDefaultFixture(async (fixture) => {
284+
integTest('can deploy modern-synthesized stack even if bootstrap stack name is unknown', withoutBootstrap(async (fixture) => {
277285
const bootstrapStackName = fixture.bootstrapStackName;
278286

279287
await fixture.cdkBootstrapModern({
@@ -293,7 +301,7 @@ integTest('can deploy modern-synthesized stack even if bootstrap stack name is u
293301
});
294302
}));
295303

296-
integTest('create ECR with tag IMMUTABILITY to set on', withDefaultFixture(async (fixture) => {
304+
integTest('create ECR with tag IMMUTABILITY to set on', withoutBootstrap(async (fixture) => {
297305
const bootstrapStackName = fixture.bootstrapStackName;
298306

299307
await fixture.cdkBootstrapModern({

0 commit comments

Comments
 (0)