shebang screws up line count in coverage

[bcoe]

No description provided.

[profnandaa]

@bcoe - a little more description on this one, how to reproduce this?

[l3laze]

Tested using Ubuntu 18.04.1 (with UserLAnd on an Android 6.0.1 device) both with and without a shebang in each of two scripts -- a minimal single-file script and a real project (only 110 lines, but still) being run using it's test script.

With a shebang

• It does not report coverage for some of the last lines of a file, whether they're code, comment, or blank.

• It seems to default to 3 lines considered uncovered, but it can go at least as low as 2 uncovered lines (in a 4 line file: shebang + blank + comment + comment).

• It will consider the shebang itself to be uncovered if there are 3 or less lines.

• It can be extended to up to 15 uncovered lines by adding blanks. I tested with up to 1000 blank lines at the end, and it maxed out at 15.

Without a shebang

100% coverage in both the test script and real project.

[bcoe]

@l3laze @profnandaa here's an example of code that triggers the problem:

#!/usr/bin/env node
'use strict'

const foreground = require('foreground-child')
const mkdirp = require('mkdirp')
const report = require('../lib/report')
const rimraf = require('rimraf')
const {
  hideInstrumenteeArgs,
  hideInstrumenterArgs,
  yargs
} = require('../lib/parse-args')

I'm pretty sure we pretty much just need to modify this line here to take into account the shebang when source is loaded; so a regex to detect the shebang, followed by modifying the starting point of where we start collecting coverage in the file.

[profnandaa]

@bcoe - will definitely dig in this weekend and get back, thanks for the pointer 👍

[l3laze]

I don't understand much of this, especially the module.wrapper part above, but this hack (in script.js) is working using your test script above and my real project + test script.

_buildLines (source, lines) {
  let position = 0
  source.split('\n').forEach((lineStr, i) => {
    // Ignore shebang, but mark it as covered
    if (i === 0 && /\#\!.*node/.test(lineStr)) {
      lines.push(new CovLine(i + 1, position, position + lineStr.length))
    } else {
      this.eof = position + lineStr.length
      lines.push(new CovLine(i + 1, position, this.eof))
      position += lineStr.length + 1 // also add the \n.
    }
  })
}

It would be a good idea to check for a proper shebang though, as this will of course just accept any line with #! and node and it's plenty simple to mess them up. With that it may also be a good idea to check for the portable shebang, e.g. #!/usr/bin/env node, rather than allowing "any ol' way" which may only work on some users systems as they're of course not all portable. c8 isn't a lint tool though, so it seems like it's not really something it should be handling. There's also the issue of how to report it to users.

Edit: forgot to mention where the hack of a kludge was.

[l3laze]

That was truly a hack. It's adding +1 to the line count and ruining [at least] the function counter. Spent some time trying to fix it last night, but will have another go again from a fresh copy today. This would probably be much easier with PC :-?

Edit: Wasted another two days now and with zero progress over multiple attempts from fresh copies I'm ready for a break. Going to work on something of my own instead, but will try again at some point if it's not fixed by then.

My black box approach is definitely not working though -- will need to find docs for the v8 coverage output and I think how to access it from script.js to be able to figure this out, if that even helps at all.

Another edit: Just reread the article linked from Reddit and realized the coverage is literally sitting right there..lol. Sorry, I've been pretty sick lately and am still recovering; will give this another shot ASAP and try not to cover it in my germs.

Final edit (9/30/2018):

Something deeper down has now completely broken coverage for scripts with a shebang, and so everything I learned up until now about the issue has become effectively useless. Lol. IDK for sure as I'm clearly no expert, but it seems like mixing and matching parts from Istanbul/NYC with c8 and trying to maintain compatibility with each is breaking both projects.

[shinnn]

Seems to be fixed in istanbuljs/v8-to-istanbul#15.

[profnandaa]

@shinnn -- good to hear. cc. @bcoe