Bug 38944 - Add Test::NoWarnings to tests without warnings
Summary: Add Test::NoWarnings to tests without warnings
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25515
  Show dependency treegraph
 
Reported: 2025-01-22 13:30 UTC by Jonathan Druart
Modified: 2025-05-09 13:56 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds the Test::NoWarnings module to all the tests that currently do not produce warnings. This will then be a good starting point to move toward no warnings in our tests, with the next step to clean the other tests that currently do provide warnings.
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 38944: Add new dependency Test::NoWarnings (727 bytes, patch)
2025-01-22 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (303.03 KB, patch)
2025-01-22 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Manual changes (757 bytes, patch)
2025-01-22 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add new dependency Test::NoWarnings (727 bytes, patch)
2025-01-27 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (303.03 KB, patch)
2025-01-27 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Manual changes (757 bytes, patch)
2025-01-27 09:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Test summary report - 2025-01-28 (10.91 KB, text/plain)
2025-01-27 18:02 UTC, David Nind
Details
Bug 38944: Manual changes (18.97 KB, patch)
2025-01-28 14:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.57 KB, patch)
2025-01-28 14:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (303.03 KB, patch)
2025-01-30 20:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Manual changes (18.97 KB, patch)
2025-01-30 20:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.57 KB, patch)
2025-01-30 20:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Revert changes to selenium/01-installation.t (1.13 KB, patch)
2025-01-31 12:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add new dependency Test::NoWarnings (773 bytes, patch)
2025-02-03 19:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (301.78 KB, patch)
2025-02-03 19:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 38944: Manual changes (18.95 KB, patch)
2025-02-03 19:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.59 KB, patch)
2025-02-03 19:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 38944: Revert changes to selenium/01-installation.t (1.17 KB, patch)
2025-02-03 19:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 38944: Add new dependency Test::NoWarnings (783 bytes, patch)
2025-02-12 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (301.48 KB, patch)
2025-02-12 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Manual changes (18.99 KB, patch)
2025-02-12 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.70 KB, patch)
2025-02-12 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Revert changes to selenium/01-installation.t (1.04 KB, patch)
2025-02-12 14:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Buug 38944: Revert changes to t/00-testcritic.t (1.39 KB, patch)
2025-02-12 21:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add new dependency Test::NoWarnings (839 bytes, patch)
2025-02-13 12:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (301.54 KB, patch)
2025-02-13 12:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Manual changes (19.05 KB, patch)
2025-02-13 12:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.75 KB, patch)
2025-02-13 12:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Revert changes to selenium/01-installation.t (1.09 KB, patch)
2025-02-13 12:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Revert changes to t/00-testcritic.t (1.44 KB, patch)
2025-02-13 12:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38944: Add new dependency Test::NoWarnings (839 bytes, patch)
2025-03-13 13:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Add Test::NoWarnings to tests no producing warnings (301.60 KB, patch)
2025-03-13 13:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Manual changes (19.05 KB, patch)
2025-03-13 13:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Correctly catch warnings raised by DBI (24.75 KB, patch)
2025-03-13 13:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Revert changes to selenium/01-installation.t (1.09 KB, patch)
2025-03-13 13:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Revert changes to t/00-testcritic.t (1.44 KB, patch)
2025-03-13 13:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Remove warning from Patron/Borrower_Debarments. (879 bytes, patch)
2025-03-13 13:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Fix tidiness t/db_dependent/check_kohastructure.t (914 bytes, patch)
2025-03-17 08:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Tidy t/db_dependent/Record/Record.t (881 bytes, patch)
2025-03-19 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38944: Fix for Mysql8 (2.16 KB, patch)
2025-05-06 14:09 UTC, Jonathan Druart
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-01-22 13:30:34 UTC
To prevent new warnings for test files that do not generate warnings we should use Test::NoWarnings.
Comment 1 Jonathan Druart 2025-01-22 13:44:20 UTC
Created attachment 176912 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings
Comment 2 Jonathan Druart 2025-01-22 13:44:24 UTC
Created attachment 176913 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.
Comment 3 Jonathan Druart 2025-01-22 13:44:27 UTC
Created attachment 176914 [details] [review]
Bug 38944: Manual changes

Not caught by the script
Comment 4 Jonathan Druart 2025-01-22 13:45:45 UTC
The idea here is to add Test::NoWarnings to all the tests that currently do not produce warnings. So we have a good starting point to move toward no warnings in our tests.

The next step will be to clean the other tests that provide warnings.

Test plan:
The test suite should still pass after those patches.
Comment 5 Brendan Lawlor 2025-01-24 16:44:16 UTC
I tried testing, but I think Test::NoWarnings module needs to be added to ktd to be able to be able to test.
Comment 6 Jonathan Druart 2025-01-24 19:49:06 UTC
Yes you can install it using `cpanm Test::NoWarnings`
Comment 7 David Nind 2025-01-25 09:34:13 UTC
The patch no longer applies cleanly:

....
Auto-merging t/db_dependent/UsageStats.t
CONFLICT (content): Merge conflict in t/db_dependent/UsageStats.t
Auto-merging t/db_dependent/Koha/Biblio.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 38944: Add Test::NoWarnings to tests no producing warnings
......

Testing notes (using KTD):
1. Start up KTD with selenium: ktd --selenium up
2. Install the new dependency: 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 8 Jonathan Druart 2025-01-27 09:49:20 UTC
Created attachment 177165 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings
Comment 9 Jonathan Druart 2025-01-27 09:49:23 UTC
Created attachment 177166 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.
Comment 10 Jonathan Druart 2025-01-27 09:49:26 UTC
Created attachment 177167 [details] [review]
Bug 38944: Manual changes

Not caught by the script
Comment 11 David Nind 2025-01-27 18:02:27 UTC
Created attachment 177196 [details]
Test summary report - 2025-01-28

I've attached the tests summary showing the failures from running all the tests.

Testing notes (updated) (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 12 Jonathan Druart 2025-01-28 14:51:16 UTC
Created attachment 177274 [details] [review]
Bug 38944: Manual changes

Not caught by the script
Comment 13 Jonathan Druart 2025-01-28 14:51:19 UTC
Created attachment 177275 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI
Comment 14 Tomás Cohen Arazi (tcohen) 2025-01-30 17:33:37 UTC
This is pretty awesome. I've always done this. 

Thanks!
Comment 15 David Nind 2025-01-30 19:22:00 UTC
Thanks Jonatahn!

I was too slow re-testing, and the patches no longer apply ...

git bz apply 38944

Bug 38944 - Add Test::NoWarnings to tests without warnings

177165 - Bug 38944: Add new dependency Test::NoWarnings
177166 - Bug 38944: Add Test::NoWarnings to tests no producing warnings
177274 - Bug 38944: Manual changes
177275 - Bug 38944: Correctly catch warnings raised by DBI

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 38944: Add new dependency Test::NoWarnings
Applying: Bug 38944: Add Test::NoWarnings to tests no producing warnings
Using index info to reconstruct a base tree...
M	t/Languages.t
M	t/db_dependent/Koha/CirculationRules.t
M	t/db_dependent/Koha/Holds.t
M	t/db_dependent/Koha/Object/Mixin/AdditionalFields.t
M	t/db_dependent/Languages.t
M	t/db_dependent/api/v1/items.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/api/v1/items.t
Auto-merging t/db_dependent/Languages.t
Auto-merging t/db_dependent/Koha/Object/Mixin/AdditionalFields.t
Auto-merging t/db_dependent/Koha/Holds.t
Auto-merging t/db_dependent/Koha/CirculationRules.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/CirculationRules.t
Auto-merging t/Languages.t
CONFLICT (content): Merge conflict in t/Languages.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 38944: Add Test::NoWarnings to tests no producing warnings
Comment 16 Jonathan Druart 2025-01-30 20:25:20 UTC
Created attachment 177362 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.
Comment 17 Jonathan Druart 2025-01-30 20:25:23 UTC
Created attachment 177363 [details] [review]
Bug 38944: Manual changes

Not caught by the script
Comment 18 Jonathan Druart 2025-01-30 20:25:26 UTC
Created attachment 177364 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI
Comment 19 David Nind 2025-01-30 21:26:26 UTC
Thanks Jonathan!

Still a few test failures:

Test Summary Report
-------------------
t/db_dependent/selenium/01-installation.t                                 (Wstat: 65280 (exited 255) Tests: 2 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 3 tests but ran 2.
t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t                  (Wstat: 2816 (exited 11) Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 11
  Parse errors: Bad plan.  You planned 5 tests but ran 3.
t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t                   (Wstat: 5632 (exited 22) Tests: 4 Failed: 0)
  Non-zero exit status: 22
  Parse errors: Bad plan.  You planned 14 tests but ran 4.
t/db_dependent/api/v1/erm_sushi_services.t                                (Wstat: 256 (exited 1) Tests: 2 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
xt/find-misplaced-executables.t                                           (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
t/00-testcritic.t                                                         (Wstat: 0 Tests: 3245 Failed: 1)
  Failed test:  3245
  Parse errors: Plan (1..3244) must be at the beginning or end of the TAP output
                Bad plan.  You planned 3244 tests but ran 3245.
xt/verify-yarnlock.t                                                      (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
xt/find-license-problems.t                                                (Wstat: 256 (exited 1) Tests: 1378 Failed: 1)
  Failed test:  814
  Non-zero exit status: 1
Files=733, Tests=18916, 2790.29 wallclock secs ( 5.20 usr  1.93 sys + 2999.38 cusr 571.04 csys = 3577.55 CPU)
Result: FAIL
Comment 20 Jonathan Druart 2025-01-31 12:47:28 UTC
Created attachment 177380 [details] [review]
Bug 38944: Revert changes to selenium/01-installation.t
Comment 21 Jonathan Druart 2025-01-31 12:47:41 UTC
(In reply to David Nind from comment #19)
> Thanks Jonathan!
> 
> Still a few test failures:
> 
> Test Summary Report
> -------------------
> t/db_dependent/selenium/01-installation.t                                
> (Wstat: 65280 (exited 255) Tests: 2 Failed: 0)
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 3 tests but ran 2.

Should be fixed by the last patch.

> t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t                 
> (Wstat: 2816 (exited 11) Tests: 3 Failed: 1)
>   Failed test:  2
>   Non-zero exit status: 11
>   Parse errors: Bad plan.  You planned 5 tests but ran 3.
> t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t                  
> (Wstat: 5632 (exited 22) Tests: 4 Failed: 0)
>   Non-zero exit status: 22
>   Parse errors: Bad plan.  You planned 14 tests but ran 4.

They are passing for me. Do they pass on main?

> t/db_dependent/api/v1/erm_sushi_services.t                               
> (Wstat: 256 (exited 1) Tests: 2 Failed: 1)
>   Failed test:  1
>   Non-zero exit status: 1

This is currently failing on main.

> xt/find-misplaced-executables.t                                          
> (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
>   Failed test:  1
>   Non-zero exit status: 1
> t/00-testcritic.t                                                        
> (Wstat: 0 Tests: 3245 Failed: 1)
>   Failed test:  3245
>   Parse errors: Plan (1..3244) must be at the beginning or end of the TAP
> output
>                 Bad plan.  You planned 3244 tests but ran 3245.
> xt/verify-yarnlock.t                                                     
> (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
>   Failed test:  1
>   Non-zero exit status: 1
> xt/find-license-problems.t                                               
> (Wstat: 256 (exited 1) Tests: 1378 Failed: 1)
>   Failed test:  814
>   Non-zero exit status: 1
> Files=733, Tests=18916, 2790.29 wallclock secs ( 5.20 usr  1.93 sys +
> 2999.38 cusr 571.04 csys = 3577.55 CPU)
> Result: FAIL

What's the output of those ones? This patchset does not modify them so I guess they also fail on main.
Comment 22 David Nind 2025-02-01 00:00:57 UTC
Hi Jonathan.

These are the only tests that are failing now if I run all the tests at once.

Some of the previous errors were because I had a folder with kidclamp's handy Koha scripts.

Test Summary Report
-------------------
t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t                  (Wstat: 2816 (exited 11) Tests: 3 Failed: 1)
  Failed test:  2
  Non-zero exit status: 11
  Parse errors: Bad plan.  You planned 5 tests but ran 3.
t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t                   (Wstat: 65280 (exited 255) Tests: 4 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 14 tests but ran 4.
xt/verify-yarnlock.t                                                      (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
t/00-testcritic.t                                                         (Wstat: 0 Tests: 3245 Failed: 1)
  Failed test:  3245
  Parse errors: Plan (1..3244) must be at the beginning or end of the TAP output
                Bad plan.  You planned 3244 tests but ran 3245.
Files=733, Tests=18914, 2958.36 wallclock secs ( 5.49 usr  1.96 sys + 3076.34 cusr 577.41 csys = 3661.20 CPU)
Result: FAIL

1. Elasticsearch tests
======================

Both the tests fail for me when I run all the tests after applying the patches, etc.

However, after the patches are applied, if I shutdown and restart KTD (and then add the module), the tests pass when I run them individually.

2. xt/verify-yarnlock.t
=======================

Fails for me before and after the patches. Lots of warnings and errors.

If I do a yarn install and yarn build, then run the test individually, it passes.

.....
warning "arraybuffer.prototype.slice#function.prototype.name#call-bind#get-intrinsic##has-symbols@^1.0.1" could be deduped from "1.0.3" to "has-symbols@1.0.3"
warning "string.prototype.trim#function.prototype.name#call-bind#get-intrinsic##function-bind@^1.1.1" could be deduped from "1.1.1" to "function-bind@1.1.1"
warning "string.prototype.trim#function.prototype.name#call-bind#get-intrinsic##has@^1.0.3" could be deduped from "1.0.3" to "has@1.0.3"
warning "string.prototype.trim#function.prototype.name#call-bind#get-intrinsic##has-symbols@^1.0.1" could be deduped from "1.0.3" to "has-symbols@1.0.3"
error Found 54 errors.
xt/verify-yarnlock.t .. 1/1 
#   Failed test 'verify yarn.lock file is updated correctly'
#   at xt/verify-yarnlock.t line 45.
#          got: '256'
#     expected: '0'
# Looks like you failed 1 test of 1.
xt/verify-yarnlock.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 

Test Summary Report
-------------------
xt/verify-yarnlock.t (Wstat: 256 (exited 1) Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  3 wallclock secs ( 0.04 usr  0.02 sys +  2.94 cusr  1.13 csys =  4.13 CPU)
Result: FAIL

3. t/00-testcritic.t
====================

Fails for me before and after the patches.

prove t/00-testcritic.t 
t/00-testcritic.t .. All 3244 subtests passed 

Test Summary Report
-------------------
t/00-testcritic.t (Wstat: 0 Tests: 3245 Failed: 1)
  Failed test:  3245
  Parse errors: Plan (1..3244) must be at the beginning or end of the TAP output
                Bad plan.  You planned 3244 tests but ran 3245.
Files=1, Tests=3245, 30 wallclock secs ( 0.71 usr  0.04 sys + 224.51 cusr  1.36 csys = 226.62 CPU)
Result: FAIL
Comment 23 David Nind 2025-02-03 19:47:09 UTC
Created attachment 177477 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2025-02-03 19:47:13 UTC
Created attachment 177478 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 David Nind 2025-02-03 19:47:16 UTC
Created attachment 177479 [details] [review]
Bug 38944: Manual changes

Not caught by the script

Signed-off-by: David Nind <david@davidnind.com>
Comment 26 David Nind 2025-02-03 19:47:19 UTC
Created attachment 177480 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 David Nind 2025-02-03 19:47:21 UTC
Created attachment 177481 [details] [review]
Bug 38944: Revert changes to selenium/01-installation.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 David Nind 2025-02-03 19:49:08 UTC
Now signed off - see comments on chat https://chat.koha-community.org/koha-community/pl/ck6muapewi8p5e87ehsefyii5c

"..it seems that you can signoff, you don't have more errors after the patches. The failures you get on main can be caused by several reasons (git repo not clean, node_modules not clean, ktd started withou a --es flag). Hard to tell but they do not seem related to this patch"
Comment 29 Jonathan Druart 2025-02-12 14:54:18 UTC
Created attachment 177928 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings

Signed-off-by: David Nind <david@davidnind.com>
Comment 30 Jonathan Druart 2025-02-12 14:54:21 UTC
Created attachment 177929 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.

Signed-off-by: David Nind <david@davidnind.com>
Comment 31 Jonathan Druart 2025-02-12 14:54:24 UTC
Created attachment 177930 [details] [review]
Bug 38944: Manual changes

Not caught by the script

Signed-off-by: David Nind <david@davidnind.com>
Comment 32 Jonathan Druart 2025-02-12 14:54:27 UTC
Created attachment 177931 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI

Signed-off-by: David Nind <david@davidnind.com>
Comment 33 Jonathan Druart 2025-02-12 14:54:30 UTC
Created attachment 177932 [details] [review]
Bug 38944: Revert changes to selenium/01-installation.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 34 Jonathan Druart 2025-02-12 21:19:00 UTC
Created attachment 177977 [details] [review]
Buug 38944: Revert changes to t/00-testcritic.t

Not sure what's happening here. Skipping it for now.

 t/00-testcritic.t .. 3246/?
 #   Failed test 'done_testing() was already called at /usr/share/perl5/Test/Perl/Critic.pm line 105'
 #   at t/00-testcritic.t line 22.
 t/00-testcritic.t .. All 3249 subtests passed

 Test Summary Report
 -------------------
 t/00-testcritic.t (Wstat: 0 Tests: 3251 Failed: 2)
   Failed tests:  3250-3251
   Parse errors: Plan (1..3249) must be at the beginning or end of the TAP output
                 Bad plan.  You planned 3249 tests but ran 3251.
Comment 35 Nick Clemens (kidclamp) 2025-02-13 12:57:59 UTC
Created attachment 177994 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 36 Nick Clemens (kidclamp) 2025-02-13 12:58:01 UTC
Created attachment 177995 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 37 Nick Clemens (kidclamp) 2025-02-13 12:58:04 UTC
Created attachment 177996 [details] [review]
Bug 38944: Manual changes

Not caught by the script

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 38 Nick Clemens (kidclamp) 2025-02-13 12:58:06 UTC
Created attachment 177997 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 39 Nick Clemens (kidclamp) 2025-02-13 12:58:08 UTC
Created attachment 177998 [details] [review]
Bug 38944: Revert changes to selenium/01-installation.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 40 Nick Clemens (kidclamp) 2025-02-13 12:58:10 UTC
Created attachment 177999 [details] [review]
Bug 38944: Revert changes to t/00-testcritic.t

Not sure what's happening here. Skipping it for now.

 t/00-testcritic.t .. 3246/?
 #   Failed test 'done_testing() was already called at /usr/share/perl5/Test/Perl/Critic.pm line 105'
 #   at t/00-testcritic.t line 22.
 t/00-testcritic.t .. All 3249 subtests passed

 Test Summary Report
 -------------------
 t/00-testcritic.t (Wstat: 0 Tests: 3251 Failed: 2)
   Failed tests:  3250-3251
   Parse errors: Plan (1..3249) must be at the beginning or end of the TAP output
                 Bad plan.  You planned 3249 tests but ran 3251.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 41 Katrin Fischer 2025-02-13 13:57:07 UTC
@mtj: Can you look into the dependency please?

+recommends 'Test::NoWarnings', '1.06';
Comment 42 Mason James 2025-02-14 01:34:59 UTC
(In reply to Katrin Fischer from comment #41)
> @mtj: Can you look into the dependency please?
> 
> +recommends 'Test::NoWarnings', '1.06';

hie, this looks ok

we can add libtest-nowarnings-perl_1.06-2_all.deb to the koha repo, for older debian versions
Comment 43 Katrin Fischer 2025-02-14 10:35:00 UTC
Thanks Mason, how do we coordinate here?
Comment 44 Jonathan Druart 2025-03-10 10:01:06 UTC
We should push this, really...
Comment 45 Katrin Fischer 2025-03-10 10:27:44 UTC
(In reply to Jonathan Druart from comment #44)
> We should push this, really...

I'll reach out to Mason for adding the dependency to the repo.
Comment 46 Mason James 2025-03-13 11:13:40 UTC
(In reply to Katrin Fischer from comment #45)
> (In reply to Jonathan Druart from comment #44)
> > We should push this, really...
> 
> I'll reach out to Mason for adding the dependency to the repo.

i've pushed the libtest-nowarnings-perl_1.06-2_all package to both koha and koha-staging repos
Comment 47 Mason James 2025-03-13 11:21:23 UTC
(In reply to Mason James from comment #46)
> (In reply to Katrin Fischer from comment #45)
> > (In reply to Jonathan Druart from comment #44)
> > > We should push this, really...
> > 
> > I'll reach out to Mason for adding the dependency to the repo.
> 
> i've pushed the libtest-nowarnings-perl_1.06-2_all package to both koha and
> koha-staging repos

$ docker run -it debian:buster
...
root@22861f5b59a7:/# lsb_release -a
No LSB modules are available.
Distributor ID:	Debian
Description:	Debian GNU/Linux 10 (buster)
Release:	10
Codename:	buster

root@22861f5b59a7:/# apt policy libtest-nowarnings-perl
libtest-nowarnings-perl:
  Installed: 1.06-2
  Candidate: 1.06-2
  Version table:
 *** 1.06-2 500
        500 https://debian.koha-community.org/koha stable/main amd64 Packages
        100 /var/lib/dpkg/status
     1.04-2 500
        500 http://deb.debian.org/debian buster/main amd64 Packages
Comment 48 Mason James 2025-03-13 11:24:16 UTC
(In reply to Katrin Fischer from comment #43)
> Thanks Mason, how do we coordinate here?

sorry about missing this one!
Comment 49 Katrin Fischer 2025-03-13 11:41:16 UTC
(In reply to Mason James from comment #48)
> (In reply to Katrin Fischer from comment #43)
> > Thanks Mason, how do we coordinate here?
> 
> sorry about missing this one!

@Joubu: Ready to push, but can you please rebase?
Comment 50 Jonathan Druart 2025-03-13 13:27:50 UTC
Created attachment 179241 [details] [review]
Bug 38944: Add new dependency Test::NoWarnings

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 51 Jonathan Druart 2025-03-13 13:27:56 UTC
Created attachment 179242 [details] [review]
Bug 38944: Add Test::NoWarnings to tests no producing warnings

This has been generated.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 52 Jonathan Druart 2025-03-13 13:27:59 UTC
Created attachment 179243 [details] [review]
Bug 38944: Manual changes

Not caught by the script

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 53 Jonathan Druart 2025-03-13 13:28:02 UTC
Created attachment 179244 [details] [review]
Bug 38944: Correctly catch warnings raised by DBI

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 54 Jonathan Druart 2025-03-13 13:28:06 UTC
Created attachment 179245 [details] [review]
Bug 38944: Revert changes to selenium/01-installation.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 55 Jonathan Druart 2025-03-13 13:28:09 UTC
Created attachment 179246 [details] [review]
Bug 38944: Revert changes to t/00-testcritic.t

Not sure what's happening here. Skipping it for now.

 t/00-testcritic.t .. 3246/?
 #   Failed test 'done_testing() was already called at /usr/share/perl5/Test/Perl/Critic.pm line 105'
 #   at t/00-testcritic.t line 22.
 t/00-testcritic.t .. All 3249 subtests passed

 Test Summary Report
 -------------------
 t/00-testcritic.t (Wstat: 0 Tests: 3251 Failed: 2)
   Failed tests:  3250-3251
   Parse errors: Plan (1..3249) must be at the beginning or end of the TAP output
                 Bad plan.  You planned 3249 tests but ran 3251.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 56 Jonathan Druart 2025-03-13 13:28:11 UTC
Created attachment 179247 [details] [review]
Bug 38944: Remove warning from Patron/Borrower_Debarments.

 #     Use of uninitialized value $anonymous_patron in string eq at /kohadevbox/koha/Koha/Patron.pm line 737.
Comment 57 Jonathan Druart 2025-03-13 13:28:31 UTC
(In reply to Mason James from comment #48)
> (In reply to Katrin Fischer from comment #43)
> > Thanks Mason, how do we coordinate here?
> 
> sorry about missing this one!

Thanks!
Comment 58 Katrin Fischer 2025-03-13 16:30:09 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 59 Jonathan Druart 2025-03-17 08:24:24 UTC
Created attachment 179378 [details] [review]
Bug 38944: Fix tidiness t/db_dependent/check_kohastructure.t

This is an odd one, why this change?
Comment 60 Katrin Fischer 2025-03-17 11:03:00 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 61 Jonathan Druart 2025-03-19 14:42:37 UTC
Created attachment 179478 [details] [review]
Bug 38944: Tidy t/db_dependent/Record/Record.t

The generated patch was big and I skipped the git hook :-/
Comment 62 Katrin Fischer 2025-03-19 16:36:14 UTC
(In reply to Jonathan Druart from comment #61)
> Created attachment 179478 [details] [review] [review]
> Bug 38944: Tidy t/db_dependent/Record/Record.t
> 
> The generated patch was big and I skipped the git hook :-/

Picked up for main.
Comment 63 David Cook 2025-04-15 06:43:34 UTC
I'm not seeing this in koha-testing-docker, but I'm guessing I must need to do a new pull...
Comment 64 David Nind 2025-04-15 09:01:04 UTC
(In reply to David Cook from comment #63)
> I'm not seeing this in koha-testing-docker, but I'm guessing I must need to
> do a new pull...

I think you will. I'm seeing it installed for me (I recently removed all the docker images and volumes and did a fresh ktd pull).
Comment 65 David Cook 2025-04-16 00:06:11 UTC
(In reply to David Nind from comment #64)
> (In reply to David Cook from comment #63)
> > I'm not seeing this in koha-testing-docker, but I'm guessing I must need to
> > do a new pull...
> 
> I think you will. I'm seeing it installed for me (I recently removed all the
> docker images and volumes and did a fresh ktd pull).

Thanks for confirming, David :).
Comment 66 Jonathan Druart 2025-05-06 14:09:40 UTC
Created attachment 181977 [details] [review]
Bug 38944: Fix for Mysql8

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 67 Katrin Fischer 2025-05-06 15:52:26 UTC
Pushed latest follow-up "Fix for Mysql8" to main