Sort files on actual uncovered percentage

[hoxell]

This fixes:

• Files with covered = 0 and total = 1 and 100% get equal sorting value (1), thus alphabetically interleaving files with 0% and 100% coverage.

• Files with covered = 0 and total > 1 end up after files with 100% coverage (or before, if reverse sorting is enabled), but before files that have 100% coverage because total = 0.

In terms of coloring, this change leads to color-consistent sorting (red-yellow-green, or green-yellow-red for reverse), but not the current interleaving of colors and coverage percentages.

[Spacetown]

The intention of the logic was to have the uncovered branches sorted by the total number of branches instead of alphabetical order. So it's more a change than a fix. To you have a screenshot before/after your modification?

[Spacetown]

Taking a look at the test results and thinking about it I don't get the reason for having the special case for uncovered. @latk can you recall this?

  --- reference/gcc-8/coverage.txt
  +++ coverage.txt
  @@ -4,11 +4,11 @@
   ------------------------------------------------------------------------------
   File                                       Lines    Exec  Cover   Missing
   ------------------------------------------------------------------------------
  +file4.cpp                                      2       0     0%   1,3
   file3.cpp                                      9       4    44%   8,10-12,14
   file2.cpp                                      7       4    57%   8,10-11
   file1.cpp                                      4       3    75%   4
   main.cpp                                       5       5   100%
  -file4.cpp                                      2       0     0%   1,3
   ------------------------------------------------------------------------------
   TOTAL                                         27      16    59%
   ------------------------------------------------------------------------------

[hoxell]

Here's a stripped down example of the type of sorting the current logic yields:

The various groups resulting from the branching in key_percent_uncovered(...) are sorted among themselves, but not with respect to each other, which leads to the interleaving behavior we're seeing.

It occurred to me that it would be convenient to sort the files based on the number of lines instead of the file name, but I didn't want to change the behavior. Hearing that that's the intended behavior, though, I've updated this PR such that for custom sorting, the selected sorting key is used as the primary key, total is used as the secondary key, and the filename is used as the tertiary key. This leads the sorting being correct with respect to coverage and between the previously mentioned "groups", but I believe that it also makes the overall sorting more intuitive within the groups with the same primary sorting key value.

Here's an example:

Reverse sorting example:

[latk]

I don't get the reason for having the special case for uncovered. @latk can you recall this?

Not sure either :)

It seems that in 0418545 the sign of a sorting key got flipped, unclear to me whether that was an accident or intentional.

I think at some point the intention of that code was to ensure that 0/1 branches and 0/123 branches are not both sorted as "0%", but effectively as -1% and -123% – more uncovered is worse. However, 0/0 branches should look more like 100%. Under no circumstances should there be NaNs or ZeroDivisionErrors.

I suspect that this inconsistency wasn't discovered previously because gcovr depends almost exclusively on end-to-end tests, which are high-fidelity but make it difficult to spot relevant changes in behaviour. Perhaps this PR could add unit tests to demonstrate the expected sort orders under different sort modes.

[Spacetown]

I think the -1 was removed because of the introduction of the reversed. Before this commit the sort order was from highest to lowest percentage by using this -1, see #817 (comment):

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
main.cpp                                       6       6   100%   
file1.cpp                                      4       3    75%   4
file2.cpp                                      7       4    57%   8,10-11
file3.cpp                                      9       4    44%   8,10-12,14
file4.cpp                                      2       0     0%   1,3
------------------------------------------------------------------------------
TOTAL                                         28      17    60%
------------------------------------------------------------------------------

was changed to:

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
file3.cpp                                      9       4    44%   8,10-12,14
file2.cpp                                      7       4    57%   8,10-11
file1.cpp                                      4       3    75%   4
main.cpp                                       6       6   100%
file4.cpp                                      2       0     0%   1,3
------------------------------------------------------------------------------
TOTAL                                         28      17    60%
------------------------------------------------------------------------------

After thinking about it more closely, the specific case of covered doesn't make any sense to me. The same for the usage of the total number of branches if uncovered.

[Spacetown]

@hoxell Can you update the reference data by running the docker containers local? I'll update the description for the sort options since the still reference the deprecated options and push this to your branch..

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.70%. Comparing base (5f86dcb) to head (db71f54).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #918      +/-   ##
==========================================
+ Coverage   94.66%   94.70%   +0.04%     
==========================================
  Files          50       50              
  Lines        3916     3911       -5     
  Branches      826      824       -2     
==========================================
- Hits         3707     3704       -3     
+ Misses        130      129       -1     
+ Partials       79       78       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Spacetown]

LGFM. Thanks for fixing this.

[Pesa]

is this change expected?

[Spacetown]

I also saw this when creating the single file report and fixed the error in the scrubber there. See tests/test_gcovr.py in https://github.com/gcovr/gcovr/pull/916/files#diff-14fc10b334e44e45733e71ed3f0d748422b8f8dedc6078c92f2ebca731072ccc.