[Bug 31676] New: Fix db_dependent/Circulation.t tests
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Bug ID: 31676 Summary: Fix db_dependent/Circulation.t tests Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: glasklas@gmail.com QA Contact: testopia@bugs.koha-community.org A lot of tests in db_dependent/Circulation.t are currently failing, many seems to have done so for quite a while so it's a bit puzzling how this has gone under radar for such a long time. If any of the changes are unclear I'm happy to expand more on the different addressed issues. A short summary of the changes: - The patron attribute autorenew_checkouts was added a while ago, patrons for some tests needed this set to preserve previous behavior - OverduesBlockRenewing-test relied on a state that was mutated by new tests, these have been moved to a place where the state is as was before this regression. - The test "AddRenewal left both fines" broke when account line statuses where refactored (probably), there should be one one fine - Fixed a test which expects 'auto_to_soon' error. CanBookBeRenewed will not return this status unless the $cron parameter is set to true - "Incremented fee tests" builds an item with missing "library" parameter since treating the library object as a hashref. This cases a circulation-rule not match and serveral tests fail as a result. - The "AddIssue records staff who checked out item if appropriate" subtest treats some objects as hashrefs when retrieving properties, and also checks the wrong issue objects in the last test. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 141330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141330&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 2) Apply the patch 3) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, I ran these tests just yesterday without fail, I still see it in my bash history: prove t/db_dependent/Circulation.t t/db_dependent/Circulation/* Could it depend on having an untouched sample database to start with? But it is a bit odd. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #3 from David Gustafsson <glasklas@gmail.com> --- Ok, that's strange. I think most of the test failing for me could pass with certain sysprefs set, but at least the one where two account lines are expected I just can't figure out how it would ever do so. In this test an item is issued with due date five weeks ago, UpdateFine is then run which creates an account line with type "OVERDUE" and status "UNRETURNED" (line 722 in current master). AddRenewal is then called twice for this item in turn calling _CalculateAndUpdateFine which checks for a fine calling CalcFine and then possibly calling UpdateFine if got a fine amount. In the first call to AddRenewal the due date is five weeks in the past, so CalcFine will return a fine. UpdateFine is then called which will fetch the account line and adjust the amount (to the same value as before), then calling _FixOverduesOnReturn which sets the account line status to "RENEWED". Due date for the issue is then set to a date in the future. In the second call CalcFine will not return a fine since due date is in the future thus UpdateFine is not called (which I have also verified in my instance). I just can't figure out how UpdateFine could ever add more than one account line in this test case. I also don't see any sysprefs or other state that is not already mocked which could effect AddRenewal behavior. Obviously since the tests work for you there should be such a case, but personally I can't figure out how to reproduce it. I also tried resetting all sysprefs to default values but this test still fails. From what I can see circulation rules are also cleared before running the tests so existing ones should not effect the results. The docker setup currently running koha on makes it very cumbersome to completely reset the koha instance to a completely clean installation, but if I had access to an instance where the tests do indeed pass it would probably be trial to find out what is causing this issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #4 from David Gustafsson <glasklas@gmail.com> --- *trivial -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #5 from David Gustafsson <glasklas@gmail.com> --- Now I got the test to work, the syspref cache wasn't probably cleared when ran it the first time. Tests fail for me since the "RenewalPeriodBase" syspref was set to "now". The default value is "date_due", so when renewing the item the new due date is still in the past and will incur fees also on the next renewal (even if the previously generated fee was calculated using the whole period). This is pretty weird behavior and perhaps should be looked over, but at least found the culprit and the tests should set this syspref to one of the options (personally would prefer "now" sisnce the "date_due"-behavior is pretty confusing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #6 from David Gustafsson <glasklas@gmail.com> --- Setting RenewalPeriodBase to "now" also explains why the 'OverduesBlockRenewing'-tests failed for me, since when the period is "date_due" the item is still overdue even though it was renewed twice. So if using "now" theses tests has to be moved as in the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #7 from David Gustafsson <glasklas@gmail.com> --- To make the 'Incremented fee tests' fail set CircControl to "ItemHomeLibrary" (which is the default) and HomeOrHoldingBranch to "homebranch" (default is "holdingbranch"). The test will now fail since the item does not have the correct homebranch set (since is set using $library->{branchcode} where library is an object). The reason why the tests pass anyway despite missing both homebranch and holdingbranch is that holdingbranch is set when one of the tests calls AddIssue which will in turn set the expected holdingbranch from userenv ($item_object->holdingbranch(C4::Context->userenv->{'branch'})); The $item will then have the expected homebranch (but not holdingbranch) for the rest of the tests. If the item is passed the correct branch value to begin with all combinations of the above settings will work. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #8 from David Gustafsson <glasklas@gmail.com> --- Will update the testplan (probably tomorrow) so includes how to reproduce the test-bugs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141330|0 |1 is obsolete| | --- Comment #9 from David Gustafsson <glasklas@gmail.com> --- Created attachment 141439 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141439&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Set "RenewalPeriodBase" syspref to "the current date", "CircControl" to "the library the item is from." and "HomeOrHoldingBranch" to "the item's home library (homebranch)." 2) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 3) Apply the patch 4) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #10 from David Gustafsson <glasklas@gmail.com> --- Now updated the test plan so the errors are reproducible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141439|0 |1 is obsolete| | --- Comment #11 from David Gustafsson <glasklas@gmail.com> --- Created attachment 141463 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141463&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Set "RenewalPeriodBase" syspref to "the current date", "CircControl" to "the library the item is from." and "HomeOrHoldingBranch" to "the item's home library (homebranch)." 2) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 3) Apply the patch 4) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141463|0 |1 is obsolete| | --- Comment #12 from David Gustafsson <glasklas@gmail.com> --- Created attachment 141464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141464&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Set "RenewalPeriodBase" syspref to "the current date", "CircControl" to "the library the item is from." and "HomeOrHoldingBranch" to "the item's home library (homebranch)." 2) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 3) Apply the patch 4) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #13 from David Gustafsson <glasklas@gmail.com> --- Reverted some of the changes in the patch which found wasn't actually necessary for tests to succeed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |31735 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31735 [Bug 31735] Circulation optimizations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Assignee|chris@bigballofwax.co.nz |glasklas@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141464|0 |1 is obsolete| | --- Comment #14 from Lucas Gass <lucas@bywatersolutions.com> --- Created attachment 141625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141625&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Set "RenewalPeriodBase" syspref to "the current date", "CircControl" to "the library the item is from." and "HomeOrHoldingBranch" to "the item's home library (homebranch)." 2) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 3) Apply the patch 4) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Fix |Make |db_dependent/Circulation.t |db_dependent/Circulation.t |tests |tests more robust --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This makes the tests a little more robust/less dependent on the default setup. Recently I noticed that most of the time they would only pass on a new db/reset_all database for me, so hopeful that will help with that :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141625|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142457&action=edit Bug 31676: Fix db_dependent/Circulation.t tests To test: 1) Set "RenewalPeriodBase" syspref to "the current date", "CircControl" to "the library the item is from." and "HomeOrHoldingBranch" to "the item's home library (homebranch)." 2) Run tests in t/db_dependent/Circulation.t and verify that fails (the script should actually crash before all can be run). 3) Apply the patch 4) Verifiy that all tests now pass Sponsored-by: Gothenburg University Library Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED --- Comment #18 from Lucas Gass <lucas@bywatersolutions.com> --- Does not apply cleanly to 22.05.x, no backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31676 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 31493 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org