From e8bde776f028d4d782677e0c140f7e66930832a0 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 21 Jun 2019 12:28:48 +0100 Subject: [PATCH] Bug 23177: (RM follow-up) Further test clarifications This patch makes a few minor improvements to Circulation.t 1) Adds a name to some of the scoped blocks by converting them to subtests. 2) Adds output messages to some tests where they were missing. Signed-off-by: Martin Renvoize --- t/db_dependent/Circulation.t | 52 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 6219ada92c..3fafee9b6b 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,7 +18,7 @@ use Modern::Perl; use utf8; -use Test::More tests => 130; +use Test::More tests => 44; use Test::MockModule; use Data::Dumper; @@ -276,8 +276,9 @@ $dbh->do( ); my ( $reused_itemnumber_1, $reused_itemnumber_2 ); -{ -# CanBookBeRenewed tests +subtest "CanBookBeRenewed tests" => sub { + plan tests => 71; + C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio my $biblio = $builder->build_sample_biblio(); @@ -560,7 +561,7 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); is ($new_log_size, $old_log_size + 1, 'renew log successfully added'); my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } ); - is( $fines->count, 2 ); + is( $fines->count, 2, 'AddRenewal left both fines' ); isnt( $fines->next->status, 'UNRETURNED', 'Fine on renewed item is closed out properly' ); isnt( $fines->next->status, 'UNRETURNED', 'Fine on renewed item is closed out properly' ); $fines->delete(); @@ -994,10 +995,11 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); $item_3->itemcallnumber || '' ), "Account line description must not contain 'Lost Items ', but be title, barcode, itemcallnumber" ); - } +}; + +subtest "GetUpcomingDueIssues" => sub { + plan tests => 12; -{ - # GetUpcomingDueIssues tests my $branch = $library2->{branchcode}; #Create another record @@ -1075,9 +1077,9 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); $upcoming_dues = C4::Circulation::GetUpcomingDueIssues(); is ( scalar ( @$upcoming_dues), 2, "days_in_advance is 7 in GetUpcomingDueIssues if not provided" ); -} +}; -{ +subtest "Bug 13841 - Do not create new 0 amount fines" => sub { my $branch = $library2->{branchcode}; my $biblio = $builder->build_sample_biblio(); @@ -1117,9 +1119,9 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); my $count = $hr->{count}; is ( $count, 0, "Calling UpdateFine on non-existant fine with an amount of 0 does not result in an empty fine" ); -} +}; -{ +subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { $dbh->do('DELETE FROM issues'); $dbh->do('DELETE FROM items'); $dbh->do('DELETE FROM issuingrules'); @@ -1216,7 +1218,7 @@ my ( $reused_itemnumber_1, $reused_itemnumber_2 ); ( $renewokay, $error ) = CanBookBeRenewed( $borrowernumber1, $item_1->itemnumber ); is( $renewokay, 0, 'Bug 14337 - Verify the borrower can not renew with a hold on the record if AllowRenewalIfOtherItemsAvailable is enabled but the only available item is notforloan' ); -} +}; { # Don't allow renewing onsite checkout @@ -1340,7 +1342,7 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); - is( $error->{branch_to_return}, $holdingbranch->{branchcode} ); + is( $error->{branch_to_return}, $holdingbranch->{branchcode}, 'branch_to_return matched holdingbranch' ); ## Can be issued from holdinbranch set_userenv($holdingbranch); ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); @@ -1351,7 +1353,7 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); - is( $error->{branch_to_return}, $holdingbranch->{branchcode} ); + is( $error->{branch_to_return}, $holdingbranch->{branchcode}, 'branch_to_return matches holdingbranch' ); # AllowReturnToBranch == homebranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); @@ -1365,13 +1367,13 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); - is( $error->{branch_to_return}, $homebranch->{branchcode} ); + is( $error->{branch_to_return}, $homebranch->{branchcode}, 'branch_to_return matches homebranch' ); ## Cannot be issued from holdinbranch set_userenv($otherbranch); ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); - is( $error->{branch_to_return}, $homebranch->{branchcode} ); + is( $error->{branch_to_return}, $homebranch->{branchcode}, 'branch_to_return matches homebranch' ); # TODO t::lib::Mocks::mock_preference('AllowReturnToBranch', 'homeorholdingbranch'); }; @@ -1410,42 +1412,42 @@ subtest 'AddIssue & AllowReturnToBranch' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); ## Can be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from homebranch'); set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue ## Can be issued from holdinbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from holdingbranch'); set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue ## Can be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from otherbranch'); set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue # AllowReturnToBranch == holdinbranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); ## Cannot be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '' ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from homebranch'); ## Can be issued from holdingbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - holdingbranch | Can be issued from holdingbranch'); set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue ## Cannot be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '' ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from otherbranch'); # AllowReturnToBranch == homebranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); ## Can be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - homebranch | Can be issued from homebranch' ); set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue ## Cannot be issued from holdinbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '' ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from holdingbranch' ); ## Cannot be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '' ); + is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from otherbranch' ); # TODO t::lib::Mocks::mock_preference('AllowReturnToBranch', 'homeorholdingbranch'); }; -- 2.20.1