From 9139ca96ffc010e13393aff927d7b7eacfbae4f9 Mon Sep 17 00:00:00 2001 From: Alex Rewa Date: Wed, 19 Oct 2022 23:02:47 +0300 Subject: [PATCH] fix(integ-runner): Fix call to spawnSync for hooks commands (#22429) Fixes the issue #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* --- .../lib/runner/integ-test-runner.ts | 26 ++++++++++++------- packages/@aws-cdk/integ-runner/lib/utils.ts | 8 ++++++ .../test/runner/integ-test-runner.test.ts | 14 +++++++--- .../integ.json | 8 +++--- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts index cf6fe6b9efb5b..dcdaa58f2e47f 100644 --- a/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts +++ b/packages/@aws-cdk/integ-runner/lib/runner/integ-test-runner.ts @@ -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'; @@ -222,8 +222,10 @@ 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({ @@ -231,8 +233,10 @@ export class IntegTestRunner extends IntegRunner { }); 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) { @@ -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 @@ -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), + }); }); } diff --git a/packages/@aws-cdk/integ-runner/lib/utils.ts b/packages/@aws-cdk/integ-runner/lib/utils.ts index 74e5eb2154ae7..c783e6ca2a491 100644 --- a/packages/@aws-cdk/integ-runner/lib/utils.ts +++ b/packages/@aws-cdk/integ-runner/lib/utils.ts @@ -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 diff --git a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts index 9fad8eb71a1a9..b817c9bdb7f11 100644 --- a/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts +++ b/packages/@aws-cdk/integ-runner/test/runner/integ-test-runner.test.ts @@ -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"'], ]), ])); }); diff --git a/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.test-with-snapshot-assets/integ.json b/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.test-with-snapshot-assets/integ.json index 2cd7ddd429f8a..126b54791cd02 100644 --- a/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.test-with-snapshot-assets/integ.json +++ b/packages/@aws-cdk/integ-runner/test/test-data/cdk-integ.out.test-with-snapshot-assets/integ.json @@ -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 \"postDestroy hook\""] }, "allowDestroy": [ "AWS::IAM::Role"