Running prove t/db_dependent/Circulation.t, I receive following error: t/db_dependent/Circulation.t .. 1/48 # Failed test 'AddRenewal left both fines' # at t/db_dependent/Circulation.t line 591. # got: '1' # expected: '2' # Looks like you planned 83 tests but ran 28. # Looks like you failed 1 test of 28 run. t/db_dependent/Circulation.t .. 15/48 # Failed test 'CanBookBeRenewed tests' # at t/db_dependent/Circulation.t line 1255. Can't call method "status" on an undefined value at t/db_dependent/Circulation.t line 593. Adding t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1) solved problem since bug 23051 added following to Circulation.pm: if ( !$skipfinecalc && C4::Context->preference('CalculateFinesOnReturn') ) { _CalculateAndUpdateFine( { issue => $issue, item => $item_unblessed, borrower => $patron_unblessed } ); } However I'm unsure if this is appropriate solution or are those tests invalid at this point.
Yes, we could mock the pref. Can you provide a patch please?
Created attachment 114135 [details] [review] Bug 26405: Prevent Circulation.t failing on 'AddRenewal left both fines' Bug 23051 added option to skip calculating fines even when "CalculateFinesOnReturn" is enabled. Due this Circulation.t fails on test "AddRenewal left both fines" if "CalculateFinesOnReturn" is disabled on ones system. Mocking syspref while testing fixes failig tests. To test: 1. Disable "CalculateFinesOnReturn". 2. Run prove t/db_dependent/Circulation.t => Tests fail 3. Apply patch. 4. Run prove t/db_dependent/Circulation.t again => Tests are succesful
Created attachment 118342 [details] [review] Bug 26405: Prevent Circulation.t failing on 'AddRenewal left both fines' Bug 23051 added option to skip calculating fines even when "CalculateFinesOnReturn" is enabled. Due this Circulation.t fails on test "AddRenewal left both fines" if "CalculateFinesOnReturn" is disabled on ones system. Mocking syspref while testing fixes failig tests. To test: 1. Disable "CalculateFinesOnReturn". 2. Run prove t/db_dependent/Circulation.t => Tests fail 3. Apply patch. 4. Run prove t/db_dependent/Circulation.t again => Tests are succesful Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Good catch. Those tests do not cover the case in which the syspref is set. But I believe this is tested in t/db_dependent/Circulation/Returns.t (somehow).
Created attachment 120122 [details] [review] Bug 26405: Prevent Circulation.t failing on 'AddRenewal left both fines' Bug 23051 added option to skip calculating fines even when "CalculateFinesOnReturn" is enabled. Due this Circulation.t fails on test "AddRenewal left both fines" if "CalculateFinesOnReturn" is disabled on ones system. Mocking syspref while testing fixes failig tests. To test: 1. Disable "CalculateFinesOnReturn". 2. Run prove t/db_dependent/Circulation.t => Tests fail 3. Apply patch. 4. Run prove t/db_dependent/Circulation.t again => Tests are succesful Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.06
Pushed to 20.05.x for 20.05.12
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.