Skip to content

Commit

Permalink
fix(integ-runner): Fix call to spawnSync for hooks commands (aws#22429)
Browse files Browse the repository at this point in the history
Fixes the issue aws#22344

I reworked the approach of calling `exec` by splitting each command in hook to the command itself and it's arguments. All hooks were affected: preDeploy, postDeploy, preDestroy, postDestroy.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
arewa authored and mrgrain committed Oct 24, 2022
1 parent 3acfc57 commit 73c069d
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 17 deletions.
26 changes: 17 additions & 9 deletions packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { RequireApproval } from '@aws-cdk/cloud-assembly-schema';
import { DeployOptions, DestroyOptions } from 'cdk-cli-wrapper';
import * as fs from 'fs-extra';
import * as logger from '../logger';
import { chain, exec } from '../utils';
import { chunks, exec } from '../utils';
import { DestructiveChange, AssertionResults, AssertionResult } from '../workers/common';
import { IntegRunnerOptions, IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base';

Expand Down Expand Up @@ -222,17 +222,21 @@ export class IntegTestRunner extends IntegRunner {
const actualTestCase = this.actualTestSuite.testSuite[testCaseName];
try {
if (actualTestCase.hooks?.preDestroy) {
exec([chain(actualTestCase.hooks.preDestroy)], {
cwd: path.dirname(this.snapshotDir),
actualTestCase.hooks.preDestroy.forEach(cmd => {
exec(chunks(cmd), {
cwd: path.dirname(this.snapshotDir),
});
});
}
this.cdk.destroy({
...destroyArgs,
});

if (actualTestCase.hooks?.postDestroy) {
exec([chain(actualTestCase.hooks.postDestroy)], {
cwd: path.dirname(this.snapshotDir),
actualTestCase.hooks.postDestroy.forEach(cmd => {
exec(chunks(cmd), {
cwd: path.dirname(this.snapshotDir),
});
});
}
} catch (e) {
Expand All @@ -255,8 +259,10 @@ export class IntegTestRunner extends IntegRunner {
const actualTestCase = this.actualTestSuite.testSuite[testCaseName];
try {
if (actualTestCase.hooks?.preDeploy) {
exec([chain(actualTestCase.hooks?.preDeploy)], {
cwd: path.dirname(this.snapshotDir),
actualTestCase.hooks.preDeploy.forEach(cmd => {
exec(chunks(cmd), {
cwd: path.dirname(this.snapshotDir),
});
});
}
// if the update workflow is not disabled, first
Expand Down Expand Up @@ -297,8 +303,10 @@ export class IntegTestRunner extends IntegRunner {
});

if (actualTestCase.hooks?.postDeploy) {
exec([chain(actualTestCase.hooks?.postDeploy)], {
cwd: path.dirname(this.snapshotDir),
actualTestCase.hooks.postDeploy.forEach(cmd => {
exec(chunks(cmd), {
cwd: path.dirname(this.snapshotDir),
});
});
}

Expand Down
8 changes: 8 additions & 0 deletions packages/@aws-cdk/integ-runner/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ export function chain(commands: string[]): string {
return commands.filter(c => !!c).join(' && ');
}

/**
* Split command to chunks by space
*/
export function chunks(command: string): string[] {
const result = command.match(/(?:[^\s"]+|"[^"]*")+/g);
return result ?? [];
}


/**
* A class holding a set of items which are being crossed off in time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,16 +347,22 @@ describe('IntegTest runIntegTests', () => {
// THEN
expect(spawnSyncMock.mock.calls).toEqual(expect.arrayContaining([
expect.arrayContaining([
'echo "preDeploy"',
'echo', ['"preDeploy hook"'],
]),
expect.arrayContaining([
'echo "postDeploy"',
'echo', ['"postDeploy hook"'],
]),
expect.arrayContaining([
'echo "preDestroy"',
'echo', ['"preDestroy hook"'],
]),
expect.arrayContaining([
'echo "postDestroy"',
'echo', ['"postDestroy hook"'],
]),
expect.arrayContaining([
'ls', [],
]),
expect.arrayContaining([
'echo', ['-n', '"No new line"'],
]),
]));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
"stackUpdateWorkflow": false,
"diffAssets": false,
"hooks": {
"preDeploy": ["echo \"preDeploy\""],
"postDeploy": ["echo \"postDeploy\""],
"preDestroy": ["echo \"preDestroy\""],
"postDestroy": ["echo \"postDestroy\""]
"preDeploy": ["echo \"preDeploy hook\"", "ls", "echo -n \"No new line\""],
"postDeploy": ["echo \"postDeploy hook\""],
"preDestroy": ["echo \"preDestroy hook\""],
"postDestroy": ["echo