No coverage files found for Node v5.8.0 and NPM v3.7.3

[arrowrowe]

UPDATE: See @addaleax's analysis here.

---

See these Travis logs for details:

• Using Node v5.8.0 and NPM v3.7.3 (failed)
• Using Node v5.8.0 and NPM v3.8.1 (passed)
• Using Node v5.7.1 (passed)
• Using Node v4.4.0 (passed)

ERROR: No coverage files found.

This happens only using Node v5.8.0 and NPM v3.7.3 together.

---

UPDATE: Thanks to @novemberborn's response and @thangngoc89's suggestion in MoOx/phenomic#296, I am working around telling Travis to install latest NPM (i.e. 3.8.1). Below is added to .travis.yml.

before_install:
  - npm install -g npm@latest

[novemberborn]

@arrowrowe I cloned https://github.com/arrowrowe/romi and installed it locally. npm test ran fine:

-----------|----------|----------|----------|----------|----------------|
File       |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
-----------|----------|----------|----------|----------|----------------|
 romi/     |    94.34 |      100 |      100 |      100 |                |
  index.js |    94.34 |      100 |      100 |      100 |                |
-----------|----------|----------|----------|----------|----------------|
All files  |    94.34 |      100 |      100 |      100 |                |
-----------|----------|----------|----------|----------|----------------|

This is on OS X, npm 3.8.1.

Can you reproduce this outside of Travis?

[sarbbottam]

Works fine for me in v5.8.0

[arrowrowe]

Thanks for responding! I will retry it later to confirm.

[arrowrowe]

Failed on Ubuntu 14.04.1, using Node v5.8.0 and NPM v3.7.3, as the log below shows. (No need to paste it in fact. It is same with the Travis log.)

   3 passed

----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
----------|----------|----------|----------|----------|----------------|
All files |      100 |      100 |      100 |      100 |                |
----------|----------|----------|----------|----------|----------------|

ERROR: No coverage files found.
npm ERR! Test failed.  See above for more details.

However, npm test does run fine after I update to NPM v3.8.1, (the version @novemberborn uses,) remove node_modules and npm i again. So, is this problem happening only using Node v5.8.0 and NPM v3.7.3?........ Or was I making a mistake somewhere?...

Not sure what causes this.........

[novemberborn]

So, is this problem happening only using Node v5.8.0 and NPM v3.7.3?........ Or was I making a mistake somewhere?...

If you remove node_modules and reinstall under npm 3.7.3, does it work then?

[arrowrowe]

@novemberborn I suppose not, as Travis's fresh install (Node v5.8.0 and NPM v3.7.3) fails. I am confirming it now. Will update later.

---

upd: It works on my Ubuntu............... Seriously confused now. I am restarting Travis's build. Will update later.

---

upd: The Travis build with Node v5.8.0 and NPM v3.7.3 still fails. I am confirming all this on my Linux server, in case anything local effects the result. Will update later.

---

upd: It fails on my Linux server, using Node v5.8.0 and NPM v3.7.3. Confirmed. The thing is:

• Install Node v5.8.0. It comes with NPM v3.7.3. Run npm install. Now npm test fails.
• Now npm i -g npm@latest to update NPM to v3.8.1. (NOTE: don't remove node_modules.) Now npm test passes.
• Now npm i -g npm@3.7.3. (NOTE: don't remove node_modules.) Now npm test passes.
• Uninstall Node v5.8.0 and reinstall it again. It comes with NPM v3.7.3. Now npm test fails again.

[thangngoc89]

@arrowrowe woa. Thanks for you efforts. So much trouble with npm.

[bcoe]

@arrowrowe this is a really interesting bug! I'd be interested to diff npm@3.8.1 vs. npm@3.7.3 -- I wonder if we touched anything in the script execution logic.

[addaleax]

I think I understand this.

This has nothing to do with npm versions at all. [Update: Okay, at least not the ones mentioned above. See below.] As far as I can tell, it is because the Node.js distributions have the shebang at the start of npm-cli.js replaced with a (relative) reference to the Node.js executable.
This throws spawn-wrap off, though: It relies on being able to tell that Node.js is invoked (e.g. because the command name that should be executed is node) or that adding a directory containing a wrapper script to the PATH environment variable will suffice. Since the command name is npm, not node, and the Node.js interpreter path is hardcoded in the npm-cli.js, spawn-wrap cannot attach its handler code anywhere inside the npm process.
This could even be fine, because PATH contains the spawn-wrap wrapper, so that one should get executed when a Node.js script with #!/usr/bin/env node (like mocha) is invoked, right? Well, npm likes to make its own changes to PATH, and sets the directory of the currently running Node.js executable with a higher priority than everything else in PATH. So again, no chance for spawn-wrap to attach itself. [Update: Thanks to #211, I looked up the change in npm where this was introduced (commit, PR), i.e. npm versions starting from v3.7.0 are affected.]

Re-installing npm solved the problem because npm’s copy of npm-cli.js contains no modified shebang, so the “right” wrapper script gets invoked. Another workaround might be running nyc node $(which npm) test or something like that.

(You can see the difference between the installed npm-cli.js files here.)

I don’t know whether this is fixable without either some special handing of npm commands in spawn-wrap or changes to the npm cli itself.

[arrowrowe]

Thanks a lot @addaleax! I will leave the right to close this issue to the owner @bcoe.

[addaleax]

I think I’d be willing to write a patch for spawn-wrap which adds special handling for npm, if there’s a chance it gets landed. @bcoe, what do you think? izs seems to have abandoned that module, but given that you’re a member of npm and tapjs, I imagine you can assess the situation better than I do?

[bcoe]

@addaleax wonderful find, yes please submit a patch to spawn-wrap and I can help land it.

[bcoe]

@addaleax if I'm reading this correctly, does the issue arise when the platform is detected as PORTABLE?

[addaleax]

@bcoe The issues arises either way. PORTABLE is set for generating the Node.js binary distribution tarballs, and is not set when running their make install in a source tree (I have tested this with both configurations). In any case the node binary is invoked using its full path, and that moves PATH out of the way.

[sarbbottam]

There is a work around to this, please refer this comment at #211, instead of npm test, if the command aliased to npm test is used, it works fine.

[bcoe]

@addaleax what's a good email to use for you, I'd love to get you into our slack so we can debug this issue.

[addaleax]

@bcoe Seen? Would like to delete the comment with my raw email address if that’s okay. 😄

[arrowrowe]

@jamestalmage thanks for the advice! It does benefit shortening scripts. I didn't notice the fact that my tortuous usage put npm run lint under coverage as well :(. And making scripts recursively call themselves is no more than a bad personal habit, which I will remember to avoid later on. The direct usage of AVA and XO in command line is cool; it is just sometimes there are different test suits across projects, and npm test always works. Thanks for your advice again.

[jamestalmage]

and npm test always works

Indeed, that's a good reason to use it. If you consistently use test:only and test:easy for all projects in your organization - that is good reason to include those scripts in package.json as well. It serves as documentation on the sort of build commands that can be run. I was just pointing out that there are shortcuts which should run a little faster.

[addaleax]

And making scripts recursively call themselves is no more than a bad personal habit, which I will remember to avoid later on.

I think it’s quite okay. Either way, the issue with running nyc + recursive npm scripts should be fixed soon, too.

[jamestalmage]

I think it’s quite okay.

Agreed - it's not a mistake to do so, just a bit verbose in this particular case.

[novemberborn]

You almost certainly don't want npm run lint being run with coverage

@jamestalmage I doubt this has any adverse effects though. The linter code wouldn't be instrumented and it would use the filesystem to access the code.

[jamestalmage]

I doubt this has any adverse effects though.

Wrapping with spawn-wrap and foreground-child have their cost. The use of append-transform has (minimally) adverse affects on require time. The affect should be minimal however.

[bcoe]

@addaleax @arrowrowe can we close this at this point? If you run:

npm i nyc@next

We've landed various fixes to the 5.8.0 bug.

[arrowrowe]

@bcoe good to me now! 💯

[MoOx]

Maybe close when you cut a stable release? (This helps people tracking the issue to now when it's "stable").

[bcoe]

nyc@6.2.1 is released.