pjcj / Devel--Cover

Code coverage metrics for Perl
http://www.pjcj.net/perl.html
93 stars 89 forks source link

Feature to add completely uncovered files to the coverage report #335

Open nwc10 opened 5 months ago

nwc10 commented 5 months ago

In the generated report, Devel::Cover completely ignores files which have no coverage (because no part of the test suite ever ran them). For our use case, this makes the report a bit deceptive and dangerous because it gives the impression that we have reasonable coverage, when actually we have scary big holes, where we shouldn't be refactoring (yet)

So I wondereded if it's possible to force uncovered files to be treated as 0%. I asked about this on the london.pm list but it seems no-one had attempted this. So here goes...

I hacked together a script for work - cover-touch.pl

Without this the coverage report looks like this:

                  file                 stmt  bran  cond   sub   pod  time  total
  bin/batch_monitoring.pl              98.1  83.3  n/a   93.3  n/a   0.3   96.0
  bin/check_payment_notification.pl    98.4  75.0  44.4  93.7  n/a   0.1   86.2
  bin/data_export_to_storage.pl        89.6  37.5  n/a   88.2  n/a   0.5   84.3
  bin/monitor_payment_files.pl         93.1  75.0  n/a   100.0 n/a   0.0   91.6
  bin/run_shasum.pl                    97.7  75.0  n/a   92.8  n/a   0.0   95.2
  bin/statement_monitoring.pl          90.0  78.5  47.6  89.4  n/a   0.6   81.4
  lib/Processor/BankStatement.pm       98.2  75.0  66.6  100.0 100.0 0.1   95.1

...

  lib/app/StatementMonitoring/UK.pm    100.0 n/a   n/a   100.0 0.0   0.4   85.0
  lib/logic/BankingDays.pm             98.0  91.6  100.0 100.0 0.0   1.0   94.7
  lib/notify_slack.pl                  92.0  50.0  n/a   100.0 n/a   0.3   88.5
  lib/send_sms.pl                      97.5  93.7  100.0 100.0 n/a   0.3   96.7
  Total                                94.9  74.5  58.1  95.8  11.4  100.0 88.7

With this:

                  file                 stmt  bran  cond   sub   pod  time  total
  bin/batch_monitoring.pl              98.1  83.3  n/a   93.3  n/a   0.3   96.0
  bin/check_bank_holiday.pl            n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/check_payment_notification.pl    98.4  75.0  44.4  93.7  n/a   0.1   86.2
  bin/data_export_to_storage.pl        89.6  37.5  n/a   88.2  n/a   0.6   84.3
  bin/gen_directories.pl               n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/import_statement_from_csv.pl     n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/manual_fail_rbs_transaction.pl   n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/monitor_payment_files.pl         93.1  75.0  n/a   100.0 n/a   0.0   91.6
  bin/next_bank_holiday.pl             n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/rbs_narrative_soaker.pl          n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/read_links.pl                    n/a   n/a   n/a   n/a   n/a   n/a   0.0
  bin/run_shasum.pl                    97.7  75.0  n/a   92.8  n/a   0.0   95.2
  bin/statement_monitoring.pl          90.0  78.5  47.6  89.4  n/a   0.7   81.4
  lib/Processor/BankStatement.pm       98.2  75.0  66.6  100.0 100.0 0.1   95.1

...

  lib/app/StatementMonitoring/UK.pm    100.0 n/a   n/a   100.0 0.0   0.5   85.0
  lib/commify.pl                       n/a   n/a   n/a   n/a   n/a   n/a   0.0
  lib/logic/BankingDays.pm             98.0  91.6  100.0 100.0 0.0   1.0   94.7
  lib/notify_slack.pl                  92.0  50.0  n/a   100.0 n/a   0.3   88.5
  lib/send_sms.pl                      97.5  93.7  100.0 100.0 n/a   0.3   96.7
  Total                                94.9  74.5  58.1  95.8  11.4  100.0 88.7

we see that the UNKNOWN UNKNOWNS are now improved to KNOWN UNKNOWNS.

(Output stolen from the talk http://act.yapc.eu/gpw2024/talk/7872 - video not online yet)

I have no idea if this is generating "correct" (let alone "optimal") cover_db runs, but it does seem to do the job. However, the report generation code then spews bazillions of use of uninitialized value warnings because some part of a data structure is (partially) created. It is not expecting this - it appears that the "outer" level reference that now exists for that file causes code to be entered that wan't previously, but this code then assumes a multi-level structure exists with values for statement, branch and condition coverage, and so is reading undef when it expects a number.

This patch "shuts it up" but I don't think that it's really correct, as we end up with the "average number" for the file being 0.0 (seen above) whereas I'd like it to be n/a

diff --git a/lib/perl5/site_perl/5.32.0/x86_64-linux/Devel/Cover/Report/Html_minimal.pm b/lib/perl5/site_perl/5.32.0/x86_64-linux/Devel/Cover/Report/Html_minimal.pm
index ede35908d..5ffa77f0f 100644
--- a/lib/perl5/site_perl/5.32.0/x86_64-linux/Devel/Cover/Report/Html_minimal.pm
+++ b/lib/perl5/site_perl/5.32.0/x86_64-linux/Devel/Cover/Report/Html_minimal.pm
@@ -482,10 +482,14 @@ sub print_file_report {
     my ($show, $th) = get_showing_headers($db, $opt);
     my $file_data   = $db->cover->file($fin);

+    # If we have injected files into the DB with no coverage, we don't want to
+    # accidentally autovivify a "total" structure for them, as creating the
+    # hashref here "confuses" get_summary_for_file().
+    my $total = $db->{summary}{$fin}{total};
     print_html_header($out, "File Coverage: $fin");
     print_summary($out, 'File Coverage', $fin,
-                  $db->{summary}{$fin}{total}{percentage},
-                  $db->{summary}{$fin}{total}{error},
+                  $total->{percentage},
+                  $total->{error},
                   $db);
     print_th($out, ['line', @$th, 'code']);

(I believe there's at least one talk on YouTube that says work is on 5.32.0 ("this week"), so this isn't a leak)

I'm not sure where to take this from here. I guess

I don't know how to submit a patch to do any of this. Or even if it's the right plan.

Right now (I think) if I have 5 files with totals of 0.6, 0.7, 0.8, 0.9 and 1.0, and 5 files completly uncovered, my overal "average" is 0.8, which is highlighted as orange. With my hack, those 5 files will now show up, but the averaging ignores them. I'd rather like the averaging to treat the overal totals as 0.4, which is very red.

It's actually a bit of a "bug" - right now if I delete the test for a module with low coverage, my reported average coverage goes up. Game the system!

nwc10 commented 5 months ago

Sorry, was not clear - I think that the "feature" cover would need is "treat this list of files on the CLI as having 0 coverage".

Scanning recusively, filtering and so on is the job of script or human invoking it, as I don't think it's possible to create a sufficiently generalised filter rule language for the command line. My example happens to have filtering because that's what I needed. I would assume I'd re-write it as my code doing filtering to get a list of files, and then shell out to cover with that list.

(OK, possibly whichever globbing syntax it is that includes ** to mean "any number of levels of directory" might just make it work self-contained - by implication that would be in combination with all the usual glob contructions, particularly comma seperated lists in { }. But I doubt that is easy to implement without dragging in more dependencies)