Description
Jonathan Druart
2025-01-22 13:30:34 UTC
Created attachment 176912 [details] [review] Bug 38944: Add new dependency Test::NoWarnings Created attachment 176913 [details] [review] Bug 38944: Add Test::NoWarnings to tests no producing warnings This has been generated. Created attachment 176914 [details] [review] Bug 38944: Manual changes Not caught by the script 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. I tried testing, but I think Test::NoWarnings module needs to be added to ktd to be able to be able to test. Yes you can install it using `cpanm Test::NoWarnings` 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 Created attachment 177165 [details] [review] Bug 38944: Add new dependency Test::NoWarnings Created attachment 177166 [details] [review] Bug 38944: Add Test::NoWarnings to tests no producing warnings This has been generated. Created attachment 177167 [details] [review] Bug 38944: Manual changes Not caught by the script 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
Created attachment 177274 [details] [review] Bug 38944: Manual changes Not caught by the script Created attachment 177275 [details] [review] Bug 38944: Correctly catch warnings raised by DBI This is pretty awesome. I've always done this. Thanks! 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 Created attachment 177362 [details] [review] Bug 38944: Add Test::NoWarnings to tests no producing warnings This has been generated. Created attachment 177363 [details] [review] Bug 38944: Manual changes Not caught by the script Created attachment 177364 [details] [review] Bug 38944: Correctly catch warnings raised by DBI 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 Created attachment 177380 [details] [review] Bug 38944: Revert changes to selenium/01-installation.t (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. 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 Created attachment 177477 [details] [review] Bug 38944: Add new dependency Test::NoWarnings Signed-off-by: David Nind <david@davidnind.com> 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> Created attachment 177479 [details] [review] Bug 38944: Manual changes Not caught by the script Signed-off-by: David Nind <david@davidnind.com> Created attachment 177480 [details] [review] Bug 38944: Correctly catch warnings raised by DBI Signed-off-by: David Nind <david@davidnind.com> Created attachment 177481 [details] [review] Bug 38944: Revert changes to selenium/01-installation.t Signed-off-by: David Nind <david@davidnind.com> 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" Created attachment 177928 [details] [review] Bug 38944: Add new dependency Test::NoWarnings Signed-off-by: David Nind <david@davidnind.com> 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> Created attachment 177930 [details] [review] Bug 38944: Manual changes Not caught by the script Signed-off-by: David Nind <david@davidnind.com> Created attachment 177931 [details] [review] Bug 38944: Correctly catch warnings raised by DBI Signed-off-by: David Nind <david@davidnind.com> Created attachment 177932 [details] [review] Bug 38944: Revert changes to selenium/01-installation.t Signed-off-by: David Nind <david@davidnind.com> 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. 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> 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> 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> 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> 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> 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> @mtj: Can you look into the dependency please? +recommends 'Test::NoWarnings', '1.06'; (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 Thanks Mason, how do we coordinate here? We should push this, really... (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. (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 (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 (In reply to Katrin Fischer from comment #43) > Thanks Mason, how do we coordinate here? sorry about missing this one! (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? 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> 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> 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> 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> 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> 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> 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. (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! Pushed for 25.05! Well done everyone, thank you! Created attachment 179378 [details] [review] Bug 38944: Fix tidiness t/db_dependent/check_kohastructure.t This is an odd one, why this change? Pushed for 25.05! Well done everyone, thank you! 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 :-/ (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. I'm not seeing this in koha-testing-docker, but I'm guessing I must need to do a new pull... (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). (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 :). Created attachment 181977 [details] [review] Bug 38944: Fix for Mysql8 Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed latest follow-up "Fix for Mysql8" to main |