Bug 39365 - `perl -cw` should pass on all Perl files
Summary: `perl -cw` should pass on all Perl files
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 39324 39325
  Show dependency treegraph
 
Reported: 2025-03-18 11:12 UTC by Jonathan Druart
Modified: 2025-04-23 07:43 UTC (History)
2 users (show)

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


Attachments
Bug 39365: Add tests (3.18 KB, patch)
2025-03-18 11:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39365: Fix 3 files (1.49 KB, patch)
2025-03-18 11:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39365: Add tests (3.25 KB, patch)
2025-03-26 17:11 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 39365: Fix 3 files (1.57 KB, patch)
2025-03-26 17:11 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 39365: Add tests (3.31 KB, patch)
2025-04-02 07:36 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39365: Fix 3 files (1.63 KB, patch)
2025-04-02 07:36 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-03-18 11:12:34 UTC
The QA script is testing `perl -cw`. We need a test to catch regressions.
Comment 1 Jonathan Druart 2025-03-18 11:16:17 UTC
Created attachment 179445 [details] [review]
Bug 39365: Add tests

We exclude (for now) some files that needs additional work:
1. Koha/Account/Credit.pm, Koha/Account/Debit.pm and Koha/Old/Hold.pm
  "Inconsistent hierarchy during C3 merge of class"
2. misc/translator/TmplTokenizer.pm
  Can't locate VerboseWarnings.pm in @INC

We also ignore some known warnings.
Comment 2 Jonathan Druart 2025-03-18 11:16:19 UTC
Created attachment 179446 [details] [review]
Bug 39365: Fix 3 files

Those 3 were failing but an easy fix can make them pass
Comment 3 Jonathan Druart 2025-03-18 11:17:04 UTC
The tests are quite slow however. We can discuss the need of this test.
Comment 4 Jonathan Druart 2025-03-18 12:01:50 UTC

*** This bug has been marked as a duplicate of bug 37245 ***
Comment 5 Emily Lamancusa (emlam) 2025-03-21 12:45:45 UTC
Is there a new dependency here? I tried running the test and I'm getting an error:

xt/pl_valid.t .. Can't locate Test/NoWarnings.pm in @INC (you may need to install the Test::NoWarnings module)

I also get:
Parse errors: No plan found in TAP output

though I'm not sure if that's a separate error, or if it's just because the program fails before it reaches the plan tests line.
Comment 6 Jonathan Druart 2025-03-24 10:10:12 UTC
(In reply to Emily Lamancusa (emlam) from comment #5)
> Is there a new dependency here? I tried running the test and I'm getting an
> error:
> 
> xt/pl_valid.t .. Can't locate Test/NoWarnings.pm in @INC (you may need to
> install the Test::NoWarnings module)

libtest-nowarnings-perl deps has been added by bug 38944. Please pull the latest ktd images or you can also manually install the debian package.
Comment 7 Emily Lamancusa (emlam) 2025-03-26 17:11:28 UTC
Created attachment 179740 [details] [review]
Bug 39365: Add tests

We exclude (for now) some files that needs additional work:
1. Koha/Account/Credit.pm, Koha/Account/Debit.pm and Koha/Old/Hold.pm
  "Inconsistent hierarchy during C3 merge of class"
2. misc/translator/TmplTokenizer.pm
  Can't locate VerboseWarnings.pm in @INC

We also ignore some known warnings.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 8 Emily Lamancusa (emlam) 2025-03-26 17:11:30 UTC
Created attachment 179741 [details] [review]
Bug 39365: Fix 3 files

Those 3 were failing but an easy fix can make them pass

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 9 Emily Lamancusa (emlam) 2025-03-26 17:17:15 UTC
Signing off because the test suite passes with the patches. I also fixed the file permissions on pl_valid.t inline, because xt/find-misplaced-executables.t was complaining.

Testing notes (using KTD):
1. Start up KTD with selenium: ktd --selenium up
2. Install the new dependency: sudo cpanm Test::NoWarnings
3. Apply the patches.
4. Run all the tests:
   . ktd --shell --root
   . cd /kohadevbox/koha
   . perl /kohadevbox/misc4dev/run_tests.pl --run-all-tests
Comment 10 Martin Renvoize (ashimema) 2025-04-02 07:36:56 UTC
Created attachment 180240 [details] [review]
Bug 39365: Add tests

We exclude (for now) some files that needs additional work:
1. Koha/Account/Credit.pm, Koha/Account/Debit.pm and Koha/Old/Hold.pm
  "Inconsistent hierarchy during C3 merge of class"
2. misc/translator/TmplTokenizer.pm
  Can't locate VerboseWarnings.pm in @INC

We also ignore some known warnings.

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 11 Martin Renvoize (ashimema) 2025-04-02 07:36:59 UTC
Created attachment 180241 [details] [review]
Bug 39365: Fix 3 files

Those 3 were failing but an easy fix can make them pass

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 12 Martin Renvoize (ashimema) 2025-04-02 07:37:20 UTC
Great to have this.. Passing QA
Comment 13 Katrin Fischer 2025-04-23 07:43:08 UTC
Pushed for 25.05!

Well done everyone, thank you!