search

AvianFlu / ncp

Asynchronous recursive file copying with Node.js.
MIT License
681 stars 103 forks source link

Support io.js 1.0 #78

Closed bajtos closed 9 years ago

bajtos commented 9 years ago

Fix the condition used to check whether streams are modern, correctly detect io.js 1.0 as a modern engine.

This fixes the install script used by phantomjs which hangs inside ncp at the moment.

I was not able to reproduce the problem locally inside ncp, thus there is no test to cover this change :(

@mmalecki @AvianFlu Could you please review, land and release this patch as soon as reasonably possible? ncp is used e.g. by Karma test runner (karma-phantomjs-launcher), thus most front-end people cannot upgrade to io.js until this fix is landed.

bajtos commented 9 years ago

FWIW, the test failure seems like a timing problem, tests sometimes fail sometimes pass when run on my machine.

bajtos commented 9 years ago

Workaround for phantomjs, until this patch is landed: install phantomjs globally, e.g. via brew install phantomjs. npm install phantomjs will pick the global version and skip the failing install.js script.

AvianFlu commented 9 years ago

Sorry, didn't see this until just now.

Any insight on those across-the-board test failures from Travis?

bajtos commented 9 years ago

Any insight on those across-the-board test failures from Travis?

IIRC, the test "ncp modified files copies change source file mtime and copy" fails on some runs and then passes on subsequent runs. I assume it's a timing issue in the test, but I don't have time to investigate it myself.

bajtos commented 9 years ago

You can try to re-run the CI jobs, possibly repeatedly, and hopefully after few runs the tests will be green.

bajtos commented 9 years ago

Closing in favour of #80 and/or cpr.