Attachment 112239 Details for Bug 24083 – Bug 24083: (follow-up) Fix unit tests

[patch] Bug 24083: (follow-up) Fix unit tests

Bug-24083-follow-up-Fix-unit-tests.patch (text/plain), 8.60 KB, created by Andrew Isherwood on 2020-10-23 10:10:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2020-10-23 10:10:35 UTC

Size: 8.60 KB

patch

obsolete

>From 5dd3f25fe931e5411831b358c5423be0011a3024 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Tue, 6 Oct 2020 11:32:32 +0100
>Subject: [PATCH] Bug 24083: (follow-up) Fix unit tests
>
>Some changes in issue.t had broken the tests for unseen renewals (the
>unseen tests were using variables that had been moved out of the tests'
>scope).
>
>Also now using Koha::CirculationRules::set_rules to set circ rules
>rather than using SQL queries.
>
>Fixed expected number of return values from GetRenewCount
>
>Moved unseen tests in issue.t to the bottom of the file to remove the
>risk of interference with other test circ rules.
>---
> t/db_dependent/Circulation.t       | 26 ++++++++++--
> t/db_dependent/Circulation/issue.t | 64 +++++++++++++++++++-----------
> 2 files changed, 64 insertions(+), 26 deletions(-)
>
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 9fd56e1b77..493e923045 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -278,7 +278,7 @@ Koha::CirculationRules->set_rules(
> 
> my ( $reused_itemnumber_1, $reused_itemnumber_2 );
> subtest "CanBookBeRenewed tests" => sub {
>-    plan tests => 87;
>+    plan tests => 89;
> 
>     C4::Context->set_preference('ItemsDeniedRenewal','');
>     # Generate test biblio
>@@ -1158,12 +1158,32 @@ subtest "CanBookBeRenewed tests" => sub {
>     is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)');
> 
>     # Too many unseen renewals
>-    $dbh->do('UPDATE issuingrules SET unseen_renewals_allowed = 2, renewalsallowed = 10');
>+    Koha::CirculationRules->set_rules(
>+        {
>+            categorycode => undef,
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rules        => {
>+                unseen_renewals_allowed => 2,
>+                renewalsallowed => 10,
>+            }
>+        }
>+    );
>     $dbh->do('UPDATE issues SET unseen_renewals = 2 where borrowernumber = ? AND itemnumber = ?', undef, ($renewing_borrowernumber, $item_1->itemnumber));
>     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
>     is( $renewokay, 0, 'Cannot renew, 0 unseen renewals allowed');
>     is( $error, 'too_unseen', 'Cannot renew, returned code is too_unseen');
>-    $dbh->do('UPDATE issuingrules SET norenewalbefore = NULL, renewalsallowed = 0');
>+    Koha::CirculationRules->set_rules(
>+        {
>+            categorycode => undef,
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rules        => {
>+                norenewalbefore => undef,
>+                renewalsallowed => 0,
>+            }
>+        }
>+    );
> 
>     # Test WhenLostForgiveFine and WhenLostChargeReplacementFee
>     t::lib::Mocks::mock_preference('WhenLostForgiveFine','1');
>diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t
>index 6c8c9ba30b..3fbf46c590 100755
>--- a/t/db_dependent/Circulation/issue.t
>+++ b/t/db_dependent/Circulation/issue.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 46;
>+use Test::More tests => 48;
> use DateTime::Duration;
> 
> use t::lib::Mocks;
>@@ -297,20 +297,6 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub {
>     }
> };
> 
>-# Unseen rewnewals
>-t::lib::Mocks::mock_preference('UnseenRenewals', 1);
>-
>-# Does an unseen renewal increment the issue's count
>-my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-AddRenewal( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber}, $branchcode_1, undef, undef, 0 );
>-my ( $unseen_after ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-is( $unseen_after, $unseen_before + 1, 'unseen_renewals increments' );
>-
>-# Does a seen renewal reset the unseen count
>-AddRenewal( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber}, $branchcode_1, undef, undef, 1 );
>-my ( $unseen_reset ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-is( $unseen_reset, 0, 'seen renewal resets the unseen count' );
>-
> #Test GetBiblioIssues
> is( GetBiblioIssues(), undef, "GetBiblio Issues without parameters" );
> 
>@@ -327,19 +313,19 @@ my $issue3 = C4::Circulation::AddIssue( $borrower_1, $barcode_1 );
> @renewcount = C4::Circulation::GetRenewCount();
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ], # FIXME Need to be fixed, see FIXME in GetRenewCount
>+    [ 0, 0, 0, 0, 0, 0 ], # FIXME Need to be fixed, see FIXME in GetRenewCount
>     "Without issuing rules and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = undef, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount(-1);
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ], # FIXME Need to be fixed
>+    [ 0, 0, 0, 0, 0, 0 ], # FIXME Need to be fixed
>     "Without issuing rules and without wrong parameter, GetRenewCount returns renewcount = 0, renewsallowed = undef, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 0, 0 ],
>+    [ 2, 0, 0, 0, 0, 0 ],
>     "Without issuing rules and with a valid parameter, renewcount = 2, renewsallowed = undef, renewsleft = 0"
> );
> 
>@@ -347,19 +333,19 @@ is_deeply(
> @renewcount = C4::Circulation::GetRenewCount();
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ],
>+    [ 0, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = 0, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount(-1);
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ],
>+    [ 0, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and without wrong parameter, GetRenewCount returns renewcount = 0, renewsallowed = 0, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 0, 0 ],
>+    [ 2, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and with a valid parameter, Getrenewcount returns renewcount = 2, renewsallowed = 0, renewsleft = 0"
> );
> 
>@@ -377,7 +363,7 @@ Koha::CirculationRules->set_rules(
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 3, 1 ],
>+    [ 2, 3, 1, 0, 0, 0 ],
>     "With issuing rules (renewal allowed) and with a valid parameter, Getrenewcount of item1 returns 3 renews left"
> );
> 
>@@ -386,7 +372,7 @@ AddRenewal( $borrower_id1, $item_id1, $branchcode_1,
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 3, 3, 0 ],
>+    [ 3, 3, 0, 0, 0, 0 ],
>     "With issuing rules (renewal allowed, 1 remaining) and with a valid parameter, Getrenewcount of item1 returns 0 renews left"
> );
> 
>@@ -504,5 +490,37 @@ AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' );
> my $hold = Koha::Holds->find( $reserve_id );
> is( $hold, undef, 'The reserve should have been correctly cancelled' );
> 
>+# Unseen rewnewals
>+t::lib::Mocks::mock_preference('UnseenRenewals', 1);
>+# Add a default circ rule: 3 unseen renewals allowed
>+Koha::CirculationRules->set_rules(
>+    {
>+        categorycode => undef,
>+        itemtype     => undef,
>+        branchcode   => undef,
>+        rules        => {
>+            renewalsallowed => 10,
>+            unseen_renewals_allowed => 3
>+        }
>+    }
>+);
>+
>+my $unseen_library = $builder->build_object( { class => 'Koha::Libraries' } );
>+my $unseen_patron  = $builder->build_object( { class => 'Koha::Patrons' } );
>+my $unseen_item = $builder->build_sample_item(
>+    { library => $unseen_library->branchcode, itype => $itemtype } );
>+my $unseen_issue = C4::Circulation::AddIssue( $unseen_patron->unblessed, $unseen_item->barcode );
>+
>+# Does an unseen renewal increment the issue's count
>+my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 0 );
>+my ( $unseen_after ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+is( $unseen_after, $unseen_before + 1, 'unseen_renewals increments' );
>+
>+# Does a seen renewal reset the unseen count
>+AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 1 );
>+my ( $unseen_reset ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+is( $unseen_reset, 0, 'seen renewal resets the unseen count' );
>+
> #End transaction
> $schema->storage->txn_rollback;
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 24083: 95861 | 95862 | 95863 | 95864 | 95865 | 95866 | 95867 | 95868 | 98749 | 98750 | 98751 | 98752 | 98765 | 98766 | 98767 | 98768 | 100244 | 100245 | 100246 | 100247 | 100380 | 100381 | 100382 | 100383 | 100392 | 110607 | 110608 | 110609 | 110610 | 110611 | 111147 | 111148 | 111149 | 111150 | 111151 | 111152 | 111283 | 111284 | 111285 | 111286 | 111287 | 111288 | 111289 | 111290 | 111291 | 111292 | 111293 | 111849 | 111850 | 111851 | 111852 | 111853 | 111854 | 111855 | 111856 | 111857 | 111858 | 111859 | 111860 | 112230 | 112231 | 112232 | 112233 | 112234 | 112235 | 112236 | 112237 | 112238 | 112239 | 112240 | 112241 | 112242 | 112243 | 112410 | 112411 | 112412 | 112413 | 112414 | 112415 | 112416 | 112417 | 112418 | 112419 | 112420 | 112421 | 112422 | 112423 | 113073 | 113074 | 113075 | 113076 | 113077 | 113078 | 113079 | 113080 | 113314 | 113315 | 113484 | 113485 | 113486 | 113487 | 113488 | 113489 | 113490 | 113491 | 113492 | 113493 | 113494 | 113546 | 113556 | 113557 | 113814