Fix: Windows - wrap npm path in quotes, use shell when spawning npm

[BarryThePenguin]

Fixes #26

[mysticatea]

Thank you very much!

Hmmm, but this PR is not working on my PC.
I'll investigate.

     Error: spawn "C:\Users\t-nagashima.AD\nodist\bin\npm.CMD" ENOENT
      at exports._errnoException (util.js:856:11)
      at Process.ChildProcess._handle.onexit (internal/child_process.js:178:32)
      at onErrorNT (internal/child_process.js:344:16)

[BarryThePenguin]

Sure. I didn't think it would be that simple... It could need a check for the existence of a space in the path?

I'm just guessing now though...

[mysticatea]

I'm trying to make a test case for this.
And I guess maybe we can use just npm (without which) if we use {shell: true}

[BarryThePenguin]

Just a suggestion, would it be worth using an existing cross platform spawn utility like node-cross-spawn-async

[mysticatea]

Good idea.

... I'm confusing now....
The current master is passing the test for a path which includes a space.

  [common] npm-run-all
    should run tasks even if npm exists at a path which includes a space:
npm: C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced bin\npm.CMD
      √ lib version (1727ms)
      √ command version (1705ms)


  2 passing (3s)

    describe("should run tasks even if npm exists at a path which includes a space:", () => {
        const originalPath = process.env.PATH;

        before(() => {
            process.env.PATH = Path.resolve(__dirname, "../test-workspace/spaced bin/") + ";" + process.env.PATH; // eslint-disable-line
        });
        after(() => {
            process.env.PATH = originalPath;
        });

        it("lib version", () =>
            runAll(["test-task:append a"], {parallel: false})
                .then(() => {
                    assert(result() === "aa");
                })
        );

        it("command version", () =>
            command(["test-task:append a"])
                .then(() => {
                    assert(result() === "aa");
                })
        );
    });

[BarryThePenguin]

Do you have the test available for me to try? What version node are we both running?
I'm on v5.5.0

[mysticatea]

Thank you, I pushed it: https://github.com/mysticatea/npm-run-all/tree/fix-for-space

I tried 4.3.2 and 5.7.1

[mysticatea]

Sorry, I pushed again.

[BarryThePenguin]

Yeah... ok. Tests work here too

But not the project this issue originated from...

[mysticatea]

I confirmed to reproduce it on webpack-validator.

C:\Users\t-nagashima.AD\Documents\GitHub\webpack-validator [master]> npm run validate

> webpack-validator@1.0.0-beta.6 validate C:\Users\t-nagashima.AD\Documents\GitHub\webpack-validator
> npm-run-all --parallel lint cover build test:configs --sequential check-coverage

npm: C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced bin\npm.CMD

'C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced' is not recognized as an internal or external command,
operable program or batch file.
'C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced' is not recognized as an internal or external command,
operable program or batch file.
'C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced' is not recognized as an internal or external command,
operable program or batch file.
'C:\Users\t-nagashima.AD\Documents\GitHub\npm-run-all\test-workspace\spaced' is not recognized as an internal or external command,
operable program or batch file.
ERROR: lint: None-Zero Exit(1);

[mysticatea]

OK, I confirmed to resolve this issue with using cross-spawn-async.
Though I'm still not sure why the test works, I will make npm-run-all using cross-spawn-async and release it.

[mysticatea]

@BarryThePenguin I really appreciate your works and suggestions!

[BarryThePenguin]

👏 No problem, happy to help. Ping me anytime