LCOV report misaligned after upgrade to Node 10.16

[oliversalzburg]

• Version: 10.16.0
• Platform: Windows 10 x64 / Amazon Linux

After upgrading to NodeJS 10.16, the coverage report is off. Here is an example:

10.15.3

10.16.0

I'm assuming a relation to nodejs/node#26579

[oliversalzburg]

Node 12.4.0 works as expected again.

[bcoe]

@oliversalzburg unfortunately Node 10 has some bugs that don't exist in 11 or 12 ... and it's fairly tricky to back-port. It might be worth keeping this open just in case there is a clear cause that jumps out looking at the underlying coverage reports.

[oliversalzburg]

What can I do to help? Is this even the right place for the report or should it be reported in the NodeJS tracker?

[bcoe]

@oliversalzburg here is a fine place to report 👍 I theoretically work on Node.js any ways 😝 (although haven't had much time to do so lately).

Would happily accept help on this issue, or on other issues on c8. To debug this sort of issue I usually look at the raw output from V8, which is placed in coverage/tmp -- looking at the delta between Node 10 and Node 12 can be a good way to see what's off.

[bcoe]

... unfortunately, to actually fix what's off it potentially means back-porting work from V8, which isn't always easy or possible. Would happily provide direction on these topics though.

[shinnn]

@bcoe The real cause is not a Node.js 10 bug but https://github.com/istanbuljs/v8-to-istanbul/blob/4e926ba71682e49ea357026c36d9a3bf04331714/lib/v8-to-istanbul.js#L11-L15 , I think. v8-to-istanbul still assumes Node.js v10.6.0 uses a CJS wrapper to require files, although nodejs/node#26579 is merged.

The possible solution is,

- const isNode10 = !!process.version.match(/^v10/)
+ const isOlderNode10 = /^v10\.[0-5]/u.test(process.version)

[bcoe]

@shinnn good catch, I'd blanked on the fact that we corrected this issue; @oliversalzburg would you like to submit a patch for this?

[oliversalzburg]

@bcoe Sorry, seems like I missed your message :( Thanks for taking care of it.