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

Calculation of Branch Coverage Totals is wrong - or at least misleading - when untested code exists #1007

Open
theseer opened this issue Aug 8, 2023 · 3 comments

Comments

@theseer
Copy link
Contributor

theseer commented Aug 8, 2023

Q A
php-code-coverage version 10.1.3
PHP version 8.2.9
Driver Xdebug
Xdebug version (if used) 3.2.2
Installation Method PHPUnit PHAR
Usage Method PHPUnit
PHPUnit version (if used) 10.3.1

image

The above 100% - 5/5 total for serializer is wrong, as the next screenshot shows:

image

The 100% - 5/5 total is only correct for the tested/covered files of this folder. Given there are untested/uncovered files present, that total should be less than 100%, and whatever the amount of branches might be ;)

I do realize we probably do not have that information (hence the n/a). But then we probably shouldn't calculate the total?

@sebastianbergmann
Copy link
Owner

@dvdoug Do you have any insight on this? Thanks!

@dvdoug
Copy link
Contributor

dvdoug commented Aug 10, 2023

I do realize we probably do not have that information (hence the n/a)

Yes, it's exactly that - the branches/paths are calculated by Xdebug, so if a file isn't processed then it's unknown (N.B. processed does not mean tested, merely loaded is sufficient). For the purposes of the statistical calculations, unknown is treated as 0 (see https://github.com/sebastianbergmann/php-code-coverage/blob/main/src/Node/File.php#L393 etc)

For v9, anyone who had processUncoveredFiles set to true wouldn't encounter this, as all files would be include()d but that setting doesn't exist anymore.

But then we probably shouldn't calculate the total?

I wouldn't recommend that, because that would mean that the root-level stats would display n/a if there was even a single uncovered file in the entire report - but maybe the number could be displayed with an asterisk and a tooltip that explains it's based on partial data?

@sebastianbergmann
Copy link
Owner

@dvdoug Thank you for your insight!

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

No branches or pull requests

3 participants