feat(ospec): Accept list of filenames through stdin

[maranomynet]

Description

In response to suggestions made in #2133

Motivation and Context

See rationale in #2133 ...plus add support for custom file-name and folder patterns.

How Has This Been Tested?

(Same as #2133 except with piping)
Tested on the command line within the mithril repo itself.

> cd mithril.js
> find {stream,ospec}/**/tests/**/*.js | node ospec/bin/ospec
1 assertions completed in 13ms, of which 0 failed
377 assertions completed in 19ms, of which 0 failed
> cd docs
> find ../stream/**/tests/**/*.js | node ../ospec/bin/ospec
95 assertions completed in 11ms, of which 0 failed

etc. etc...

It should be safe to pass multiple instances of the same file as multiple require() calls to the same file only result in a single invocation of their side-effects.

> cd mithril.js
> find stream/tests/*.js stream/**/tests/*.js | node ospec/bin/ospec
95 assertions completed in 11ms, of which 0 failed

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation change

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.
• I have updated docs/change-log.md

[maranomynet]

This is my first time playing with process.stdin and friends, so this approach may have embarrassing beginner's mistakes.

[pygy]

@maranomynet thanks for the PR!

Indeed, there are some issues with how you handle the stdin stream, you may leave some data out. See https://nodejs.org/dist/latest-v10.x/docs/api/stream.html#stream_readable_read_size for an example that pulls data exhaustively.

[pygy]

Once this is merged I'll release ospec v2.0 (or it could be part of v2.1, actually, no hurry @maranomynet).

[maranomynet]

Thanks for the pointer. I've amended the PR accordingly.

[pygy]

Thanks @maranomynet, but actually the chunks will not necessarily split between file names (that's a non-obvious caveat that I should have pointed out explicitly, sorry).

It would be best to accumulate the whole thing into a string before splitting on \n.

Something along this (also taking Windows into account):

let fileList = ""
let chunk
while ((chunk = process.stdin.read()) !== null) fileList += chunk

fileList.toString().split(/\r?\n/).forEach(function(fileName) {
  if (fileName) {
    require(path.join(process.cwd(),  fileName)) // eslint-disable-line global-require
  }
})
o.run()

It may be best to wait for @tivac's input before updating the PR, he may have other suggestions.

The try/catch block isn't very useful here... the .catch() thing is there to avoid having a unhandled rejection warning.

BTW, shouldn't that .catch() handler call console.error rather than console.log()?

[maranomynet]

Ah interesting. All this is sort of obvious in hindsight. :-)

But my assumption was that the .catch() on the Promise was in case require() errored, so I tried to mimic that behaviour in the .forEach() callback, to make both invocation styles as equal as possible.

...and yes console.error seems more logical

[pygy]

Yes, require() can throw, but by default in an async, promise handler context that needs to be caught to prevent the additional warning. When processing the plain file list that's not an issue...

[maranomynet]

Removed try

[pygy]

We're getting there :-)

In the example find module/**/*.test.js doesn't work.

ls module/**/*.test.js would though, but it is POSIX-only.

I hoped that cash-ls could be used cross-platform, but its piped output is polluted (colors, bad format).

[maranomynet]

In what way does it not "work"?
Is it because find is not a default Windows command (unless you run bash via the linux subsystem)?

[pygy]

On macOS, find doesn't expand globs:

$ ls *.js
foo.js  bar.js

$ find *.js
find: *.js: No such file or directory

Edit: could it be a badly configured shell? Is the shell supposed to expand globs before passing them to commands?

[maranomynet]

Hmm.. really?
I'm on OSX running all of the above find commands, and they work fine.

[maranomynet]

Ah, I just realised that there seems to be a huge difference between globs in zsh and bash.
I'm using zsh (omyzsh) and find **/*.js returns a longer list of files than if I run the same command in bash.

Nontheless, bash still gives me a list of files - it just takes the ** part more literally and only looks in sub-folders one level down.

[maranomynet]

I'll update the example to use a dumber/simpler file pattern.

[maranomynet]

I'm still a bit baffled about the bit you quoted:

$ find *.js
find: *.js: No such file or directory

Regardless of which shell I run (bash, zsh, sh) this sort of command always returns the same thing as the ls command. (In fact ls -1 *.js returns the exact same, single column list of files)

man find indicates that both * and ? work as simple wildcards, so the results I get seem correct.

Anyway, what do you suggest I do?

[pygy]

find *.js works from bash, but not from zsh (configured through prezto eons ago, which probably explains why it doesn't work as it should).

Even with globs, actually, in bash, ls node_modules/**/lib/*.js | cat and find node_modules/**/lib/*.js` have the exact same output, so your initial example was probably fine (it looked fine, I only tested it to be sure because I don't use globs day to day and wanted to be sure...), sorry for the noise.

[maranomynet]

OK, I'm leaving the example in its current simpler state, as it seems good enough to get the general point across.

[pygy]

https://github.com/latentflip/node-glob-cli would be a suitable cross-platform solution. (glob-cli on npm). There's a more popular glob-cli2 but it outputs a space-separated list.

[maranomynet]

Sure, but what are we trying to accomplish at this point?

So ospec supports reading a list of files through stdin and the documentation explains that, showing a simple, non-exhaustive example.

User's are then left to choose the file-listing strategy suitable to their use case. Right?

[pygy]

I wanted to make sure the feature was equally useful on all supported platforms. Thanks to glob-cli, it is. I mentioned it because CLI tooling is not an area I'm used to develop, so mentioning it here may elicit useful feedback.

I'll add a note about glob-cli later, the PR is fine as it is AFAI'm concerned (still waiting for @tivac's look though, just to be sure).

[maranomynet]

This is all mostly new for me too.
Also both ls and find accept multiple file-pattern arguments (i.e. ls -1 foo/*.js bar/*.js baz/*.js) so there's fair bit of flexibility possible without proper globstar support.

[pygy]

Windows is a supported platform too, and cross-platform use cases are important for teams with a non-uniform dev setup.

[pygy]

Superseeded by #2141