Bug 21985

Summary: Test t/db_dependent/Circulation.t fails if SearchEngine is set to elasticsearch
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: Test SuiteAssignee: Chris Cormack <chris>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: fridolin.somers, hayleypelham, jonathan.druart, katrin.fischer, lucas, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05
Bug Depends on: 21971    
Bug Blocks:    
Attachments: Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item
Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item
Bug 21985: Trying to fix the next failure
Bug 21985: Fix Circulation tests when ES it set, by using build_sample_item
Bug 21985: Trying to fix the next failure
Bug 21985: Fix further occurences

Description Josef Moravec 2018-12-12 13:40:27 UTC
The test t/db_dependent/Circulation.t fails with:


not ok 106 - No tests run for subtest "CanBookBeIssued & AllowReturnToBranch"

#   Failed test 'No tests run for subtest "CanBookBeIssued & AllowReturnToBranch"'
#   at t/db_dependent/Circulation.t line 1302.
Can't call method "leader" on an undefined value at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 419.
# Looks like your test exited with 255 just after 106.
Comment 1 Jonathan Druart 2018-12-16 15:30:53 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
Comment 2 Hayley Pelham 2019-03-10 21:44:48 UTC
(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'?
Comment 3 Katrin Fischer 2019-03-10 22:58:21 UTC
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.
Comment 4 Hayley Pelham 2019-03-11 02:26:50 UTC
(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!
Comment 5 Hayley Pelham 2019-03-11 03:40:30 UTC
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
Comment 6 Hayley Pelham 2019-03-11 03:46:00 UTC
(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.
Comment 7 Hayley Pelham 2019-03-12 03:44:44 UTC
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
Comment 8 Jonathan Druart 2019-03-14 22:20:10 UTC
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
Comment 9 Jonathan Druart 2019-03-14 22:20:15 UTC
Created attachment 86635 [details] [review]
Bug 21985: Trying to fix the next failure
Comment 10 Jonathan Druart 2019-03-14 22:21:48 UTC
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?
Comment 11 Jonathan Druart 2019-03-14 22:22:52 UTC
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.
Comment 12 Nick Clemens 2019-09-30 13:55:10 UTC
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>
Comment 13 Nick Clemens 2019-09-30 13:55:16 UTC
Created attachment 93260 [details] [review]
Bug 21985: Trying to fix the next failure

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens 2019-09-30 13:55:20 UTC
Created attachment 93261 [details] [review]
Bug 21985: Fix further occurences

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Martin Renvoize 2019-10-02 23:01:41 UTC
Works well, thanks Jonathan. Passing QA
Comment 16 Martin Renvoize 2019-10-03 09:31:57 UTC
Nice work!

Pushed to master for 19.11.00
Comment 17 Fridolin Somers 2019-11-08 13:59:23 UTC
Pushed to 19.05.x for 19.05.05
Comment 18 Lucas Gass 2019-11-13 23:12:43 UTC
18.11.x is missing the bz21971 dependency so I will not backport for 18.11.x