Skip to content
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

Wrong coverage rate due to loss of precision #449

Open
MEZk opened this issue Sep 11, 2016 · 13 comments
Open

Wrong coverage rate due to loss of precision #449

MEZk opened this issue Sep 11, 2016 · 13 comments

Comments

@MEZk
Copy link

MEZk commented Sep 11, 2016

JaCoCo reports wrong coverage rate due to loss of precision.

Steps to reproduce

JaCoCo version: 0.7.7.201606060606
Operating system: Linux 4.4.0-21-generic #37-Ubuntu SMP x86_64 x86_64 x86_64 GNU/Linux
Tool integration: Maven
maven-jacoco-plugin configuration: jacoco-maven-plugin-config.txt

Expected behaviour

I assume that JaCoCo uses the following formulas to calculate final coverage rates:

instructions coverage rate = ((total instructions count - missed instructions count) / total instructions count) * 100%
branches coverage rate = ((total branches count - missed branches count) / total branches count) * 100%

For example in JaCoCo coverage report I have the following numbers:

total instructions count = 60501
missed instructions count = 506

and

total branches count = 7652
missed branches count = 78

So, the instructions coverage rate and branches coverage rate will be:

instructions coverage rate = ((60501 - 506) / 60501) * 100% = 0.991636501 * 100% = 99.1636501...%
branches coverage rate = ((7652 - 78) / 7652) * 100% = 0.989806586 * 100% = 98.9806586...%

Actual behaviour

When I execute mvn clean verify , JaCoCo fails the build due to the fact that coverage rate for branches is 0.98 which is lower than the specified threshold (see jacoco-maven-plugin-config.txt):

  • for instructions: 0.99
  • for branches: 0.99

However, in the coverage report I see that:
instructions coverage rate = 99%
branches coverage rate = 99%

Problem description

There is a wrong coverage rate in the report due to loss of precision.
jacoco:check phase uses correct values which are calculated with the help of formulas specified above. As a result it is unclear for user to determine where is a problem: reports look OK, but jacoco:check fails the build.

Proposal

Use equal precision in both report coverage rates and check phase.

@marchof
Copy link
Member

marchof commented Sep 12, 2016

You can specify any precision for the check goal, while the report percentage column always display percentage with 2 digit precision ("98%").

The issue here is probably more with rounding: In your example 98.9806586 is rounded up to 99%. That's why you assume a too high coverage ratio for the check.

To be on the safe side for your scenario it would help if coverage percentage in reports is always rounded down (i.e. truncated).

@MEZk
Copy link
Author

MEZk commented Sep 12, 2016

@marchof

The issue here is probably more with rounding

Yes, you are right.

To be on the safe side for your scenario it would help if coverage percentage in reports is always rounded down (i.e. truncated).

Or maybe it would be a good idea to allow user to specify whether he/she wants to use 'round down' or 'round up' scenario.

@Godin
Copy link
Member

Godin commented Sep 12, 2016

I agree with @marchof that better if report would contain truncated value to always be on safe side.

And not sure that rounding up is useful at all. @MEZk do you have an example, when it will be useful?

Also in general prefer avoiding flexibility of configuration as it increases complexity of usage, development and support.

@MEZk
Copy link
Author

MEZk commented Sep 12, 2016

@Godin
I'm completely OK to use rounding down. My point was to avoid breaking backward compatibility in reports.

@marchof
Copy link
Member

marchof commented Sep 12, 2016

To make it customizable we might add format specs to #382. Users might want to also configure decimals for percentage columns.

@Godin
Copy link
Member

Godin commented Sep 24, 2016

@marchof While I think that better if report will use RoundingMode#FLOOR by default instead of RoundingMode#HALF_EVEN as now, there are still something fishy in how check interacts with user.

Let's say I have 1 missed instruction and 199 covered, so that ratio is 0.995.
Let's set maximum value to 0.99 as check, this will lead to a message:

instructions covered ratio is 1.00, but expected maximum is 0.99

Which is IMO misleading, assuming that report will be showing 99%.

After seeing this message and without opening report, let's set 1.00 as minimum value for check, this will lead to a message:

instructions covered ratio is 0.99, but expected minimum is 1.00

IMO

  • not intuitive that RoundingMode#CEILING is used in case of specification of maximum and RoundingMode#FLOOR in case of minimum for showing of actual value
  • range in which actual value is not intuitive from messages

Also I have some serious doubts about such checks based on my practical usage, but this is another story, which deserves separate big discussion. Leaving it aside: wondering what can be done to improve current messages?

@Godin
Copy link
Member

Godin commented Sep 24, 2016

Since this discussion started from "check" and was continued, I created separate PR about rounding in HTML report - #452

@marchof
Copy link
Member

marchof commented Dec 5, 2016

@Godin What if we always show absolute figures, also for percentage limits?

@Godin
Copy link
Member

Godin commented Dec 5, 2016

Sorry @marchof but I don't understand what do you mean. Could you please elaborate using examples? Maybe even using the same as I provided.

@nhoxbypass
Copy link

Any progression on this?

I think this feature is really helpful.

@jacoco jacoco deleted a comment from jorisgeer Dec 9, 2021
@kurahaupo
Copy link

kurahaupo commented Jun 11, 2023

This seems like a good example of why "x nines" is great marketing and poor engineering. When dealing in fractions between 0.9 and 1 it makes more sense to talk in terms of the complement: the example test fails because it has 1.02%>1.00% "uncoverage" Or even better "exceeds 0.97% uncoverage limit by 0.0007%". When expressed this way the significant figures are not swamped out by useless leading nines.

Of course, you still want rounding to go in the direction that highlights when you've stepped over the limit, in this case always rounding up.

@marchof
Copy link
Member

marchof commented Jun 11, 2023

@kurahaupo This is how I also prefer to look at it and it possible with JaCoCo. The supported counters are:

  • TOTALCOUNT
  • COVEREDCOUNT
  • MISSEDCOUNT
  • COVEREDRATIO
  • MISSEDRATIO <---

See documentation.

@jorisgeer
Copy link

What I have seen is that if the reporting precision is set low, e.g. single digit, then internal rounding is done at that precision : 89% gets reported as 80%. This in turn because people specify a minimum of '0.9', blissfully unaware that this specifies both the value and the precision. As a result, coverage gets truncates to 10 percent 'clicks', including internal averaging.
I believe precision should be a separate config item, if available at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants