-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
(integ-runner): Hooks do not follow spawnSync argument requirements #22344
Comments
@iriskraja77 thanks for reporting this bug! We shouldn't be using chain at all since exec expects a list and the hooks already provide a list. |
Hi guys, @iriskraja77, @corymhall I prepared a PR with the fix, could you review it? I reworked the call to |
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*
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*
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*
I think this was closed by #22429 |
Comments on closed issues and PRs are hard for our team to see. |
Describe the bug
Currently the integration test command hooks are not working.
spawnSync requires the main command and its arguments to be split into two arguments. For example, to run
npm run test
, the command specified to the spawnSync should be:spawnSync('npm', 'run test', {....})
However, using the chain function, will simply provide the full command as:
spawnSync('npm run test', {....})
which simply does not work with spawn.Expected Behavior
Commands defined in the hooks argument would work.
Current Behavior
Impossible to use hooks.
Reproduction Steps
Simply provide a command as part of the preDestroy hook.
e.g.
echo hi
Will throw an error:
spawnSync <command> ENOENT
Possible Solution
Fix the chain function to support spawn.
Additional Information/Context
No response
CDK CLI Version
2.41.0
Framework Version
No response
Node.js Version
16.15.1
OS
MacOS
Language
Typescript
Language Version
No response
Other information
No response
The text was updated successfully, but these errors were encountered: