test: fix spawn on windows - #7049
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Could you just do const opts = {shell: common.isWindows};
Sorry, something went wrong.
There was a problem hiding this comment.
Changed.
Sorry, something went wrong.
|
LGTM. How was this working on the CI machines? |
Sorry, something went wrong.
|
@cjihrig Probably they have some sort of mingw/cygwin/etc. environment installed? I noticed it while running tests on a local Windows 7 VM that doesn't have any of that kind of stuff installed. |
Sorry, something went wrong.
Sorry, something went wrong.
|
Windows CI machines have Git for Windows installed, and use the executables provided by that. There should be a few more tools required to run the tests, I once compiled a list but it should be quite outdated now: https://github.com/nodejs/node/wiki/Installation#building-on-windows (bottom of that section). It's good to see one of those dependencies go away, LGTM. |
Sorry, something went wrong.
|
CI one last time before landing: https://ci.nodejs.org/job/node-test-pull-request/2942/ |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: nodejs#7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Most Windows systems do not have an external `echo` program installed, so any attempts to spawn `echo` as a child process will fail with `ENOENT`. This commit forces the use of the built-in `echo` provided by `cmd.exe`. PR-URL: #7049 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: João Reis <reis@janeasystems.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
@thealphanerd While this landed cleanly on V4 it doesn't work and the test case still fails there. The shell:true option only exists in V6 and later. I'm submitting a separate PR to implement the fix differently in that branch |
Sorry, something went wrong.
Checklist
Affected core subsystem(s)
Description of change
Most Windows systems do not have an external echo program installed, so any attempts to spawn echo as a child process will fail with ENOENT. This commit forces the use of the built-in echo provided by cmd.exe.