-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
test: do not assume process.execPath
contains no spaces
#55028
base: main
Are you sure you want to change the base?
Conversation
We had a bunch of tests that would fail if run from an executable that contains any char that should be escaped when run from a shell.
Review requested:
|
// The execPath might contain chars that should be escaped in a shell context. | ||
// On non-Windows, we can pass the path via the env; `"` is not a valid char on | ||
// Windows, so we can simply pass the path. | ||
const execNode = (args, callback) => exec( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be moved to common?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, (almost) each test has a different implementation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55028 +/- ##
==========================================
- Coverage 88.04% 88.03% -0.01%
==========================================
Files 652 652
Lines 183764 183797 +33
Branches 35862 35868 +6
==========================================
+ Hits 161787 161810 +23
- Misses 15233 15237 +4
- Partials 6744 6750 +6 |
We had a bunch of tests that would fail if run from an executable that contains any char that should be escaped when run from a shell.