fix: make tests compatible with Node.js -pre releases

[Trott]

This PR contains:

• [x] bugfix
• [x] refactor
• [x] tests

Breaking Changes?

• [ ] yes
• [x] no

Please Describe Your Changes

I don't know why, but execa.node() with the input option works with Node.js -pre releases where the current configuration fails. This has the benefit of making sure that the test is run with the version of Node.js currently being used to execute the test suite, rather than whatever is first in the user's path.

Before, with Node.js 13.0.0-pre:

$ ~/io.js/node node_modules/.bin/ava test/test.js

  1 test failed

  binary stanity

  /Users/trott/temp/csv-parser/test/test.js:247

   246:
   247:   t.snapshot(stdout)
   248: })

  Did not match snapshot

  Difference:

  - ''
  + '{"a":"1"}'
$

FWIW, this means that the test fails with CITGM run against the Node.js master branch.

This is somewhat strange because the offending commit in Node.js to be removed to fix this would seem to be ones like this: https://github.com/nodejs/node/commit/bbbe07a4475b06b9295007c6f5bcefb5ba1431e5 I imagine the issue is somewhere inside execa but I'm not 100% sure. Nonetheless, I think this is a better/more-clear/less-brittle way to write the particular line?

[shellscape]

@Trott thanks for taking the time to put together a PR. Node pre-releases aren't something I'm in the habit of supporting in any project I maintain. This odd incompatibility seems like something better taken up with execa than here in the tests. It looks like the node() method just dropped in June (https://github.com/sindresorhus/execa/releases/tag/v2.0.0), which would explain why I hadn't heard of it :smile:

I think the current test as-is is clear to anyone familiar with shell commands, and an echo and pipe shouldn't be foreign. For clearness, I think the current is the winner. For intent, we def want this to be executed using the shell. node() docs state the shell option cannot be used, which is a negative here (https://github.com/sindresorhus/execa/blob/master/index.js#L251), as the goal is to simulate running in the/a shell.

[Trott]

@Trott thanks for taking the time to put together a PR. Node pre-releases aren't something I'm in the habit of supporting in any project I maintain. This odd incompatibility seems like something better taken up with execa than here in the tests. It looks like the node() method just dropped in June (https://github.com/sindresorhus/execa/releases/tag/v2.0.0), which would explain why I hadn't heard of it 😄

I think the current test as-is is clear to anyone familiar with shell commands, and an echo and pipe shouldn't be foreign. For clearness, I think the current is the winner. For intent, we def want this to be executed using the shell. node() docs state the shell option cannot be used, which is a negative here (https://github.com/sindresorhus/execa/blob/master/index.js#L251), as the goal is to simulate running in the/a shell.

Fair enough. I'll see if I can find and fix the bug in execa (if it's actually there!).

For this, I'd still recommend changing node to ${process.execPath} so that you're testing with whatever version of Node.js the rest of the tests ran with rather than whatever is in the user's path (which will usually be the same node binary but not always, of course).