Files reported twice with different case

[jimbuck]

I created a yeoman generator that creates Typescript projects with support for nyc and ava out of the box. Below is the output I am receiving when trying to get the coverage results (run npm test or gulp coverage):

The same thing also happens when opening an lcov report in the browser. See this project as the current example. It is currently reporting a lowercase c:\ path and an uppercase C:\ path.

You can install the template with npm i -g generator-jb-module.

[jimbuck]

Any insight as to where to begin? Is this a bug with nyc or istanbul?

[addaleax]

Hm. Does this still happen with nyc 7.x.x? What Node.js version are you using? (If v6: does this happen with pre-v6 versions, too?)

[jimbuck]

"nyc": "^6.4.4",

>node -v
v6.2.1

Let me update nyc and try again.

[jimbuck]

Yep:

[jimbuck]

Maybe the map files are pointing to the source files with the wrong case?

I just tried with both inlineSourceMaps and individual sourcemaps. Both display the same result. And the sourcemaps use relative paths.

[addaleax]

It’s a long shot, but it really might be worth trying this out with Node v4. There have been changes to fs.realpath, which is used in the module loader, and I know that there has been some case-related Windows weirdness based on it. I think that particular issue only applied to certain kinds of drives, so it’s probably not that, but it’s the only guess I have.

[jimbuck]

I'll test it out with older versions of node and report back. Thanks for the tip!

[jimbuck]

Well, I have no idea what was causing that:

Node Version	Working Correctly
4.4.7	✓
6.0.0	✓
6.2.1	✓
6.3.0	✓

It was probably just nyc@6.4 with an older version of node.

I'll re-open if it occurs again.

Thanks for your suggestions!

[jimbuck]

Well, it just re-appeared. First within the terminal of Visual Studio Code, and now also appearing in the regular cmd windows as well. Is there a cache that nyc reads from that I should try clearing?

[jimbuck]

From @JaKXz, on jimbuck/bio-sim#2:

@JimmyBoh I'm not seeing duplicate file reports (#269) with this setup. Could you try npm cache clear before npm i -D nyc?

Trying that now...

Nope, that did not seem to help.

[jimbuck]

Might be an outdated version of ava.

Checking now...

So after trying a few different tricks, I think I've found a working solution after updating to the latest version of ava. When running nyc ava -v it appeared to work fine. So the issue had to lie in my gulpfile.js where more complex commands were being run.

In the end:
test: "nyc ava -v --color"
and
coverage: "nyc report --reporter=lcov ava -v & start coverage/lcov-report/index.html"

did the trick for me.

Closing this once and for all. If the issue happens again I will dig deeper first.

[jimbuck]

I've done some research, and this is definitely a bug, but where the bug exists depends on your perspective.

Summary of the Issue: On Windows files are being reported twice, with duplicates having zero coverage. This only occurs when the --all flag is specified!

Within nyc:
If you do not specify the --all flag then everything works fine. Files are loaded based on which tests are run. The absolute paths to these files begin with an uppercase drive letter. From what I learned these paths enter nyc in the _handleJS method.

If you do specify the --all flag, then before the _handleJS method gets called all of the files matching the includes/excludes are added via the addAllFiles method. These files begin with a lowercase drive letter.

Within external dependencies:
So why the difference in drive letters? When specifying --all there are two possible "culprits".

The first is glob which is used in the walkAllFiles method but that specifies the cwd exactly and it matches what we gave it. So I'd say that it is working as expected. "Goood glob!"

The second possible offender is istanbul-lib-hook which is used in the _addHook method (which happens to be the only place where _handleJS is called). I'm not sure what happens within this module, but it does result in different cased drive letters being passed in to the _handleJS method. Most likely istanbul-lib-hook is working fine, since nothing jumped out at me when I saw the code. So what is it then?

Within Node.js
Turns out this issue is actually rooted all the way down inside Node.js. nodejs/node#6624 documents the various cases, but here is the tl;dr from what @thorn0 said:

In cmd:

d:\dev\foo>node test
__dirname D:\dev\foo
__filename D:\dev\foo\test.js
require.resolve("./test") D:\dev\foo\test.js
require("path").resolve("test.js") d:\dev\foo\test.js
process.cwd() d:\dev\foo

The basic gist is that things aren't consistent with Node.js on Windows.

Conclusion:
So where is the real bug? Who should fix it? That would be up to the @istanbuljs team. Whether it is fixed in istanbul-lib-hook may or may not fix it (and if it does fix it, what else might it break?). The other fix (that I will gladly make a PR for) requires one new line right within the _handleJS method, and makes everything work perfectly. Ahhh, that feels nice...

I will create the PR and we can discuss it's validity further.

[addaleax]

Aaaah, right, nodejs/node#6624 was what I had in mind… thanks for the digging!

[bcoe]

@JimmyBoh great digging! sounds like something that would be worth patching in Node itself potentially -- the danger with istanbul getting overly creative, e.g., down-casing, is that it could break case-sensitive file-systems.

Let's keep this open and track nodejs/node#6624

[jimbuck]

@bcoe Thanks! I agree that this should be patched on Node itself, but it seems like they are hesitant to change it for backwards compat.

My change doesn't force any particular case, just overrides the provided absolute path to use the internally specified path (this.cwd).

Either way, I'm just glad I figured it out. The only "downside" to not using --all and waiting for a fix is that I cannot get any metric for files that are missing tests. Obviously the work-around is to create a test file for everything, but who wants to do that? 😜