c8 Code Coverage Displaying Code Misses on Multi-line Comments

[eliatcodecov]

(Note that the reported issue below was encountered in CI on github action's ubuntu-latest (the ci.yaml used). I'm not directly sure what version of Node is packaged with github action's ubuntu-latest, but I'll try to find out.)

• Version:
• Platform: ubuntu-latest image on GitHub Actions.

Greetings from Codecov! Recently we've encountered some strange reporting using the v8 (i.e., c8) reporter in vitest that I wanted to surface as an Issue for your consideration.

It seems like multiline comments are are showing up as coverage misses. Additionally, it seems like -- in some cases -- the first line of these multi-line comments is counted as a conditional false (an uncovered branch in other words). You can see precisely what this looks like on Codecov for this commit.

Note the addition of the license comment in the diff and how Codecov is reporting as fully uncovered.

We can see v8 as the coverage reporter in the vitest config of the repo here

I dug into the raw uploaded reports a bit (the can be downloaded from the Uploads sidebar here). And this is clearly an issue with the coverage output itself and not Codecov directly. Here's a snippet of the output for the tracker.js file at SHA 250b1b2:

 <package name="counterscale.public">
      <metrics statements="165" coveredstatements="0" conditionals="1" coveredconditionals="0" methods="1" coveredmethods="0"/>
      <file name="tracker.js" path="/home/runner/work/counterscale/counterscale/public/tracker.js">
        <metrics statements="165" coveredstatements="0" conditionals="1" coveredconditionals="0" methods="1" coveredmethods="0"/>
        <line num="1" count="0" type="cond" truecount="0" falsecount="1"/>
        <line num="2" count="0" type="stmt"/>
        <line num="3" count="0" type="stmt"/>
        <line num="4" count="0" type="stmt"/>
        <line num="5" count="0" type="stmt"/>
        <line num="6" count="0" type="stmt"/>
        <line num="7" count="0" type="stmt"/>
        <line num="8" count="0" type="stmt"/>
        <line num="9" count="0" type="stmt"/>
        <line num="10" count="0" type="stmt"/>
        //...truncated
        <line num="32" count="0" type="stmt"/>
        <line num="33" count="0" type="stmt"/>
        <line num="34" count="0" type="stmt"/>
        <line num="35" count="0" type="stmt"/>
        //...

In the above xml output you can clearly see the conditional false reporting on line 1 as well as the multiple coverage misses for the multiline comment itself. I'm not sure if the issue is with the clover based XML output, or something more fundamental, but thought I'd surface it here since the example that led to the misreported coverage is so straightforward. If there's anything else you need, please let me know. Thanks!

[bcoe]

I am unable to reproduce this missing coverage with a simple isolated case:

https://github.com/bcoe/c8-515

Is remix perhaps performing some bundling?

[bcoe]

@eliatcodecov digging a bit more, the issue appears to be cropping up for a file that has not been exercised by a test.

When a file isn't run, if the --all flag is used, c8 creates a zeroed out coverage report by simply looking at the length of the file:

              emptyReports.push({
                scriptId: 0,
                url: resolve(fullPath),
                functions: [{
                  functionName: '(empty-report)',
                  ranges: [{
                    startOffset: 0,
                    endOffset: stat.size,
                    count: 0
                  }],
                  isBlockCoverage: true
                }]
              })

I'm curious what behaviour you were expecting?

first line of these multi-line comments is counted as a conditional false (an uncovered branch in other words)

This behaviour seems like a bug to me, I'll see if I can get to the bottom of it.

[eliatcodecov]

It's possible that the vitest run --coverage command is passing in the --all flag by default. I'm not sure what vitest is doing under the hood, but the --all flag isn't being explicitly passed, at least not that I can see in the CI output. It would make sense in this case, though, as I don't believe any of the tests in this repo are actually exercising the code in the impacted file, tracker.js.

[cenfun]

The issue you're encountering seems to be related to v8-to-istanbul.

• sometimes the sourcemap does not work as expected, which prevents an accurate match to the position in the source file.
• It seems incapable of converting comment lines and empty lines

Previously, we also used v8-to-istanbul, but faced some issues which you can see here. As no resolution for this problem was found, we implemented our own converter instead of v8-to-istanbul, aiming to rectify these issues. You can find our project here: monocart-coverage-reports.

Let's use https://github.com/bcoe/c8-515 as the example to see a comparison of the results.

On the left, c8 is in use, and on the right mcr is in use.

{
  "scripts": {
    "test-c8": "npx c8 --reporter=text --reporter=html --reporter=lcovonly node cov.js",
    "test-mcr": "npx mcr --reports text,html,lcovonly -o coverage-html \"node cov.js\"",
    "test": "npm run test-c8 && npm run test-mcr"
  },
  "devDependencies": {
    "c8": "^9.1.0",
    "monocart-coverage-reports": "^2.2.1"
  }
}

Comparison of results from lcov:

Note, when use mcr (right side) the comment lines and empty lines will be ignored.

[AriPerkkio]

Recently we've encountered some strange reporting using the v8 (i.e., c8) reporter in vitest

The c8 usage was removed from Vitest about 7 months ago. Issues that you are seeing there do not apply to c8 package directly.

• feat!: add @vitest/coverage-v8 package vitest-dev/vitest#3339
• feat!: remove @vitest/coverage-c8 package vitest-dev/vitest#3614

[bcoe]

@AriPerkkio I was looking at @vitest/coverage-v8, and it also uses @bcoe/v8-coverage, which is where I believe the bug is being described by @eliatcodecov (so I'm fine with keeping a bug open here).

@cenfun is probably correct that there's issues with how source-maps are applied, but I don't think this is what's happening in this case, it seems related to the --all flag.

With regards to the output shared...

Looking through old issues, it looks like I was aware of this issue #182, but have just never prioritized the work to implement the functionality (of skipping comment blocks).

I'd like to start by digging into the branch discrepancy described in this issue, and then am open to addressing #182 (since it sounds like a few folks are excited to see this functionality).

[cenfun]

@bcoe Thanks for the feedback.
Actually, this case also has issues might caused by the sourcemap.

From the earlier code, we know that 'queue' should be '[]', therefore, I don't think the left side coverage is correct.

const window = {
};
let queue = (window.counterscale && window.counterscale.q) || [];

I want to provide an example to explain the issue about the comment lines and blank lines.

if (false) 
console.log("uncovered")

No doubt, the line coverage of the above code is 50%.

After adding some comments, the line coverage has changed to 90%, but we expect it should be 50%.
What you think?

[bcoe]

I did a bit more digging. I believe v8-to-istanbul is intentionally adding a single function and a single block, so that we show 0% coverage when --all is used.

Because v8-to-istanbul does not parse the files when --all is used, we don't know the percentage of block coverage / function coverage. Instead this is displayed:

It seems like what might be better for this scenario would be introducing the concept of unknown, which would show up as 0% coverage, but not introduce a placeholder block and function coverage entry.