nyc icon indicating copy to clipboard operation
nyc copied to clipboard

Merged results have a lower coverage rating

Open skerit opened this issue 4 years ago • 11 comments

Link to bug demonstration repository

I'm trying this on my https://github.com/11ways/hawkejs repository.

It's a template manager that can run on the server and in the browser, nearly all of the code is shared.

So I'd like to merge the coverage results generated by nyc on the server, and the coverage results generated by puppeteer/v8 in the browser. Which I eventually managed to do, but the results are not as I expected

The problem

The server-side run coverage creates a file like f6ff46e8-956d-4325-8d02-b9b26ffb70cd.json in the .nyc_output, as expected.

Then I use puppeteer to generate the browser-side coverage. The browser can't access the original source files, but it gets 1 big concatenated js file with an internal source-map. (I don't use browserify for that, but it's a similar approach)

Unfortunately, when I pipe those puppeteer coverage results as-is into puppeteer-to-istanbul, it ignores the source map info and just adds it as a new file to the coverage result, like so:

 hawkejs/.nyc_output/js/127.0.0.1_36473/hawkejs |    51.05 |      100 |      100 |    51.05 |                   |
  hawkejs-client.js                             |    51.05 |      100 |      100 |    51.05 |... 01,50602,50603 |

So I wrote some code to convert the original puppeteer coverage ranges into ranges for the actual original files. (That code can be found here: https://github.com/11ways/protoblast/blob/322a4dbf3c41e572c273b19e983b1dfba1e0615b/lib/init.js#L1359)

Unfortunately for me again, puppeteer-to-istanbul insists on copying the source files into .nyc_output/js/ even though I supply an absolute path as the original url, so then I still get duplicate (& different) file results:

 hawkejs/.nyc_output/js/mnt/pom/develry/hawkejs/lib/core       |     3.19 |      100 |      100 |     3.19 |                   |
  base.js                                                      |     1.72 |      100 |      100 |     1.72 |... 13,114,115,116 |
  block_buffer.js                                              |     2.87 |      100 |      100 |     2.87 |... 29,730,731,732 |
< ... cut ... >
 hawkejs/lib/core                                              |    69.69 |    56.45 |    72.82 |    69.78 |                   |
  base.js                                                      |    89.19 |       50 |     87.5 |    89.19 |       72,95,96,99 |
  block_buffer.js                                              |     63.4 |    61.65 |    61.29 |     63.4 |... 33,634,637,638 |

So I forked puppeteer-to-istanbul (https://github.com/skerit/puppeteer-to-istanbul) and modified it so it would leave absolute paths alone.

And that kind of works, now it merges those files together! But... the merged report has a lower coverage result compared to when I only run the server-side coverage.

Here's the report on a single file, based on the server-side coverage:

File                         | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
  base.js                    |   89.19 |       50 |     87.5 |    89.19 |       72,95,96,99 |

Here's the same file, but then based only on the browser-side coverage:

File                         | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
  base.js                    |    6.19 |      100 |      100 |     6.19 |... 10,111,112,113 |

And then this is the same file, but combined:

File                         | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
  base.js                    |    29.2 |    42.86 |    87.5 |   89.19 | 72,95-99 

Shouldn't it add the 2 reports together? It almost seems like the results are averaged. Does it have anything to do with how puppeteer-to-istanbul results always seem to report 100% branch & function coverage?

Troubleshooting steps

  • [x] still occurring when I put cache: false in my nyc config

Environment Information

  System:
    OS: Linux 5.4 Ubuntu 20.04 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i7-6770HQ CPU @ 2.60GHz
    Memory: 1.22 GB / 22.95 GB
  Binaries:
    Node: 12.18.2 - /usr/local/bin/node
    npm: 6.14.5 - /usr/local/bin/npm
  npmPackages:
    nyc: ^15.1.0 => 15.1.0 
    puppeteer-to-istanbul: skerit/puppeteer-to-istanbul => 1.4.0

skerit avatar Jul 17 '20 15:07 skerit

Alright... So if I'm understanding it right, when nyc merges 2 reports it kind of assumes they have the same branch layout?

*Edit: Yes, that's exactly it. I switched over to instrumenting my code using istanbul & then getting the __coverage__ object from puppeteer after all the tests are done. No need to mess with the puppeteer/v8 coverage results.

skerit avatar Jul 18 '20 10:07 skerit

Is this project abandoned? This issue just reared its ugly-ass head in my work, but with Cypress instead of puppeteer. Would like to work around it, but if the entire thing is abandonware we will have to take another route

cellog avatar Oct 27 '20 16:10 cellog

@cellog With "this project" you mean puppeteer-to-istanbul? No idea, but my solution to instrument the code using istanbul is working perfectly, much better than anything the internal v8 coverage results could offer I think.

skerit avatar Oct 28 '20 11:10 skerit

I mean nyc

cellog avatar Oct 28 '20 13:10 cellog

still a thing - im trying to merge coverage reports from parallel test runs on circleCI but the merged reports are missing some coverage

Dakuan avatar Apr 08 '22 15:04 Dakuan

We also had a lower coverage when merging multiple coverage reports generated by multiple parallel agents on the CI.

The issue was caused by the absolute path in front of each file name. In fact, because it was generated by 2 agents, the paths were different but for the same file. By removing the absolute path, the merge went well !

jogelin avatar Aug 11 '22 11:08 jogelin

I'm facing the same issue right now and I'm looking for a solution. I'm trying to merge coverage reports from vitest and playwright, both are being produced by istanbul

spuxx-dev avatar Mar 13 '24 11:03 spuxx-dev