From 5b74a81e3d1dcb3e8a351126503605baa20241a8 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 19 Sep 2013 16:20:28 +0200 Subject: [PATCH] Bug 10683: (follow-up) improvements to the unit tests The borrowers table needs to be cleared after the items table (last_returned_by column). Some checks were missing for GetRenewCount and AddRenewal. Now the tests simulated a renewal for a item and check that the renews left is decremented. Moreover the issuingrules tables should be cleared and filled with known values. Signed-off-by: Jonathan Druart Signed-off-by: Galen Charlton --- t/db_dependent/Circulation_Branch.t | 2 +- t/db_dependent/Circulation_issue.t | 121 ++++++++++++++++++++++------ 2 files changed, 98 insertions(+), 25 deletions(-) diff --git a/t/db_dependent/Circulation_Branch.t b/t/db_dependent/Circulation_Branch.t index a2d0b90e7a..ee2645837d 100644 --- a/t/db_dependent/Circulation_Branch.t +++ b/t/db_dependent/Circulation_Branch.t @@ -26,8 +26,8 @@ $dbh->{RaiseError} = 1; $dbh->{AutoCommit} = 0; $dbh->do(q|DELETE FROM issues|); -$dbh->do(q|DELETE FROM borrowers|); $dbh->do(q|DELETE FROM items|); +$dbh->do(q|DELETE FROM borrowers|); $dbh->do(q|DELETE FROM branches|); $dbh->do(q|DELETE FROM categories|); $dbh->do(q|DELETE FROM accountlines|); diff --git a/t/db_dependent/Circulation_issue.t b/t/db_dependent/Circulation_issue.t index 09f4d3477d..7630027313 100644 --- a/t/db_dependent/Circulation_issue.t +++ b/t/db_dependent/Circulation_issue.t @@ -10,7 +10,7 @@ use C4::Circulation; use C4::Items; use C4::Context; -use Test::More tests => 16; +use Test::More tests => 24; BEGIN { use_ok('C4::Circulation'); @@ -38,20 +38,14 @@ $dbh->{RaiseError} = 1; $dbh->{AutoCommit} = 0; $dbh->do(q|DELETE FROM issues|); -$dbh->do(q|DELETE FROM borrowers|); $dbh->do(q|DELETE FROM items|); +$dbh->do(q|DELETE FROM borrowers|); $dbh->do(q|DELETE FROM branches|); $dbh->do(q|DELETE FROM categories|); $dbh->do(q|DELETE FROM accountlines|); +$dbh->do(q|DELETE FROM issuingrules|); #Add sample datas -my @USERENV = ( 1, 'test', 'MASTERTEST', 'Test', 'Test', 't', 'Test', 0, ); - -C4::Context->_new_userenv('DUMMY_SESSION_ID'); -C4::Context->set_userenv(@USERENV); - -my $userenv = C4::Context->userenv - or BAIL_OUT("No userenv"); #Add Dates @@ -66,7 +60,7 @@ my $daysago10 = output_pref( $dt_today2, 'iso', '24hr', 1 ); #Add branch and category my $samplebranch1 = { add => 1, - branchcode => 'SAB1', + branchcode => 'CPL', branchname => 'Sample Branch', branchaddress1 => 'sample adr1', branchaddress2 => 'sample adr2', @@ -85,7 +79,7 @@ my $samplebranch1 = { }; my $samplebranch2 = { add => 1, - branchcode => 'SAB2', + branchcode => 'MPL', branchname => 'Sample Branch2', branchaddress1 => 'sample adr1_2', branchaddress2 => 'sample adr2_2', @@ -137,11 +131,11 @@ $dbh->do( my $record = MARC::Record->new(); $record->append_fields( MARC::Field->new( '952', '0', '0', a => $samplebranch1->{branchcode} ) ); -my ( $biblionumber, $biblioitemnumber ) = C4::Biblio::AddBiblio( $record, '', ); +my ( $biblionumber, $biblioitemnumber ) = C4::Biblio::AddBiblio( $record, '' ); my @sampleitem1 = C4::Items::AddItem( { - barcode => 1, + barcode => 'barcode_1', itemcallnumber => 'callnumber1', homebranch => $samplebranch1->{branchcode}, holdingbranch => $samplebranch1->{branchcode}, @@ -153,7 +147,7 @@ my @sampleitem1 = C4::Items::AddItem( my $item_id1 = $sampleitem1[2]; my @sampleitem2 = C4::Items::AddItem( { - barcode => 2, + barcode => 'barcode_2', itemcallnumber => 'callnumber2', homebranch => $samplebranch2->{branchcode}, holdingbranch => $samplebranch2->{branchcode}, @@ -171,12 +165,25 @@ my $borrower_id1 = C4::Members::AddMember( categorycode => $samplecat->{categorycode}, branchcode => $samplebranch1->{branchcode}, ); +my $borrower_1 = C4::Members::GetMember(borrowernumber => $borrower_id1); my $borrower_id2 = C4::Members::AddMember( firstname => 'firstname2', surname => 'surname2 ', categorycode => $samplecat->{categorycode}, branchcode => $samplebranch2->{branchcode}, ); +my $borrower_2 = C4::Members::GetMember(borrowernumber => $borrower_id2); + +# NEED TO BE FIXED !!! +# The first parameter for set_userenv is the class ref +#my @USERENV = ( $borrower_id1, 'test', 'MASTERTEST', 'firstname', 'username', 'CPL', 'CPL', 'email@example.org' ); +my @USERENV = ( $borrower_id1, 'test', 'MASTERTEST', 'firstname', 'CPL', 'CPL', 'email@example.org' ); + +C4::Context->_new_userenv('DUMMY_SESSION_ID'); +C4::Context->set_userenv(@USERENV); + +my $userenv = C4::Context->userenv + or BAIL_OUT("No userenv"); #Begin Tests @@ -186,7 +193,7 @@ my $sth = $dbh->prepare($query); $sth->execute; my $countissue = $sth -> fetchrow_array; is ($countissue ,0, "there is no issue"); -my $datedue1 = C4::Circulation::AddIssue( $borrower_id1, "code", $daysago10,0, $today, '' ); +my $datedue1 = C4::Circulation::AddIssue( $borrower_1, "code", $daysago10,0, $today, '' ); like( $datedue1, qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/, @@ -194,7 +201,7 @@ like( ); my $issue_id1 = $dbh->last_insert_id( undef, undef, 'issues', undef ); -my $datedue2 = C4::Circulation::AddIssue( $borrower_id1, 'Barcode2' ); +my $datedue2 = C4::Circulation::AddIssue( $borrower_1, 'nonexistent_barcode' ); is( $datedue2, undef, "AddIssue returns undef if no datedue is specified" ); my $issue_id2 = $dbh->last_insert_id( undef, undef, 'issues', undef ); @@ -242,23 +249,89 @@ is(GetItemIssue,undef,"Without parameter GetItemIssue returns undef"); is( GetOpenIssue(), undef, "Without parameter GetOpenIssue returns undef" ); is( GetOpenIssue(-1), undef, "With wrong parameter GetOpenIssue returns undef" ); -my $openissue = GetOpenIssue($item_id1); +my $openissue = GetOpenIssue($borrower_id1, $item_id1); +my @renewcount; #Test GetRenewCount -my @renewcount = C4::Circulation::GetRenewCount(); +$datedue2 = C4::Circulation::AddIssue( $borrower_1, 'barcode_1' ); +isnt( $datedue2, undef, "AddIssue does not return undef if datedue is specified" ); +#Without anything in DB +@renewcount = C4::Circulation::GetRenewCount(); is_deeply( \@renewcount, - [ 0, 1, 1 ], -"Without paramater, GetRenewCount returns renewcount0,renewsallowed = 0,renewsleft = 0" + [ 0, undef, 0 ], # FIXME Need to be fixed + "Without issuing rules and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = undef, renewsleft = 0" ); @renewcount = C4::Circulation::GetRenewCount(-1); is_deeply( \@renewcount, - [ 0, 1, 1 ], -"Without wrong, GetRenewCount returns renewcount0,renewsallowed = 0,renewsleft = 0" + [ 0, undef, 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, + [ 0, undef, 0 ], # FIXME Need to be fixed + "Without issuing rules and with a valid parameter, renewcount = 0, renewsallowed = undef, renewsleft = 0" ); -@renewcount = C4::Circulation::GetRenewCount($item_id1); -is_deeply( \@renewcount, [ 0, 1, 1 ], "Getrenewcount of item1 returns" ); + +#With something in DB +# Add a default rule: No renewal allowed +$dbh->do(q| + INSERT INTO issuingrules( categorycode, itemtype, branchcode, issuelength, renewalsallowed ) + VALUES ( '*', '*', '*', 10, 0 ) +|); +@renewcount = C4::Circulation::GetRenewCount(); +is_deeply( + \@renewcount, + [ 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 ], + "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, + [ 0, 0, 0 ], + "With issuing rules (renewal disallowed) and with a valid parameter, Getrenewcount returns renewcount = 0, renewsallowed = 0, renewsleft = 0" +); + +# Add a default rule: renewal is allowed +$dbh->do(q| + UPDATE issuingrules SET renewalsallowed = 3 +|); +@renewcount = C4::Circulation::GetRenewCount(); +is_deeply( + \@renewcount, + [ 0, 3, 3 ], + "With issuing rules (renewal allowed) and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = 3, renewsleft = 3" +); +@renewcount = C4::Circulation::GetRenewCount(-1); +is_deeply( + \@renewcount, + [ 0, 3, 3 ], + "With issuing rules (renewal allowed) and without wrong parameter, GetRenewCount returns renewcount = 0, renewsallowed = 3, renewsleft = 3" +); +@renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1); +is_deeply( + \@renewcount, + [ 0, 3, 3 ], + "With issuing rules (renewal allowed) and with a valid parameter, Getrenewcount of item1 returns 3 renews left" +); + +AddRenewal( $borrower_id1, $item_id1, $samplebranch1->{branchcode}, + $datedue3, $daysago10 ); +@renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1); +is_deeply( + \@renewcount, + [ 1, 3, 2 ], + "With issuing rules (renewal allowed, 2 remaining) and with a valid parameter, Getrenewcount of item1 returns 2 renews left" +); + #End transaction $dbh->rollback; -- 2.39.5