-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] Incorrect code coverage when converting from V8 to istanbul #50
Comments
if If they can't fix the bug, maybe there is another way is convert v8 format to lcov format directly, but I don't know anything about lcov now. |
Got it, I've reported this issue at istanbuljs/v8-to-istanbul#216. |
For anyone else reading this thread, I'd like to leave here feedback about a workaround I've used until the issue the the What I did was have 2 code coverage related fixtures as part of my playwright tests:
To understand more about how you could implement the above please read this comment on microsoft/playwright#7030. For a code example read the README and see the code on the edumserrano/monocart-code-coverage-demo repo. My workaround is essentially the merging of both configurations and fixtures on /tests/babel-istanbul and [/tests/monocart(https://github.com/edumserrano/monocart-code-coverage-demo/tree/main/tests/monocart). A potential downside of this workaround is that I'm instrumenting the code twice. I haven't noticed any added slowness when running the tests but your mileage may vary. You could argue that instrumenting the code via |
@edumserrano I'm guessing you are using the workaround for normal playwright tests, not Component Testing right? |
@azamat-sharapov yes, not doing component testing as far as I'm aware. Doing "normal" playwright tests. The sample repo I created shows the kind of tests I'm doing. Also I'm not sure what you mean by "the workaround for normal playwright tests"... |
@edumserrano It's been a week, but there is no update on the issue. |
@cenfun you're a genius! 👏 I can confirm this is now all working properly! Well done, thank you very much 🎉 |
Hi @cenfun,
Bug description
I mainly use the V8 code coverage when working in my dev machine and all works great. However, I have a requirement to upload code coverage to SonarQube and it requires an
lcov
file. When I enable themonocart-reporter
option to get thelcov
file as such:The code coverage that I get is incorrect. Please see screenshots below for more information. The code coverage screenshots were produced by running with:
this shows correct code coverage
.toIstanbul
property set which defaults tofalse
:this shows correct code coverage
.toIstanbul
property set totrue
:this shows INCORRECT code coverage
.babel+istanbul
app/core/ui-services/ui-helper-functions.ts
app/features/find-institution/institutions-list/institutions-list.component.ts
v8 + monocart reporter without the toIstanbul property set which defaults to false
src/app/core/ui-services/ui-helper-functions.ts
src/app/features/find-institution/institutions-list/institutions-list.component.ts
v8 + monocart reporter with the toIstanbul property set to true
app/core/ui-services/ui-helper-functions.ts
app/features/find-institution/institutions-list/institutions-list.component.ts
How to reproduce
npm install
to install all the required packagesTo get the babel+istanbul code coverage:
npm run test-istanbul
. The first time tests will fail because of missing snapshots, you can ignore this.npm run nyc-report
.npm run open-istanbul-coverage
.To get the v8 + monocart reporter without the toIstanbul property set which defaults to false code coverage:
npm run test-monocart
. The first time tests will fail because of missing snapshots, you can ignore this.npm run open-monocart-coverage
.To get the v8 + monocart reporter with the toIstanbul property set to true code coverage:
npm run test-monocart
. The first time tests will fail because of missing snapshots, you can ignore this.npm run open-monocart-coverage
.More notes
To clarify, the problem is that when using
toIstanbul
set to true the code coverage conversion marks some lines as covered when they should not be.Let me know if you need more information.
The text was updated successfully, but these errors were encountered: