Description
Josef Moravec
2018-12-12 13:40:27 UTC
Created attachment 83254 [details] [review] Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item We should use build_sample item (from bug 21971) to create items, otherwise we may not have a valid biblioitem and/or MARC record. Test plan: Set SearchEngine to ES prove that the tests in Circulation.t now pass (In reply to Jonathan Druart from comment #1) > Created attachment 83254 [details] [review] [review] > Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item > > We should use build_sample item (from bug 21971) to create items, > otherwise we may not have a valid biblioitem and/or MARC record. > > Test plan: > Set SearchEngine to ES > prove that the tests in Circulation.t now pass Hi, could you please provide a more detailed description for the test step 'set SearchEngine to ES'? Hi Hayley, there is a system preference SearchEngine you can switch between Zebra and Elasticsearch. Depending on what you use for testing there might be more steps needed, in a kohadevbox you should be ok with switching the pref and doing a reindex. The script for reindexing Elasticsearch can be run using koha-shell and is in misc. (In reply to Katrin Fischer from comment #3) > Hi Hayley, there is a system preference SearchEngine you can switch between > Zebra and Elasticsearch. Depending on what you use for testing there might > be more steps needed, in a kohadevbox you should be ok with switching the > pref and doing a reindex. The script for reindexing Elasticsearch can be run > using koha-shell and is in misc. Thank you Katrin! Before applying the patch, I get this error when trying to prove those tests: t/db_dependent/Circulation.t .. 61/126 DBD::mysql::st execute failed: Duplicate entry '10' for key 'PRIMARY' [for Statement "INSERT INTO `old_issues` ( `auto_renew`, `auto_renew_error`, `borrowernumber`, `branchcode`, `date_due`, `issue_id`, `issuedate`, `itemnumber`, `lastreneweddate`, `note`, `notedate`, `noteseen`, `onsite_checkout`, `renewals`, `returndate`, `timestamp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0=0, 1=undef, 2=54, 3='bWmeAOZip', 4='2019-03-25 23:59:00', 5=10, 6='2019-03-11 03:36:44', 7=991, 8=undef, 9=undef, 10=undef, 11=undef, 12=0, 13=undef, 14='2019-03-11 03:36:47', 15='2019-03-11 03:36:47'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. {UNKNOWN}: Can't locate object method "rethrow" via package "DBD::mysql::st execute failed: Duplicate entry '10' for key 'PRIMARY' [for Statement "INSERT INTO `old_issues` ( `auto_renew`, `auto_renew_error`, `borrowernumber`, `branchcode`, `date_due`, `issue_id`, `issuedate`, `itemnumber`, `lastreneweddate`, `note`, `notedate`, `noteseen`, `onsite_checkout`, `renewals`, `returndate`, `timestamp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0=0, 1=undef, 2=54, 3='bWmeAOZip', 4='2019-03-25 23:59:00', 5=10, 6='2019-03-11 03:36:44', 7=991, 8=undef, 9=undef, 10=undef, 11=undef, 12=0, 13=undef, 14='2019-03-11 03:36:47', 15='2019-03-11 03:36:47'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. " (perhaps you forgot to load "DBD::mysql::st execute failed: Duplicate entry '10' for key 'PRIMARY' [for Statement "INSERT INTO `old_issues` ( `auto_renew`, `auto_renew_error`, `borrowernumber`, `branchcode`, `date_due`, `issue_id`, `issuedate`, `itemnumber`, `lastreneweddate`, `note`, `notedate`, `noteseen`, `onsite_checkout`, `renewals`, `returndate`, `timestamp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0=0, 1=undef, 2=54, 3='bWmeAOZip', 4='2019-03-25 23:59:00', 5=10, 6='2019-03-11 03:36:44', 7=991, 8=undef, 9=undef, 10=undef, 11=undef, 12=0, 13=undef, 14='2019-03-11 03:36:47', 15='2019-03-11 03:36:47'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. "?) at /home/vagrant/kohaclone/Koha/Object.pm line 185. at /home/vagrant/kohaclone/C4/Circulation.pm line 2206 # Looks like your test exited with 255 just after 71. t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 55/126 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 65280 Tests: 71 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 126 tests but ran 71. Files=1, Tests=71, 7 wallclock secs ( 0.03 usr 0.01 sys + 3.42 cusr 0.38 csys = 3.84 CPU) Result: FAIL And the patch does not apply: Applying: Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item error: sha1 information is lacking or useless (t/db_dependent/Circulation.t). error: could not build fake ancestor Patch failed at 0001 Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item (In reply to Hayley Mapley from comment #5) > Before applying the patch, I get this error when trying to prove those tests: > > t/db_dependent/Circulation.t .. 61/126 DBD::mysql::st execute failed: > Duplicate entry '10' for key 'PRIMARY' [for Statement "INSERT INTO > `old_issues` ( `auto_renew`, `auto_renew_error`, `borrowernumber`, > `branchcode`, `date_due`, `issue_id`, `issuedate`, `itemnumber`, > `lastreneweddate`, `note`, `notedate`, `noteseen`, `onsite_checkout`, > `renewals`, `returndate`, `timestamp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, > ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0=0, 1=undef, 2=54, 3='bWmeAOZip', > 4='2019-03-25 23:59:00', 5=10, 6='2019-03-11 03:36:44', 7=991, 8=undef, > 9=undef, 10=undef, 11=undef, 12=0, 13=undef, 14='2019-03-11 03:36:47', > 15='2019-03-11 03:36:47'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line > 1836. > {UNKNOWN}: Can't locate object method "rethrow" via package "DBD::mysql::st > execute failed: Duplicate entry '10' for key 'PRIMARY' [for Statement > "INSERT INTO `old_issues` ( `auto_renew`, `auto_renew_error`, > `borrowernumber`, `branchcode`, `date_due`, `issue_id`, `issuedate`, > `itemnumber`, `lastreneweddate`, `note`, `notedate`, `noteseen`, > `onsite_checkout`, `renewals`, `returndate`, `timestamp`) VALUES ( ?, ?, ?, > ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0=0, 1=undef, > 2=54, 3='bWmeAOZip', 4='2019-03-25 23:59:00', 5=10, 6='2019-03-11 03:36:44', > 7=991, 8=undef, 9=undef, 10=undef, 11=undef, 12=0, 13=undef, 14='2019-03-11 > 03:36:47', 15='2019-03-11 03:36:47'] at > /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. > " (perhaps you forgot to load "DBD::mysql::st execute failed: Duplicate > entry '10' for key 'PRIMARY' [for Statement "INSERT INTO `old_issues` ( > `auto_renew`, `auto_renew_error`, `borrowernumber`, `branchcode`, > `date_due`, `issue_id`, `issuedate`, `itemnumber`, `lastreneweddate`, > `note`, `notedate`, `noteseen`, `onsite_checkout`, `renewals`, `returndate`, > `timestamp`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with > ParamValues: 0=0, 1=undef, 2=54, 3='bWmeAOZip', 4='2019-03-25 23:59:00', > 5=10, 6='2019-03-11 03:36:44', 7=991, 8=undef, 9=undef, 10=undef, 11=undef, > 12=0, 13=undef, 14='2019-03-11 03:36:47', 15='2019-03-11 03:36:47'] at > /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. > "?) at /home/vagrant/kohaclone/Koha/Object.pm line 185. at > /home/vagrant/kohaclone/C4/Circulation.pm line 2206 > # Looks like your test exited with 255 just after 71. > t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280, > 0xff00) > Failed 55/126 subtests > > Test Summary Report > ------------------- > t/db_dependent/Circulation.t (Wstat: 65280 Tests: 71 Failed: 0) > Non-zero exit status: 255 > Parse errors: Bad plan. You planned 126 tests but ran 71. > Files=1, Tests=71, 7 wallclock secs ( 0.03 usr 0.01 sys + 3.42 cusr 0.38 > csys = 3.84 CPU) > Result: FAIL > > And the patch does not apply: > Applying: Bug 21985: Fix Circulation tests when ES it set, by using > build_sample_item > error: sha1 information is lacking or useless (t/db_dependent/Circulation.t). > error: could not build fake ancestor > Patch failed at 0001 Bug 21985: Fix Circulation tests when ES it set, by > using build_sample_item Actually reading this error, I see that it's an issue with the DB, which I will fix and then try again. Test fails with the following error: t/db_dependent/Circulation.t .. 122/126 # No tests run! t/db_dependent/Circulation.t .. 123/126 # Failed test 'No tests run for subtest "AddRenewal and AddIssuingCharge tests"' # at t/db_dependent/Circulation.t line 2948. Can't call method "lengthunit" on an undefined value at /home/vagrant/kohaclone/Koha/Charges/Fees.pm line 101. # Looks like your test exited with 11 just after 123. t/db_dependent/Circulation.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 4/126 subtests Test Summary Report ------------------- t/db_dependent/Circulation.t (Wstat: 2816 Tests: 123 Failed: 1) Failed test: 123 Non-zero exit status: 11 Parse errors: Bad plan. You planned 126 tests but ran 123. Files=1, Tests=123, 38 wallclock secs ( 0.07 usr 0.01 sys + 18.59 cusr 3.22 csys = 21.89 CPU) Result: FAIL Created attachment 86634 [details] [review] Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item We should use build_sample item (from bug 21971) to create items, otherwise we may not have a valid biblioitem and/or MARC record. Test plan: Set SearchEngine to ES prove that the tests in Circulation.t now pass Created attachment 86635 [details] [review] Bug 21985: Trying to fix the next failure Hayley, you do not have all the sample data. You must have them into your DB to have the tests running correctly. However there is an error, but another one: t/db_dependent/Circulation.t .. 114/126 # Failed test 'Lost item with cancelled hold has holding branch equallying the transfers source branch' # at t/db_dependent/Circulation.t line 2441. # got: 'XX_vS1zKTd' # expected: 'KazYKXl' # Looks like you failed 1 test of 5. t/db_dependent/Circulation.t .. 117/126 # Failed test 'Cancel transfers on lost items' I am not able to fix it. Could someone else have a try? Nick, I am raising the severity of this one. Since I have written the patch we have added new occurrences of the problematic pattern. We must have this one included into master to prevent further regressions. Created attachment 93259 [details] [review] Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item We should use build_sample item (from bug 21971) to create items, otherwise we may not have a valid biblioitem and/or MARC record. Test plan: Set SearchEngine to ES prove that the tests in Circulation.t now pass Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 93260 [details] [review] Bug 21985: Trying to fix the next failure Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 93261 [details] [review] Bug 21985: Fix further occurences Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Works well, thanks Jonathan. Passing QA Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.05 18.11.x is missing the bz21971 dependency so I will not backport for 18.11.x |