Use shell option when spawning lerna exec

[jwb]

Motivation and Context

The README example in the exec section would not supply any argument to npm view.

How Has This Been Tested?

When I invoked the command properly, I discovered that environment variables are not expanded at all, so I enabled the shell option in the call to spawn.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[evocateur]

Can you add an integration test for this?

[evocateur]

I'm not sure this property is necessary, as execa is handling this for us.

[evocateur]

lerna exec -- npm view \$LERNA_PACKAGE_NAME works perfectly fine as-is on master, for example.

[jwb]

It was broken with spawn...

[jwb]

npm view reports on the package in the current directory.

[evocateur]

Ah yes, I just reproduced it locally.

  test.concurrent("$LERNA_PACKAGE_NAME", () => {
    return initFixture("ExecCommand/basic").then((cwd) => {
      const args = [
        "exec",
        "echo",
        "\$LERNA_PACKAGE_NAME",
        "--concurrency=1",
      ];

      return execa(LERNA_BIN, args, { cwd }).then((result) => {
        expect(result.stdout).toMatchSnapshot("echo: $LERNA_PACKAGE_NAME");
      });
    });
  });

[evocateur]

Interestingly, it doesn't seem to matter if you pass "$LERNA_PACKAGE_NAME", or "\$LERNA_PACKAGE_NAME", in the integration test, but it certainly matters in the CLI command.

[jwb]

The integration test isn't evaluating the variables before sending the arguments to lerna.

[jwb]

Added an integration test that fails on the master branch and passes with the fix.

[evocateur]

Thanks @jwb!

[evocateur]

oh noes Windows :P

[jwb]

How should I make the test use a different value on WinDies?

[evocateur]

Actually, my windows env var syntax was wrong, it should be %LERNA_PACKAGE_NAME% apparently

[jwb]

Thanks for all the help @evocateur!

[lock]

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.