Skip to content

Commit c8de5be

Browse files
rix0rrrrix0rrr
authored
chore(cli-integ): make it possible to run on GitHub Actions (#33175)
Migrate some changes back from the new testing repo. These changes are necessary to make the tests run on GitHub Actions. If we keep them here, in the future we can do a `cp -R` on the test directory. If not, we'll have to do manual sorting on every copy over, which is annoying and easy to make mistakes in. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 8c13cf2 commit c8de5be

File tree

12 files changed

+111
-47
lines changed

12 files changed

+111
-47
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ export async function sleep(ms: number) {
252252
}
253253

254254
function chainableCredentials(region: string): AwsCredentialIdentityProvider | undefined {
255-
if (process.env.CODEBUILD_BUILD_ARN && process.env.AWS_PROFILE) {
255+
if ((process.env.CODEBUILD_BUILD_ARN || process.env.GITHUB_RUN_ID) && process.env.AWS_PROFILE) {
256256
// in codebuild we must assume the role that the cdk uses
257257
// otherwise credentials will just be picked up by the normal sdk
258258
// heuristics and expire after an hour.

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

+43-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ if (SKIP_TESTS) {
1111
process.stderr.write(`ℹ️ Skipping tests: ${JSON.stringify(SKIP_TESTS)}\n`);
1212
}
1313

14+
// Whether we want to stop after the first failure, for quicker debugging (hopefully).
15+
const FAIL_FAST = process.env.FAIL_FAST === 'true';
16+
17+
// Keep track of whether the suite has failed. If so, we stop running.
18+
let failed = false;
19+
1420
export interface TestContext {
1521
readonly randomString: string;
1622
readonly name: string;
@@ -49,6 +55,10 @@ export function integTest(
4955
const now = Date.now();
5056
process.stderr.write(`[INTEG TEST::${name}] Starting (pid ${process.pid})...\n`);
5157
try {
58+
if (FAIL_FAST && failed) {
59+
throw new Error('FAIL_FAST requested and currently failing. Stopping test early.');
60+
}
61+
5262
return await callback({
5363
output,
5464
randomString: randomString(),
@@ -58,13 +68,41 @@ export function integTest(
5868
},
5969
});
6070
} catch (e: any) {
61-
process.stderr.write(`[INTEG TEST::${name}] Failed: ${e}\n`);
71+
failed = true;
72+
73+
// Print the buffered output, only if the test fails.
6274
output.write(e.message);
6375
output.write(e.stack);
64-
// Print output only if the test fails. Use 'console.log' so the output is buffered by
65-
// jest and prints without a stack trace (if verbose: false).
66-
// eslint-disable-next-line no-console
67-
console.log(output.buffer().toString());
76+
process.stderr.write(`[INTEG TEST::${name}] Failed: ${e}\n`);
77+
78+
const isGitHub = !!process.env.GITHUB_RUN_ID;
79+
80+
if (isGitHub) {
81+
// GitHub Actions compatible output formatting
82+
// https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-an-error-message
83+
let written = process.stderr.write(`::error title=Failed ${name}::${e.message}\n`);
84+
if (!written) {
85+
// Wait for drain
86+
await new Promise((ok) => process.stderr.once('drain', ok));
87+
}
88+
89+
// Print output only if the test fails. Use 'console.log' so the output is buffered by
90+
// jest and prints without a stack trace (if verbose: false).
91+
written = process.stdout.write([
92+
`::group::Failure details: ${name} (click to expand)\n`,
93+
`${output.buffer().toString()}\n`,
94+
'::endgroup::\n',
95+
].join(''));
96+
if (!written) {
97+
// Wait for drain
98+
await new Promise((ok) => process.stdout.once('drain', ok));
99+
}
100+
} else {
101+
// Use 'console.log' so the output is buffered by
102+
// jest and prints without a stack trace (if verbose: false).
103+
// eslint-disable-next-line no-console
104+
console.log(output.buffer().toString());
105+
}
68106
throw e;
69107
} finally {
70108
const duration = Date.now() - now;

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

+16-9
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
2525
writeToOutputs(`💻 ${command.join(' ')}\n`);
2626
const show = options.show ?? 'always';
2727

28-
if (process.env.VERBOSE) {
29-
outputs.add(process.stdout);
30-
}
31-
3228
const env = options.env ?? (options.modEnv ? { ...process.env, ...options.modEnv } : process.env);
3329

3430
const child = child_process.spawn(command[0], command.slice(1), {
@@ -81,7 +77,7 @@ export interface ShellOptions extends child_process.SpawnOptions {
8177
/**
8278
* Properties to add to 'env'
8379
*/
84-
readonly modEnv?: Record<string, string>;
80+
readonly modEnv?: Record<string, string | undefined>;
8581

8682
/**
8783
* Don't fail when exiting with an error
@@ -138,22 +134,33 @@ export class ShellHelper {
138134

139135
/**
140136
* rm -rf reimplementation, don't want to depend on an NPM package for this
137+
*
138+
* Returns `true` if everything got deleted, or `false` if some files could
139+
* not be deleted due to permissions issues.
141140
*/
142-
export function rimraf(fsPath: string) {
141+
export function rimraf(fsPath: string): boolean {
143142
try {
143+
let success = true;
144144
const isDir = fs.lstatSync(fsPath).isDirectory();
145145

146146
if (isDir) {
147147
for (const file of fs.readdirSync(fsPath)) {
148-
rimraf(path.join(fsPath, file));
148+
success &&= rimraf(path.join(fsPath, file));
149149
}
150150
fs.rmdirSync(fsPath);
151151
} else {
152152
fs.unlinkSync(fsPath);
153153
}
154+
return success;
154155
} catch (e: any) {
155-
// We will survive ENOENT
156-
if (e.code !== 'ENOENT') { throw e; }
156+
// Can happen if some files got generated inside a Docker container and are now inadvertently owned by `root`.
157+
// We can't ever clean those up anymore, but since it only happens inside GitHub Actions containers we also don't care too much.
158+
if (e.code === 'EACCES' || e.code === 'ENOTEMPTY') { return false; }
159+
160+
// Already gone
161+
if (e.code === 'ENOENT') { return true; }
162+
163+
throw e;
157164
}
158165
}
159166

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,8 @@ export class TestFixture extends ShellHelper {
513513
AWS_DEFAULT_REGION: this.aws.region,
514514
STACK_NAME_PREFIX: this.stackNamePrefix,
515515
PACKAGE_LAYOUT_VERSION: this.packages.majorVersion(),
516+
// CI must be unset, because we're trying to capture stdout in a bunch of tests
517+
CI: 'false',
516518
...awsCreds,
517519
...options.modEnv,
518520
},
@@ -607,7 +609,10 @@ export class TestFixture extends ShellHelper {
607609
// If the tests completed successfully, happily delete the fixture
608610
// (otherwise leave it for humans to inspect)
609611
if (success) {
610-
rimraf(this.integTestDir);
612+
const cleaned = rimraf(this.integTestDir);
613+
if (!cleaned) {
614+
console.error(`Failed to clean up ${this.integTestDir} due to permissions issues (Docker running as root?)`);
615+
}
611616
}
612617
}
613618

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

+3
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ __EOS__`], {
131131
AWS_DEFAULT_REGION: this.aws.region,
132132
STACK_NAME_PREFIX: this.stackNamePrefix,
133133
PACKAGE_LAYOUT_VERSION: this.packages.majorVersion(),
134+
// Unset CI because we need to distinguish stdout/stderr and this variable
135+
// makes everything go to stdout
136+
CI: 'false',
134137
...options.modEnv,
135138
},
136139
});

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

+24-21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as child_process from 'child_process';
2-
import * as fs from 'fs';
32
import * as os from 'os';
43
import * as path from 'path';
54
import axios from 'axios';
@@ -139,6 +138,7 @@ export class SamIntegrationTestFixture extends TestFixture {
139138
args.push('--port');
140139
args.push(port.toString());
141140

141+
// "Press Ctrl+C to quit" looks to be printed by a Flask server built into SAM CLI.
142142
return this.samShell(['sam', 'local', 'start-api', ...args], 'Press CTRL+C to quit', ()=>{
143143
return new Promise<ActionOutput>((resolve, reject) => {
144144
axios.get(`http://127.0.0.1:${port}${apiPath}`).then( resp => {
@@ -157,7 +157,11 @@ export class SamIntegrationTestFixture extends TestFixture {
157157
// If the tests completed successfully, happily delete the fixture
158158
// (otherwise leave it for humans to inspect)
159159
if (success) {
160-
rimraf(this.integTestDir);
160+
const cleaned = rimraf(this.integTestDir);
161+
if (!cleaned) {
162+
// eslint-disable-next-line no-console
163+
console.error(`Failed to clean up ${this.integTestDir} due to permissions issues (Docker running as root?)`);
164+
}
161165
}
162166
}
163167
}
@@ -207,23 +211,24 @@ export async function shellWithAction(
207211
let actionOutput: any;
208212
let actionExecuted = false;
209213

210-
function executeAction(chunk: any) {
214+
async function maybeExecuteAction(chunk: any) {
211215
out.push(Buffer.from(chunk));
212216
if (!actionExecuted && typeof filter === 'string' && Buffer.concat(out).toString('utf-8').includes(filter) && typeof action === 'function') {
213217
actionExecuted = true;
214-
writeToOutputs('before executing action');
215-
action().then((output) => {
216-
writeToOutputs(`action output is ${output}`);
218+
writeToOutputs('before executing action\n');
219+
try {
220+
const output = await action();
221+
writeToOutputs(`action output is ${JSON.stringify(output)}\n`);
217222
actionOutput = output;
218223
actionSucceeded = true;
219-
}).catch((error) => {
220-
writeToOutputs(`action error is ${error}`);
224+
} catch (error: any) {
225+
writeToOutputs(`action error is ${error}\n`);
221226
actionSucceeded = false;
222227
actionOutput = error;
223-
}).finally(() => {
224-
writeToOutputs('terminate sam sub process');
228+
} finally {
229+
writeToOutputs('terminate sam sub process\n');
225230
killSubProcess(child, command.join(' '));
226-
});
231+
}
227232
}
228233
}
229234

@@ -234,6 +239,7 @@ export async function shellWithAction(
234239
() => {
235240
if (!actionExecuted) {
236241
reject(new Error(`Timed out waiting for filter ${JSON.stringify(filter)} to appear in command output after ${actionTimeoutSeconds} seconds\nOutput so far:\n${Buffer.concat(out).toString('utf-8')}`));
242+
killSubProcess(child, command.join(' '));
237243
}
238244
}, actionTimeoutSeconds * 1_000,
239245
).unref();
@@ -242,20 +248,22 @@ export async function shellWithAction(
242248
child.stdout!.on('data', chunk => {
243249
writeToOutputs(chunk);
244250
stdout.push(chunk);
245-
executeAction(chunk);
251+
void maybeExecuteAction(chunk);
246252
});
247253

248254
child.stderr!.on('data', chunk => {
249255
writeToOutputs(chunk);
250256
if (options.captureStderr ?? true) {
251257
stderr.push(chunk);
252258
}
253-
executeAction(chunk);
259+
void maybeExecuteAction(chunk);
254260
});
255261

256262
child.once('error', reject);
257263

258-
child.once('close', code => {
264+
// Wait for 'exit' instead of close, don't care about reading the streams all the way to the end
265+
child.once('exit', (code, signal) => {
266+
writeToOutputs(`Subprocess has exited with code ${code}, signal ${signal}\n`);
259267
const output = (Buffer.concat(stdout).toString('utf-8') + Buffer.concat(stderr).toString('utf-8')).trim();
260268
if (code == null || code === 0 || options.allowErrExit) {
261269
let result = new Array<string>();
@@ -270,7 +278,6 @@ export async function shellWithAction(
270278
reject(new Error(`'${command.join(' ')}' exited with error code ${code}. Output: \n${output}`));
271279
}
272280
});
273-
274281
});
275282
}
276283

@@ -279,10 +286,6 @@ function killSubProcess(child: child_process.ChildProcess, command: string) {
279286
* Check if the sub process is running in container, so child_process.spawn will
280287
* create multiple processes, and to kill all of them we need to run different logic
281288
*/
282-
if (fs.existsSync('/.dockerenv')) {
283-
child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`);
284-
} else {
285-
child.kill('SIGINT');
286-
}
287-
289+
child.kill('SIGINT');
290+
child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`);
288291
}

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ class NotificationArnsStack extends cdk.Stack {
748748
const arnsFromEnv = process.env.INTEG_NOTIFICATION_ARNS;
749749
super(parent, id, {
750750
...props,
751-
// comma separated list of arns.
751+
// comma separated list of arns.
752752
// empty string means empty list.
753753
// undefined means undefined
754754
notificationArns: arnsFromEnv == '' ? [] : (arnsFromEnv ? arnsFromEnv.split(',') : undefined)
@@ -793,12 +793,12 @@ class MetadataStack extends cdk.Stack {
793793
const handle = new cdk.CfnWaitConditionHandle(this, 'WaitConditionHandle');
794794
handle.addMetadata('Key', process.env.INTEG_METADATA_VALUE ?? 'default')
795795

796-
}
796+
}
797797
}
798798

799799
const app = new cdk.App({
800800
context: {
801-
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build
801+
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID ?? process.env.GITHUB_RUN_ID, // Force all assets to be unique, but consistent in one build
802802
},
803803
});
804804

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class RollbacktestStack extends cdk.Stack {
9191

9292
const app = new cdk.App({
9393
context: {
94-
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build
94+
'@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID ?? process.env.GITHUB_RUN_ID, // Force all assets to be unique, but consistent in one build
9595
},
9696
});
9797

‎packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/sam_cdk_integ_app/lib/test-stack.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') {
2121
}
2222

2323
const isRunningOnCodeBuild = !!process.env.CODEBUILD_BUILD_ID;
24+
const isRunningOnGitHubActions = !!process.env.GITHUB_RUN_ID;
25+
const isRunningOnCi = isRunningOnCodeBuild || isRunningOnGitHubActions;
2426

2527
class CDKSupportDemoRootStack extends Stack{
2628
constructor(scope, id, props) {
@@ -102,7 +104,7 @@ class CDKSupportDemoRootStack extends Stack{
102104
bundling: {
103105
forcedDockerBundling: true,
104106
// Only use Google proxy in the CI tests, as it is blocked on workstations
105-
goProxies: isRunningOnCodeBuild ? [GoFunction.GOOGLE_GOPROXY, 'direct'] : undefined,
107+
goProxies: isRunningOnCi ? [GoFunction.GOOGLE_GOPROXY, 'direct'] : undefined,
106108
},
107109
});
108110

‎packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v1.130.0/bootstrapping.integtest.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/@aws-cdk-testing/cli-integ/tests/init-go/init-go.integtest.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import { integTest, withTemporaryDirectory, ShellHelper, withPackages } from '..
1313
// Canaries will use the generated go.mod as is
1414
// For pipeline tests we replace the source with the locally build one
1515
if (!isCanary) {
16-
await shell.shell(['go', 'mod', 'edit', '-replace', 'github.com/aws/aws-cdk-go/awscdk/v2=$CODEBUILD_SRC_DIR/go/awscdk']);
16+
const dir = process.env.CODEBUILD_SRC_DIR ?? process.env.GITHUB_WORKSPACE;
17+
if (!dir) {
18+
throw new Error('Cannot figure out CI system root directory');
19+
}
20+
21+
await shell.shell(['go', 'mod', 'edit', '-replace', `github.com/aws/aws-cdk-go/awscdk/v2=${dir}/go/awscdk`]);
1722
}
1823

1924
await shell.shell(['go', 'mod', 'tidy']);

‎packages/@aws-cdk-testing/cli-integ/tests/init-typescript-app/init-typescript-app.integtest.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ const TYPESCRIPT_VERSION_AGE_DAYS = 2 * 365;
2424

2525
const TYPESCRIPT_VERSIONS = typescriptVersionsYoungerThanDaysSync(TYPESCRIPT_VERSION_AGE_DAYS, typescriptVersionsSync());
2626

27-
// eslint-disable-next-line no-console
28-
console.log(TYPESCRIPT_VERSIONS);
29-
3027
/**
3128
* Test our generated code with various versions of TypeScript
3229
*/
@@ -45,6 +42,10 @@ TYPESCRIPT_VERSIONS.forEach(tsVersion => {
4542
await removeDevDependencies(context);
4643

4744
await shell.shell(['npm', 'install', '--save-dev', `typescript@${tsVersion}`]);
45+
46+
// After we've removed devDependencies we need to re-install ts-node because it's necessary for `cdk synth`
47+
await shell.shell(['npm', 'install', '--save-dev', 'ts-node@^10']);
48+
4849
await shell.shell(['npm', 'install']); // Older versions of npm require this to be a separate step from the one above
4950
await shell.shell(['npx', 'tsc', '--version']);
5051
await shell.shell(['npm', 'prune']);

0 commit comments

Comments
 (0)