Bug 20357 - Pod Coverage checking for no public subs
Summary: Pod Coverage checking for no public subs
Status: Failed QA
Alias: None
Product: Project Infrastructure
Classification: Unclassified
Component: QA tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Mark Tompsett
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-07 19:27 UTC by Mark Tompsett
Modified: 2019-03-08 22:35 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20357: Correct POD Coverage checking for zero sub case (1.27 KB, patch)
2018-03-07 19:32 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20357: Correct POD Coverage checking for zero sub case (1.31 KB, patch)
2018-03-07 20:12 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2018-03-07 19:27:00 UTC
While testing bug 19929, the qa test tools complained of pod coverage. I believe this is in error. Reading the Reports.pm file, it makes no sense to complain about a changed rating when there is nothing in subs before and after.
Comment 1 Mark Tompsett 2018-03-07 19:32:17 UTC
Created attachment 72519 [details] [review]
Bug 20357: Correct POD Coverage checking for zero sub case

Currently, bug 19929 fails QA Test tools, but it
makes no sense.

TEST PLAN
---------
1) in kohaclone branch apply 19929
2) Run the following commands:
    kshell
    /home/vagrant/qa-test-tools/koha-qa.pl -v 2 -c 1
   -- This will fail with POD coverage messages.
   exit
3) apply this patch to qa-test-tools
4) re do step 2
   -- POD coverage messages will be cleared up.
Comment 2 Mark Tompsett 2018-03-07 20:12:26 UTC
Created attachment 72521 [details] [review]
Bug 20357: Correct POD Coverage checking for zero sub case

Currently, bug 19929 fails QA Test tools, but it
makes no sense.

TEST PLAN
---------
1) in kohaclone branch apply 19929
2) Run the following commands:
    kshell
    /home/vagrant/qa-test-tools/koha-qa.pl -v 2 -c 1
   -- This will fail with POD coverage messages.
   exit
3) apply this patch to qa-test-tools
4) re do step 2
   -- POD coverage messages will be cleared up.
Comment 3 Nick Clemens 2018-03-15 13:02:55 UTC
Moved to SeeAlso for 19929 to allow testing, only needed for QA
Comment 4 Marcel de Rooy 2018-03-15 14:26:40 UTC
Mark, what about:

            } elsif ( scalar @{ $after->{subs} } == 0 ) {
                # This is perfectly okay.

If there are no subs anymore, do not complain about pod coverage.
Note that no subs means no 'public' subs; there may still be subs like _type with an underscore.
Comment 5 Mark Tompsett 2018-03-15 14:39:26 UTC
(In reply to Marcel de Rooy from comment #4)
> Mark, what about:
> 
>             } elsif ( scalar @{ $after->{subs} } == 0 ) {
>                 # This is perfectly okay.
> 
> If there are no subs anymore, do not complain about pod coverage.
> Note that no subs means no 'public' subs; there may still be subs like _type
> with an underscore.

Because, if you cut out a function, I rather fall to the next check, then just allow out right cuts. I can't recall the test case I hit that made me want to do both. Probably something about increasing, then decreasing in the same patch set.
Comment 6 Hayley Pelham 2019-03-08 01:05:21 UTC
It looks like this isn't necessary anymore, at step 2 of the test plan the tests did not fail.
Comment 7 Hayley Pelham 2019-03-08 01:07:01 UTC
(In reply to Hayley Mapley from comment #6)
> It looks like this isn't necessary anymore, at step 2 of the test plan the
> tests did not fail.

Actually, on second look the first patch of 19929 did not apply cleanly. I'll request a rebase and then test this again if you deem it still necessary.
Comment 8 Hayley Pelham 2019-03-08 01:08:13 UTC
(In reply to Hayley Mapley from comment #6)
> It looks like this isn't necessary anymore, at step 2 of the test plan the
> tests did not fail.

Actually, on second look the first patch of 19929 did not apply cleanly. I'll request a rebase and then test this again if you deem it still necessary.(In reply to Hayley Mapley from comment #7)
> (In reply to Hayley Mapley from comment #6)
> > It looks like this isn't necessary anymore, at step 2 of the test plan the
> > tests did not fail.
> 
> Actually, on second look the first patch of 19929 did not apply cleanly.
> I'll request a rebase and then test this again if you deem it still
> necessary.

I now see that it's been pushed to master, so nevermind. Sorry for the noise everyone.
Comment 9 Jonathan Druart 2019-03-08 22:35:05 UTC
Please provide tests Mark.