[Bug 18146] New: C4:: Circulation CanBookBeRenewed lacks full test coverage.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Bug ID: 18146 Summary: C4::Circulation CanBookBeRenewed lacks full test coverage. 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: mtompset@hotmail.com QA Contact: gmcharlt@gmail.com While testing bug 17941, I discovered two cases where test coverage was lacking from t/db_dependent/Circulation.t Around line 2725: if ( $restricted and $restrictionblockrenewing ) { return ( 0, 'restriction'); } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) { return ( 0, 'overdue'); } The elseif is never triggered. Around line 2759: if ( $soonestrenewal > DateTime->now( time_zone => C4::Context->tz() ) ) { return ( 0, "auto_too_soon" ) if $itemissue->{auto_renew}; return ( 0, "too_soon" ); } elsif ( $itemissue->{auto_renew} ) { return ( 0, "auto_renew" ); } There is no else, but the code never attempts something that would trigger one. To reproduce add: print STDERR "CHECK!\n"; just before the return ( 0, 'overdue') in the first case, and after the elsif's curly brace in the second case. running t/db_dependent/Circulation should not generate the CHECK points. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Nick Clemens <nick@bywatersolutions.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=18146 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 116040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116040&action=edit Bug 18146: Fix tests for OverduesBlockRenewing This pref was supposedly covered by tests, but the conditions were wrong and we didn't test the reasons we were failing so the code was being missed To test: 1 - Add a warn around in the conditional at line 2748: } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) { warn "SUCCESS!" return ( 0, 'overdue'); } 2 - prove -v t/db_dependent/Circulation.t | grep SUCCESS 3 - No output 4 - Apply patch 5 - Repeat 6 - SUCCESS! https://bugs.koha-community.org/show_bug.cgi?id=18146 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 116041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116041&action=edit Bug 18146: (follow-up) Remove warn for uninitialized value In the previous patch you may have noticed many warns when running the tests We add guarantor charges to a variable to determine if over the limit, but we don't initialize that value We should To test: 1 - Apply first patch and follow test plan 2 - Note warns when proving test 3 - Apply this patch 4 - prove 5 - No more warns -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |nick@bywatersolutions.com CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 David Nind <david@davidnind.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=18146 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116040|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 116089 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116089&action=edit Bug 18146: Fix tests for OverduesBlockRenewing This pref was supposedly covered by tests, but the conditions were wrong and we didn't test the reasons we were failing so the code was being missed To test: 1 - Add a warn around in the conditional at line 2748: } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) { warn "SUCCESS!" return ( 0, 'overdue'); } 2 - prove -v t/db_dependent/Circulation.t | grep SUCCESS 3 - No output 4 - Apply patch 5 - Repeat 6 - SUCCESS! https://bugs.koha-community.org/show_bug.cgi?id=18416 Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116041|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 116090 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116090&action=edit Bug 18146: (follow-up) Remove warn for uninitialized value In the previous patch you may have noticed many warns when running the tests We add guarantor charges to a variable to determine if over the limit, but we don't initialize that value We should To test: 1 - Apply first patch and follow test plan 2 - Note warns when proving test 3 - Apply this patch 4 - prove 5 - No more warns Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #5 from David Nind <david@davidnind.com> --- Testing notes - for step 1 of the first test plan: - the file to edit is C4/Circulation.pm - there should be a semicolon after warn "SUCCESS!" (that is: warn "SUCCESS!";) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116089|0 |1 is obsolete| | --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 116865 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116865&action=edit Bug 18146: Fix tests for OverduesBlockRenewing This pref was supposedly covered by tests, but the conditions were wrong and we didn't test the reasons we were failing so the code was being missed To test: 1 - Add a warn around in the conditional at line 2748: } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($itemissue->{overdue} and $overduesblockrenewing eq 'blockitem') ) { warn "SUCCESS!" return ( 0, 'overdue'); } 2 - prove -v t/db_dependent/Circulation.t | grep SUCCESS 3 - No output 4 - Apply patch 5 - Repeat 6 - SUCCESS! https://bugs.koha-community.org/show_bug.cgi?id=18416 Signed-off-by: David Nind <david@davidnind.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=18146 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116090|0 |1 is obsolete| | --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 116866 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116866&action=edit Bug 18146: (follow-up) Remove warn for uninitialized value In the previous patch you may have noticed many warns when running the tests We add guarantor charges to a variable to determine if over the limit, but we don't initialize that value We should To test: 1 - Apply first patch and follow test plan 2 - Note warns when proving test 3 - Apply this patch 4 - prove 5 - No more warns Signed-off-by: David Nind <david@davidnind.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=18146 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|gmcharlt@gmail.com |testopia@bugs.koha-communit | |y.org Summary|C4::Circulation |C4::Circulation |CanBookBeRenewed lacks |CanBookBeRenewed lacks |full test coverage. |full test coverage --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- More tests are better, but there are still a lot of warns running theses thests before and after the patch: Use of uninitialized value in concatenation (.) or string at /home/vagrant/kohaclone/C4/Biblio.pm line 657. t/db_dependent/Circulation.t .. 32/52 Use of uninitialized value in concatenation (.) or string at /home/vagrant/kohaclone/C4/Biblio.pm line 657. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement improves release notes| |the test coverage for | |OverduesBlockRenewing and | |removes some of the warning | |messages. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18146 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #10 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 20.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org