cenfun / monocart-coverage-reports

A code coverage tool to generate native V8 reports or Istanbul reports.
MIT License
69 stars 6 forks source link

[Question] Odd coverage of `catch` #41

Closed mrazauskas closed 4 months ago

mrazauskas commented 4 months ago

Minor problem. I noticed one catch marked as partially covered:

Screenshot 2024-06-15 at 14 29 54

Context. fs.watch is asynchronous api, hence signal is passed to it. Aborting the signal is the only way to close the watcher. That’s why AbortError is ignored in the catch block.

As you see code within the catch block is executed, but why it is marked as partially covered?

Hm.. or this is because of that for await loop? I mean, the try block is never executed till the end, because the loop is infinite. The execution can only jump to catch straight from for await. That could explain why there is partial mark.

mrazauskas commented 4 months ago

In the other news, I just managed to generate and upload Codacy native coverage report. Now I use monocart-coverage-reports to transform native V8 coverage instead lcov generated by c8. There is a lot of nice improvements.

Would you be interested to land a PR implementing experimental Codacy native coverage report? Uploading via API endpoint is somewhat tricky, that’s why I call it experimental. But the code itself is trivial: https://github.com/tstyche/tstyche/blob/271f1632e724503d8affbc425707de9d0ca6f59d/scripts/merge-coverage-reports.js#L10

cenfun commented 4 months ago

Minor problem. I noticed one catch marked as partially covered:

It could be an incorrect position conversion with sourcemap.

The position in the V8 coverage data should be based on generated code but not original code. We need to convert the position of generated code to the position of original code with sourcemap. In other words, it seems that it is a meaningless catch partially covered, but the generated code could be something else. It requires debugging to find out the root cause. Or could you provide a minimal reproduction or demo repo so I can help to debug it.

Would you be interested to land a PR implementing experimental Codacy native coverage report? Uploading via API endpoint is somewhat tricky, that’s why I call it experimental.

I'm interested in is that Codacy can support this feature: https://github.com/codacy/codacy-coverage-reporter/issues/506 It is similar to codecov.json report, then we could provide build-in codacy.json report too.

mrazauskas commented 4 months ago

Right. This can be caused by mapping. I check at the build file and it looks like this:

}
catch (error) {
    if (error instanceof Error && error.name === "AbortError") ;
}

I formatted the source code in the same fashion and got this:

Screenshot 2024-06-15 at 16 28 30

The end of the try block is never reached, but catch (error) is executed. And that is why } catch (error) { gets marked as partial hit in the OP example. That’s correct. Thanks for a hint.


Let’s wait for response from Codacy.

cenfun commented 4 months ago

😂 sounds reasonable, but I don't think it's possible to cover part of the word. I just debugged this issue in your project tstyche and found it is a mapping issue. open https://evanw.github.io/source-map-visualization/ and load tstyche.js and tstyche.js.map, the mapping shows image That's why letter c is uncovered.

mrazauskas commented 4 months ago

Ah.. I missed those pink marks in the report. Indeed it is a mapping issue.

To be honest, I don’t like this catch block. It works correctly, but semantics are broken. I mean, ignoring some error, because it is not error feels wrong. So now I have one more reason to rewrite this code.