codecov / feedback

A place to discuss feedback about the pull request and web product experience.
38 stars 8 forks source link

Incorrect line coverage | Kover == covered, codecov == miss #564

Open oxisto opened 3 weeks ago

oxisto commented 3 weeks ago

Describe the bug

There is a discrepancy in line coverage between Kotlin kover and codecov. The issue is, that the following line in the XML file produces a "miss" in codecov:

<line nr="195" mi="0" ci="0" mb="0" cb="0"/>

Where as it produces a "ok" in Kover's HTML report.

The originating problem seems to be related to @ JvmOverloads and hidden code, so Kover adds coverage information to the line, buts its neither covered nor uncovered according to the XML (but it is shown as green in the Kover HTML report). So I am not even sure if this is really a codecov issue or a Kover issue 🤦

To Reproduce Steps to reproduce the behavior:

Try to produce coverage information about a Kotlin function that uses @JvmOverloads

Expected behavior

I would assume that this line is basically ignored, but not shown as a miss

Screenshots Image Image

Additional context

It seems that this occurs also in other contexts, where there is no "real" code on a line and Kover seems to produce a similar XML.

drazisil-codecov commented 3 weeks ago

Our logic is

I can't speak to how Kover reports it in the HTML report. I also don't know whose behavior , if either, should change, Codecov's or Kover's.

This isn't a bug, but open to your thoughts if you think we should make changes. Please keep in mind that we never see the code, so we have no clue as to the contents of any lines reported. The code overlay in the UI happens in real time with your creds and direct API calls to the repo to fetch th contents.

oxisto commented 3 weeks ago

Our logic is

  • If line is in report, it's coverable
  • If line does not report having coverage: ci="0" (Covered Instructions = 0), it's called a miss.

I can't speak to how Kover reports it in the HTML report. I also don't know whose behavior , if either, should change, Codecov's or Kover's.

This isn't a bug, but open to your thoughts if you think we should make changes. Please keep in mind that we never see the code, so we have no clue as to the contents of any lines reported. The code overlay in the UI happens in real time with your creds and direct API calls to the repo to fetch th contents.

Thanks! I see where you are coming from. I am leaning towards seeing this as a bug in Kover, since this line should just not exist - or if it does exist it should contain at least 1 covered instruction.

Is there a way that codecov could ignore this „special case“? I not too familiar with the spec of the XML but I am not sure if this case should exist at all.

oxisto commented 3 weeks ago

FYI the way I see it what Kover does in the HTML interface, they call it a miss if mi > 0. it’s probably a philosophical question whether if it’s a miss if „missed instructions are greater 0“ or „covered instructions are 0“. in the idea case both should come to the same result, only in this case they do not.

drazisil-codecov commented 3 weeks ago

If both covered and missed are zero? We've have to redo the logic here https://github.com/codecov/worker/blob/9c703653fa1b78b8bc7ab8bf42c0d760c5acafae/services/report/languages/jacoco.py#L27

That would be a feature request though, not an easy change without possibly changing the coverage for other customers. So it would probably need to hide behind a flag..

https://github.com/codecov/feedback/discussions/categories/ideas