Skip to content
This repository has been archived by the owner on Jul 6, 2019. It is now read-only.

fix: spawn child processes with node without relying on the shebang. #174

Merged
merged 1 commit into from
Apr 13, 2018
Merged

fix: spawn child processes with node without relying on the shebang. #174

merged 1 commit into from
Apr 13, 2018

Conversation

jdalton
Copy link
Contributor

@jdalton jdalton commented Apr 12, 2018

This PR attempts to fix the issue reported in #172 (comment) by running commands through Node in much the same way as when passed a nodeArg, or when run in the npx process. The only time it doesn't run through Node is when existing is falsey, in which case it will be allowed to pass through unchanged to child.runCommand so it errors appropriately.

@jdalton
Copy link
Contributor Author

jdalton commented Apr 12, 2018

Depends on #173.

@zkat
Copy link
Owner

zkat commented Apr 12, 2018

@jdalton #173 has been merged.

I think I'd definitely prefer to have a unit test for this 'cause it was more dangerous than I expected :)

@jdalton
Copy link
Contributor Author

jdalton commented Apr 13, 2018

Added test for --always-spawn as well as for resolving the promise after the command is run and executing the command with node.

@zkat
Copy link
Owner

zkat commented Apr 13, 2018

Did you rebase this onto latest? I see appveyor is failing

@jdalton
Copy link
Contributor Author

jdalton commented Apr 13, 2018

A different case of needing escapeArg. All fixed!

@jdalton
Copy link
Contributor Author

jdalton commented Apr 13, 2018

One of the tests is flakey (timeouts in ci). I'll fix that.

Hmm may just be a bad travis-ci night...

@jdalton
Copy link
Contributor Author

jdalton commented Apr 13, 2018

Ok it looks like the ci hiccup has passed and so have the tests 😋

@zkat zkat merged commit cba97bb into zkat:latest Apr 13, 2018
@jdalton jdalton deleted the fix-spawn branch April 13, 2018 23:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants