[BUG] Inaccurate code coverage

[edumserrano]

Hi @cenfun,

When instrumenting playwright tests for an angular app using istanbul via the babel-plugin-istanbul plugin the code coverage shows accurately whilst when using V8 + monocart-reporter some lines that should be covered show as uncovered even though there's an indication those lines have been executed X number of times.

The images below show a part of the code coverage for a file which show the problem.

As per our conversation on Playwright discord channel I've created a repo which allows you to easily reproduce the problem. Please see edumserrano/monocart-code-coverage-demo.

• Istanbul code coverage report:

• Monocart code coverage report:

[cenfun]

It should be fixed, please try new version monocart-reporter@1.6.29

The root cause is that the original location cannot be found from the sourcemap sometimes.

Sourcemap matching is always a problem, especially when some non-native js syntax is used.
For example, the following problem still cannot be solved because there is a wrong match in the sourcemap:

we can find out the mismatch with tool: https://evanw.github.io/source-map-visualization/
(please download dist file and related sourcemap file like http://localhost:4999/main.js and http://localhost:4999/main.js.map )

It seems that sourcemap related issues should be traced back to compilation tools like babel.

Thanks for such a detailed description of the problem, more suggestions are welcome.

screenshots for sourcemap issues:

[edumserrano]

Something didn't quite work for me. I updated monocart-reporter to 1.6.29 on the demo repo, then run the commands npm run test-monocart followed by npm run open-monocart-coverage and all I got on the coverage report was this:

If I rollback to 1.6.28 and run the same commands I get the coverage again (even if incorrect):

Any thoughts?

[cenfun]

please update the config tests/monocart/playwright.config.ts , the previous sourceFilter is not right

[edumserrano]

Awesome! All is working 🎉 and you made it even better by not having the webpack:/ at the start so now it starts with just src in the summary tree 👏

I do have a follow up question. In the coverage details fly out could we also remove the webpack mention from there ? (I've highlighted it on the top right of the image below)

I'd assume it would be related with the fix you did to remove webpack mention from the summary tree ?

[cenfun]

no problem.
actually, the webpack:/xxx is the url of the source file, and it will be converted to the path of the source file,
If we don't like url but prefer path, we can use path instead of url next time

[edumserrano]

I can't be sure if that change makes sense for all cases but I think it does at least for this scenario. I'd like to have it without webpack, I do think using the sourcePath makes more sense but I'll leave that up for you to decide.

To help others, I'll create a branch on the demo repo that contains the fix and update the README to mention the fix is on the branch:

• update to 1.6.29
• update the playwright.config.ts

I think you can close this issue whenever you want @cenfun . Thank you for sorting this out and so quickly 🥇

[cenfun]

just removed the webpack:/// see commit: 1e9542c
I prefer to have it without webpack:/// too.

I understand that you spend a lot of time on the code coverage, so do I.
But currently few users use this code coverage feature, suggestions are welcome.
Thanks

[edumserrano]

This might be a stupid question but the change you mentioned from commit 1e9542c will be in a new version right? 1.6.30 ?
Just checking cause I've been refreshing https://www.npmjs.com/package/monocart-reporter waiting for the new version 😆

[cenfun]

yes, 1.6.30 will be released when few minor issues are fixed. sorry for the later.

[cenfun]

1.6.30 have released. please have a try.

[edumserrano]

I can confirm version 1.6.30 is working as expected. All looking good 👏

[edumserrano]

@cenfun found some more examples where code coverage from V8+monocart-reporter is incorrect when comparing with coverage using babel+istanbul. Perhaps we should reopen this issue.

I've had to expand the code of the demo repo and created a branch named temp edumserrano/monocart-code-coverage-demo/tree/temp.

Following again the instructions on the README:

• if you run npm run test-monocart and then npm run open-monocart-coverage you will see the code coverage with V8+monocart-reporter.
• if you run npm run test-istanbul and then npm run open-istanbul-coverage you will see the code coverage with babel+istanbul.

You can see a difference in for instance:

• src/app/features/find-institution/find-institution.view-model.ts
• src/app/features/find-institution/recently-used/recently-used-institutions.service.ts

In both of the above files, the babel+istanbul shows correct code coverage as opposed to the V8+monocart-reporter version. See images below:

babel+istanbul find-institution.view-model.ts

V8+monocart-reporter find-institution.view-model.ts

The return statement that shows as uncovered should show as covered.

babel+istanbul recently-used-institutions.service.ts

V8+monocart-reporter recently-used-institutions.service.ts

The moveToFront() function and the if/else branch above show as uncovered in this image should show as covered.

Another example that shows incorrect coverage is on an html file. For this I can't compare with babel+istanbul because that setup is only capturing code coverage for TypeScript files. However, the image below from the V8+monocart-reporter setup is for the src/app/features/find-institution/institutions-list/institutions-list.component.html file and it shows as such:

Both of the above red lines should show as covered. And, similarly to the original bug reported on this issue, notice that one of the lines that shows as uncovered has the indication that it has been executed 5x.

Note

1. there are many tests now in this branch, it make take a few seconds or a minute to complete.
2. many are screenshot tests which will only work the second time you run then because I haven't pushed the screenshots to the repo. And even if you rerun there are 2 tests that will always fail (that is expected).
3. this is a temp branch to show the problem and it contains an expanded version of the code in main branch. However I'm not comfortable with leaving this branch available forever so I will be deleting it once you're able to replicate and diagnose the issue.

[cenfun]

Let's check if v8 coverage is correct in dist file first

• set excludeDistFile: false and run test, the dist file main.js will be in report
  
  

• find out what the line is in dist file for line 102 in find-institution.view-model.ts (uncovered)
  
  download http://localhost:4999/main.js and http://localhost:4999/main.js.map when server is startup.
  open them in the source-map-visualization and find out the line is 11135 in dist file:
  

• check the line 11135 and it is uncovered, so the v8 coverage should be correct.
  

thoughts

• the runtime code is different even the source code is the same, but the dist file is different
• the tests are not executed exactly the same, for example, playwright execute the tests in parallel, test1 may be executed before or after test2
• whether the page is reloaded during testing? coverage data may need to be merged. it looks complicated, need more verification.

[edumserrano]

Thank you for looking into this.

If it helps, on that temp branch for both TypeScript example files I provided, the find-institution.view-model.ts and recently-used-institutions.service.ts, the parts that show as red and should be covered are exercised by the test recently used @find-institution @recently-used at tests\monocart\recently-used.spec.ts.

Let me know if there's anything I can help with.

[cenfun]

confirmed it is a bug. the uncovered lines have not mered with covered lines some times. thanks for the example provided.

[cenfun]

It's so hard to find the root cause.
First, upgrage to monocart-reporter@1.6.34, I refactored the coverage related codes.
But still needs some changes to auto fixtures, adding one more option resetOnNavigation: false

await Promise.all([
    page.coverage.startJSCoverage({
        resetOnNavigation: false
    }),
    page.coverage.startCSSCoverage({
        resetOnNavigation: false
    }),
]);

This is because if there are multiple pages in one test, the coverage data will be reset if navigation happened

test(`recently used @find-institution @recently-used`, async ({ page }) => {
    const findInstitutionPage = new FindInstitutionPage(page);
    const chooseSignInPage = new ChooseSignInPage(page);

see resetOnNavigation

[edumserrano]

The issue on the HTML has been fixed with 1.6.34

Before

After

[edumserrano]

And the issues with the find-institution.view-model.ts and recently-used-institutions.service.ts are also fixed 🥳
Code coverage is showing properly now and I've checked all other files in my solution and couldn't find anything that was innacurate.

You were right, after upgrading to 1.6.4 I had to set the resetOnNavigation to false on the code coverage methods and coverage is now reported correctly:

await Promise.all([
    page.coverage.startJSCoverage({
        resetOnNavigation: false
    }),
    page.coverage.startCSSCoverage({
        resetOnNavigation: false
    }),
]);

Awesome work once again @cenfun 🙏

Feel free to close this issue.