Skip to content
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.

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

chore(cli): integ tests don't buffer shell output #31903

Merged
merged 2 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 21 additions & 21 deletions packages/@aws-cdk-testing/cli-integ/lib/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,19 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
throw new Error('Use either env or modEnv but not both');
}

const outputs = new Set(options.outputs);
const writeToOutputs = (x: string) => {
for (const outputStream of outputs) {
outputStream.write(x);
}
};

// Always output the command
(options.output ?? process.stdout).write(`💻 ${command.join(' ')}\n`);

let output: NodeJS.WritableStream | undefined = options.output ?? process.stdout;
switch (options.show ?? 'always') {
case 'always':
break;
case 'never':
case 'error':
output = undefined;
break;
}
writeToOutputs(`💻 ${command.join(' ')}\n`);
const show = options.show ?? 'always';

if (process.env.VERBOSE) {
output = process.stdout;
outputs.add(process.stdout);
}

const env = options.env ?? (options.modEnv ? { ...process.env, ...options.modEnv } : process.env);
Expand All @@ -46,12 +44,16 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
const stderr = new Array<Buffer>();

child.stdout!.on('data', chunk => {
output?.write(chunk);
if (show === 'always') {
writeToOutputs(chunk);
}
stdout.push(chunk);
});

child.stderr!.on('data', chunk => {
output?.write(chunk);
if (show === 'always') {
writeToOutputs(chunk);
}
if (options.captureStderr ?? true) {
stderr.push(chunk);
}
Expand All @@ -66,8 +68,8 @@ export async function shell(command: string[], options: ShellOptions = {}): Prom
if (code === 0 || options.allowErrExit) {
resolve(out);
} else {
if (options.show === 'error') {
(options.output ?? process.stdout).write(out + '\n');
if (show === 'error') {
writeToOutputs(`${out}\n`);
}
reject(new Error(`'${command.join(' ')}' exited with error code ${code}.`));
}
Expand Down Expand Up @@ -97,10 +99,8 @@ export interface ShellOptions extends child_process.SpawnOptions {

/**
* Pass output here
*
* @default stdout unless quiet=true
*/
readonly output?: NodeJS.WritableStream;
readonly outputs?: NodeJS.WritableStream[];

/**
* Only return stderr. For example, this is used to validate
Expand All @@ -127,9 +127,9 @@ export class ShellHelper {
private readonly _cwd: string,
private readonly _output: NodeJS.WritableStream) { }

public async shell(command: string[], options: Omit<ShellOptions, 'cwd' | 'output'> = {}): Promise<string> {
public async shell(command: string[], options: Omit<ShellOptions, 'cwd' | 'outputs'> = {}): Promise<string> {
return shell(command, {
output: this._output,
outputs: [this._output],
cwd: this._cwd,
...options,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk-testing/cli-integ/lib/staging/maven.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export async function uploadJavaPackages(packages: string[], login: LoginInforma
`-Dfile=${pkg.replace(/.pom$/, '.jar')}`,
...await pathExists(sourcesFile) ? [`-Dsources=${sourcesFile}`] : [],
...await pathExists(javadocFile) ? [`-Djavadoc=${javadocFile}`] : []], {
output,
outputs: [output],
modEnv: {
// Do not try to JIT the Maven binary
MAVEN_OPTS: `${NO_JIT} ${process.env.MAVEN_OPTS ?? ''}`.trim(),
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk-testing/cli-integ/lib/staging/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export async function uploadNpmPackages(packages: string[], login: LoginInformat
await shell(['node', require.resolve('npm'), 'publish', path.resolve(pkg)], {
modEnv: npmEnv(usageDir, login),
show: 'error',
output,
outputs: [output],
});

console.log(`✅ ${pkg}`);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk-testing/cli-integ/lib/staging/nuget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export async function uploadDotnetPackages(packages: string[], usageDir: UsageDi
'--disable-buffering',
'--timeout', '600',
'--skip-duplicate'], {
output,
outputs: [output],
});

console.log(`✅ ${pkg}`);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk-testing/cli-integ/lib/staging/pypi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export async function uploadPythonPackages(packages: string[], login: LoginInfor
TWINE_REPOSITORY_URL: login.pypiEndpoint,
},
show: 'error',
output,
outputs: [output],
});

console.log(`✅ ${pkg}`);
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,9 @@ export interface CdkCliOptions extends ShellOptions {
* Prepare a target dir byreplicating a source directory
*/
export async function cloneDirectory(source: string, target: string, output?: NodeJS.WritableStream) {
await shell(['rm', '-rf', target], { output });
await shell(['mkdir', '-p', target], { output });
await shell(['cp', '-R', source + '/*', target], { output });
await shell(['rm', '-rf', target], { outputs: output ? [output] : [] });
await shell(['mkdir', '-p', target], { outputs: output ? [output] : [] });
await shell(['cp', '-R', source + '/*', target], { outputs: output ? [output] : [] });
}

interface CommonCdkBootstrapCommandOptions {
Expand Down
21 changes: 13 additions & 8 deletions packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export function withSamIntegrationFixture(block: (context: SamIntegrationTestFix
export class SamIntegrationTestFixture extends TestFixture {
public async samShell(command: string[], filter?: string, action?: () => any, options: Omit<ShellOptions, 'cwd' | 'output'> = {}): Promise<ActionOutput> {
return shellWithAction(command, filter, action, {
output: this.output,
outputs: [this.output],
cwd: path.join(this.integTestDir, 'cdk.out').toString(),
...options,
});
Expand Down Expand Up @@ -182,7 +182,12 @@ export async function shellWithAction(
throw new Error('Use either env or modEnv but not both');
}

options.output?.write(`💻 ${command.join(' ')}\n`);
const writeToOutputs = (x: string) => {
for (const output of options.outputs ?? []) {
output.write(x);
}
};
writeToOutputs(`💻 ${command.join(' ')}\n`);

const env = options.env ?? (options.modEnv ? { ...process.env, ...options.modEnv } : undefined);

Expand All @@ -206,17 +211,17 @@ export async function shellWithAction(
out.push(Buffer.from(chunk));
if (!actionExecuted && typeof filter === 'string' && Buffer.concat(out).toString('utf-8').includes(filter) && typeof action === 'function') {
actionExecuted = true;
options.output?.write('before executing action');
writeToOutputs('before executing action');
action().then((output) => {
options.output?.write(`action output is ${output}`);
writeToOutputs(`action output is ${output}`);
actionOutput = output;
actionSucceeded = true;
}).catch((error) => {
options.output?.write(`action error is ${error}`);
writeToOutputs(`action error is ${error}`);
actionSucceeded = false;
actionOutput = error;
}).finally(() => {
options.output?.write('terminate sam sub process');
writeToOutputs('terminate sam sub process');
killSubProcess(child, command.join(' '));
});
}
Expand All @@ -235,13 +240,13 @@ export async function shellWithAction(
}

child.stdout!.on('data', chunk => {
options.output?.write(chunk);
writeToOutputs(chunk);
stdout.push(chunk);
executeAction(chunk);
});

child.stderr!.on('data', chunk => {
options.output?.write(chunk);
writeToOutputs(chunk);
if (options.captureStderr ?? true) {
stderr.push(chunk);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,7 @@ integTest(
const targetName = template.replace(/.js$/, '');
await shell([process.execPath, template, '>', targetName], {
cwd: cxAsmDir,
output: fixture.output,
outputs: [fixture.output],
modEnv: {
TEST_ACCOUNT: await fixture.aws.account(),
TEST_REGION: fixture.aws.region,
Expand Down
Loading