cenfun / monocart-coverage-reports

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

[Bug] Lines are marked as covered in `lcovonly`, but they are not executed #37

Closed mrazauskas closed 4 months ago

mrazauskas commented 4 months ago

Describe the bug

lcovonly and v8 reports are generated. All is marked correctly in v8 (great UI, by the way):

Screenshot 2024-06-12 at 16 05 40

The lcov report is uploaded to Codacy and in their UI one can see that line which was not executed (test are running on Ubuntu, so the else branch is not reachable) got marked as covered:

Screenshot 2024-06-12 at 15 58 04

Here is data for this file from lcov.info:

SF:source/path/Path.ts
FN:8,(anonymous_0)
FN:10,(anonymous_1)
FN:14,(anonymous_2)
FN:18,(anonymous_3)
FN:22,(anonymous_4)
FN:32,(anonymous_5)
FNF:6
FNH:5
FNDA:4358,(anonymous_0)
FNDA:0,(anonymous_1)
FNDA:890,(anonymous_2)
FNDA:1303,(anonymous_3)
FNDA:849,(anonymous_4)
FNDA:612,(anonymous_5)
DA:3,293
DA:7,293
DA:8,293
DA:10,293
DA:15,890
DA:19,1303
DA:23,849
DA:25,849
DA:26,849
DA:29,849
DA:33,612
LF:11
LH:11
BRDA:7,0,0,293
BRDA:7,0,1,0
BRDA:25,1,0,849
BRDA:25,1,1,0
BRF:4
BRH:2
end_of_record
TN:

I do not understand this data unfortunately. Hopefully it helps to understand, if Codacy’s report is correct or not?

To Reproduce

Expected behavior

Same output in both reports.

Additional context

Codacy coverage report can be seen here: https://app.codacy.com/gh/tstyche/tstyche/coverage/files/117115485336?bid=42144053&fileBranchId=42144053

mrazauskas commented 4 months ago

I drop the lcov.info into https://lcov-viewer.netlify.app. Here is the result:

Screenshot 2024-06-12 at 16 12 39

Looks odd that lines can have full coverage while branches and functions are not fully covered. But, as it was mentioned, I do not understand lcov.

mrazauskas commented 4 months ago

If that is helpful, here is console-details output:

Screenshot 2024-06-12 at 16 37 28

The last line is the important one. Here lines are not reported as 100% covered and that is correct. Or?

cenfun commented 4 months ago

Seems there is a bug in lcov lines coverage, I will check it.

v8 report should be the best one, as it directly uses native V8 coverage data, while lcov needs to convert V8 to Istanbul, which may lose accuracy.

By the way, do you have the opportunity to use codecov instead of codacy, because the report codecov is directly converted from V8, it seems it is right in codecov.json image

cenfun commented 4 months ago

Yes, console-details is directly uses native V8 coverage data too.

mrazauskas commented 4 months ago

Indeed, it is tempting to switch to Codecov. I saw links in your readme and was comparing Codecov's and Codacy’s reports. Generating codecov.json directly sounds like the best solution. I was using Codecov some time ago. Hard to decide to come back, but I will think about that.

Anyway, I though it might help to report a bug as well.

mrazauskas commented 4 months ago

Interesting. I was looking through Codacy's documentation, apparently internally they have their own protocol similar to Codecov. See here: https://api.codacy.com/swagger#tocscoveragereport

It is possible to upload coverage in that format via API endpoint: https://api.codacy.com/swagger#savecoverage

I will drop them a line asking, if they could make it possible uploading codacy.json via CLI as well. Or I could write custom reporter (looks rather trivial task, did I find the right lines?) and simply use curl. That sounds fun ;D

cenfun commented 4 months ago

Good found, I didn't notice this before. Maybe I could provide build-in codacy.json report for MCR next time. Please feel free to write and use custom report. It should be easy, see example

{
    reports: [
        [path.resolve('./test/custom-v8-reporter.js'), {
            type: 'v8',
            outputFile: 'custom-v8-coverage.json'
        }]
    ]
}

https://github.com/cenfun/monocart-coverage-reports/blob/main/test/custom-v8-reporter.js

codecov example: handleCodecovReport https://github.com/cenfun/monocart-coverage-reports/blob/main/lib/generate.js#L263

cenfun commented 4 months ago

Good news is I found out the root cause which is replated to static initialization blocks There are different range between v8 and AST, so we haven't got the right line coverage in lcov. image I need to figure out how to fix it and may require more time.

mrazauskas commented 4 months ago

Thanks! Meanwhile I have send a feature request to Codacy: https://github.com/codacy/codacy-coverage-reporter/issues/506

mrazauskas commented 4 months ago

@cenfun I have just spotted another issue related with static initialization blocks. Might be it already got fixed in the commit you have push.

Here is v8 coverage. Look at calls count in the static block. 4358 initializations does not sound realistic, because there are only 331 test.

Below you can find lcov generated by c8. Funny result for line 5, but otherwise 293 initializations is the number I would expect:

Screenshot 2024-06-13 at 14 58 03
Screenshot 2024-06-13 at 14 58 26
mrazauskas commented 4 months ago

Ah.. Here raw data.lines:

{
  "1": 1,
  "3": 1,
  "4": 293,  // correct
             // skipped line 5, so much correct!
  "6": 293,  // correct
  "7": 293,  // correct
  "8": 4358,  // hm.. and what is this?
  "9": "1/2",
  "10": 0,
  "11": 0,
  "12": 293,
  "14": 890,
  "15": 890,
  "16": 890,
  "18": 1303,
  "19": 1303,
  "20": 1303,
  "22": 849,
  "23": 849,
  "25": 849,
  "26": 849,
  "27": 849,
  "29": 849,
  "30": 849,
  "32": 612,
  "33": 612,
  "34": 612,
  "35": 1,
}
cenfun commented 4 months ago

Thanks! Meanwhile I have send a feature request to Codacy: https://github.com/codacy/codacy-coverage-reporter/issues/506

Great. I've also tried uploading a Codacy-formatted JSON but wasn't successful. Otherwise, we could provide built-in Codacy report, just like Codecov.

I have just spotted another issue related with static initialization blocks. Might be it already got fixed in the commit you have push.

Yes. It should be fixed, see commit. A patch will be released, and I still have several other issues to solve together.

Here is v8 coverage. Look at calls count in the static block. 4358 initializations does not sound realistic, because there are only 331 test. Below you can find lcov generated by c8. Funny result for line 5, but otherwise 293 initializations is the number I would expect

c8 used v8-to-istanbul to convert the format to istanbul from V8, It's an excellent initiative to implement native V8 coverage reports. At the beginning, I also used v8-to-istanbul to implement the conversion. However, there are some issues that it seems unable to resolve. Because it just provide a simple conversion, not an accurate one. So I try to implement a better conversion which is analyzing the AST of the source code to achieve more accurate conversion. for example, excluding comments lines, blank lines and so on. see here for more history.

BTW, I have contributed MCR to c8 as an experimental feature, you can use it like below in the latest version.

c8 --experimental-monocart --reporter=v8 --reporter=console-details node foo.js
cenfun commented 4 months ago

@mrazauskas It should be fixed. Please try monocart-coverage-reports@2.8.4

mrazauskas commented 4 months ago

Thanks. Yes, it works now. Closing this issue.

I hope to find some time to play with Codacy’s API today or tomorrow. Let’s see.

And thanks once again for creating this library!

cenfun commented 4 months ago

@mrazauskas Sorry, seems that I missed a question.

{
  "1": 1,
  "3": 1,
  "4": 293,  // correct
             // skipped line 5, so much correct!
  "6": 293,  // correct
  "7": 293,  // correct
  "8": 4358,  // hm.. and what is this?
  "9": "1/2",

image

For line 8, there are two parts:

Currently, it take the maximum hits as the hits of the line. Does this make sense?

mrazauskas commented 4 months ago

Ah.. That makes sense now. I thought function executions will be marked on line 4. If V8 reports these on line 8, that is perfectly fine.