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

Fix non-executable execution with input option #212

Merged
merged 5 commits into from May 7, 2019

Conversation

stroncium
Copy link
Contributor

@stroncium stroncium commented May 6, 2019

execa throws internal error instead of rejecting with proper error when running non-executable with input.

fixes #166

Failing test: https://travis-ci.org/sindresorhus/execa/builds/528756511?utm_source=github_status&utm_medium=notification

Added test, fixed the problem.

test.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@ehmicky ehmicky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for comment about styling consistency with other tests.

@ehmicky
Copy link
Collaborator

ehmicky commented May 6, 2019

Thanks for this PR @stroncium!

This issue was reported with #166. There is an ongoing fix within Node.js here: nodejs/node#26852

However for current and older Node.js versions, we should definitely use that check, i.e. looks good to me.

Just need @sindresorhus review as well before merging.

@stroncium stroncium changed the title Fix non-executable execution with input Fix non-executable execution with input, fixes #166 May 6, 2019
index.js Show resolved Hide resolved
Yanis Benson and others added 2 commits May 7, 2019 09:40
@sindresorhus sindresorhus changed the title Fix non-executable execution with input, fixes #166 Fix non-executable execution with input option May 7, 2019
@sindresorhus sindresorhus merged commit a428249 into sindresorhus:master May 7, 2019
@stroncium stroncium deleted the fix-non-executable branch May 7, 2019 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash with input option and absolute path without execute permissions
3 participants