From 0ff81f52fde8c5e0e59f5b7276142f2c08edfadf Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 15 Nov 2016 16:07:35 +0000 Subject: [PATCH] Bug 17633: Do not call set_preference from tests If memcached is running, the pref's value will be set in cache, what we do not want. t/db_dependent/Context.t and t/db_dependent/sysprefs.t are exceptions for obvious reasons. Test plan: Make sure all these tests still pass Signed-off-by: Josef Moravec Signed-off-by: Tomas Cohen Arazi --- .../Circulation/NoIssuesChargeGuarantees.t | 3 +- t/db_dependent/Circulation/Returns.t | 16 ++++---- t/db_dependent/DecreaseLoanHighHolds.t | 17 ++++---- t/db_dependent/HoldsQueue.t | 47 +++++++++++----------- t/db_dependent/Items.t | 4 +- t/db_dependent/Template/Plugin/Branches.t | 5 ++- 6 files changed, 48 insertions(+), 44 deletions(-) diff --git a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t index a5cbb62..dc2c880 100644 --- a/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t +++ b/t/db_dependent/Circulation/NoIssuesChargeGuarantees.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::More tests => 2; use t::lib::TestBuilder; +use t::lib::Mocks; use C4::Accounts qw( manualinvoice ); use C4::Circulation qw( CanBookBeIssued ); @@ -53,7 +54,7 @@ my $guarantee = $builder->build( } ); -C4::Context->set_preference( 'NoIssuesChargeGuarantees', '5.00' ); +t::lib::Mocks::mock_preference( 'NoIssuesChargeGuarantees', '5.00' ); my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->{barcode} ); is( $issuingimpossible->{DEBT_GUARANTEES}, undef, "Patron can check out item" ); diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index 280406a..06c44e6 100644 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -70,7 +70,7 @@ subtest "InProcessingToShelvingCart tests" => sub { my $itemnumber = $built_item->{ itemnumber }; my $item; - C4::Context->set_preference( "InProcessingToShelvingCart", 1 ); + t::lib::Mocks::mock_preference( "InProcessingToShelvingCart", 1 ); AddReturn( $barcode, $branch ); $item = GetItem( $itemnumber ); is( $item->{location}, 'CART', @@ -79,7 +79,7 @@ subtest "InProcessingToShelvingCart tests" => sub { $item->{location} = $location; ModItem( $item, undef, $itemnumber ); - C4::Context->set_preference( "InProcessingToShelvingCart", 0 ); + t::lib::Mocks::mock_preference( "InProcessingToShelvingCart", 0 ); AddReturn( $barcode, $branch ); $item = GetItem( $itemnumber ); is( $item->{location}, $permanent_location, @@ -92,11 +92,11 @@ subtest "AddReturn logging on statistics table (item-level_itypes=1)" => sub { plan tests => 2; # Set item-level item types - C4::Context->set_preference( "item-level_itypes", 1 ); + t::lib::Mocks::mock_preference( "item-level_itypes", 1 ); # Make sure logging is enabled - C4::Context->set_preference( "IssueLog", 1 ); - C4::Context->set_preference( "ReturnLog", 1 ); + t::lib::Mocks::mock_preference( "IssueLog", 1 ); + t::lib::Mocks::mock_preference( "ReturnLog", 1 ); # Create an itemtype for biblio-level item type my $blevel_itemtype = $builder->build({ source => 'Itemtype' })->{ itemtype }; @@ -180,11 +180,11 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub { plan tests => 2; # Make sure logging is enabled - C4::Context->set_preference( "IssueLog", 1 ); - C4::Context->set_preference( "ReturnLog", 1 ); + t::lib::Mocks::mock_preference( "IssueLog", 1 ); + t::lib::Mocks::mock_preference( "ReturnLog", 1 ); # Set item-level item types - C4::Context->set_preference( "item-level_itypes", 0 ); + t::lib::Mocks::mock_preference( "item-level_itypes", 0 ); # Create an itemtype for biblio-level item type my $blevel_itemtype = $builder->build({ source => 'Itemtype' })->{ itemtype }; diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index ca14fb4..b6a175c 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -25,6 +25,7 @@ use Koha::Item; use Koha::Holds; use Koha::Hold; use t::lib::TestBuilder; +use t::lib::Mocks; use Test::More tests => 14; @@ -103,11 +104,11 @@ $builder->build( my $item = pop(@items); my $patron = pop(@patrons); -C4::Context->set_preference( 'decreaseLoanHighHolds', 1 ); -C4::Context->set_preference( 'decreaseLoanHighHoldsDuration', 1 ); -C4::Context->set_preference( 'decreaseLoanHighHoldsValue', 1 ); -C4::Context->set_preference( 'decreaseLoanHighHoldsControl', 'static' ); -C4::Context->set_preference( 'decreaseLoanHighHoldsIgnoreStatuses', 'damaged,itemlost,notforloan,withdrawn' ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHolds', 1 ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsDuration', 1 ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 1 ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsControl', 'static' ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsIgnoreStatuses', 'damaged,itemlost,notforloan,withdrawn' ); my $item_hr = { itemnumber => $item->id, biblionumber => $biblio->id, homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, barcode => $item->barcode }; my $patron_hr = { borrowernumber => $patron->id, branchcode => $library->{branchcode} }; @@ -118,7 +119,7 @@ is( $data->{outstanding}, 6, "Should have 5 outstanding holds" ); is( $data->{duration}, 1, "Should have duration of 1" ); is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); -C4::Context->set_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsControl', 'dynamic' ); $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); is( $data->{exceeded}, 0, "Should not exceed threshold" ); @@ -136,7 +137,7 @@ for my $i ( 5 .. 10 ) { $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); is( $data->{exceeded}, 1, "Should exceed threshold of 1" ); -C4::Context->set_preference( 'decreaseLoanHighHoldsValue', 2 ); +t::lib::Mocks::mock_preference( 'decreaseLoanHighHoldsValue', 2 ); $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); is( $data->{exceeded}, 0, "Should not exceed threshold of 2" ); @@ -168,7 +169,7 @@ $unholdable->store(); $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); is( $data->{exceeded}, 1, "Should exceed threshold with one withdrawn item" ); -C4::Context->set_preference('CircControl', 'PatronLibrary'); +t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary'); my ( undef, $needsconfirmation ) = CanBookBeIssued( $patron_hr, $item->barcode ); ok( $needsconfirmation->{HIGHHOLDS}, "High holds checkout needs confirmation" ); diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index 43043d5..2d62b19 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -18,6 +18,7 @@ use Koha::Database; use Koha::DateUtils; use t::lib::TestBuilder; +use t::lib::Mocks; BEGIN { use FindBin; @@ -61,8 +62,8 @@ my $itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => #Set up the stage # Sysprefs and cost matrix -C4::Context->set_preference('HoldsQueueSkipClosed', 0); -C4::Context->set_preference('LocalHoldsPriority', 0); +t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 0); +t::lib::Mocks::mock_preference('LocalHoldsPriority', 0); $dbh->do("UPDATE systempreferences SET value = ? WHERE variable = 'StaticHoldsQueueWeight'", undef, join( ',', @other_branches, $borrower_branchcode, $least_cost_branch_code)); $dbh->do("UPDATE systempreferences SET value = '0' WHERE variable = 'RandomizeHoldsQueueWeight'"); @@ -118,7 +119,7 @@ my $test_sth = $dbh->prepare("SELECT * FROM hold_fill_targets WHERE borrowernumber = $borrowernumber"); # We have a book available homed in borrower branch, no point fiddling with AutomaticItemReturn -C4::Context->set_preference('AutomaticItemReturn', 0); +t::lib::Mocks::mock_preference('AutomaticItemReturn', 0); test_queue ('take from homebranch', 0, $borrower_branchcode, $borrower_branchcode); test_queue ('take from homebranch', 1, $borrower_branchcode, $borrower_branchcode); @@ -127,14 +128,14 @@ $dbh->do("DELETE FROM hold_fill_targets"); $dbh->do("DELETE FROM issues WHERE itemnumber IN (SELECT itemnumber FROM items WHERE homebranch = '$borrower_branchcode' AND holdingbranch = '$borrower_branchcode')"); $dbh->do("DELETE FROM items WHERE homebranch = '$borrower_branchcode' AND holdingbranch = '$borrower_branchcode'"); # test_queue will flush -C4::Context->set_preference('AutomaticItemReturn', 1); +t::lib::Mocks::mock_preference('AutomaticItemReturn', 1); # Not sure how to make this test more difficult - holding branch does not matter $dbh->do("DELETE FROM tmp_holdsqueue"); $dbh->do("DELETE FROM hold_fill_targets"); $dbh->do("DELETE FROM issues WHERE itemnumber IN (SELECT itemnumber FROM items WHERE homebranch = '$borrower_branchcode')"); $dbh->do("DELETE FROM items WHERE homebranch = '$borrower_branchcode'"); -C4::Context->set_preference('AutomaticItemReturn', 0); +t::lib::Mocks::mock_preference('AutomaticItemReturn', 0); # We have a book available held in borrower branch test_queue ('take from holdingbranch', 0, $borrower_branchcode, $borrower_branchcode); test_queue ('take from holdingbranch', 1, $borrower_branchcode, $borrower_branchcode); @@ -173,7 +174,7 @@ $dbh->do("DELETE FROM default_branch_circ_rules"); $dbh->do("DELETE FROM default_branch_item_rules"); $dbh->do("DELETE FROM default_circ_rules"); -C4::Context->set_preference('UseTransportCostMatrix', 0); +t::lib::Mocks::mock_preference('UseTransportCostMatrix', 0); $itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } })->{itemtype}; @@ -299,7 +300,7 @@ is( $holds_queue->[1]->{cardnumber}, $borrower2->{cardnumber}, "Holds queue fill # 1 of which is coming from MPL. Let's enable HoldsQueueSkipClosed # and make today a holiday for MPL. When we run it again we should only # have 1 row in the holds queue -C4::Context->set_preference('HoldsQueueSkipClosed', 1); +t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 1); my $today = dt_from_string(); C4::Calendar->new( branchcode => $branchcodes[0] )->insert_single_holiday( day => $today->day(), @@ -314,7 +315,7 @@ is( Koha::Calendar->new( branchcode => $branchcodes[0] )->is_holiday( $today ), C4::HoldsQueue::CreateQueue(); $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); is( scalar( @$holds_queue ), 1, "Holds not filled with items from closed libraries" ); -C4::Context->set_preference('HoldsQueueSkipClosed', 0); +t::lib::Mocks::mock_preference('HoldsQueueSkipClosed', 0); $dbh->do("DELETE FROM default_circ_rules"); $dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )"); @@ -327,22 +328,22 @@ is( @$holds_queue, 3, "Holds queue filling correct number for holds for default # one of which is coming from MPL. Let's enable HoldsQueueSkipClosed # and use our previously created holiday for MPL # When we run it again we should only have 2 rows in the holds queue -C4::Context->set_preference( 'HoldsQueueSkipClosed', 1 ); +t::lib::Mocks::mock_preference( 'HoldsQueueSkipClosed', 1 ); C4::HoldsQueue::CreateQueue(); $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); is( scalar( @$holds_queue ), 2, "Holds not filled with items from closed libraries" ); -C4::Context->set_preference( 'HoldsQueueSkipClosed', 0 ); +t::lib::Mocks::mock_preference( 'HoldsQueueSkipClosed', 0 ); ## Test LocalHoldsPriority -C4::Context->set_preference('LocalHoldsPriority', 1); +t::lib::Mocks::mock_preference('LocalHoldsPriority', 1); $dbh->do("DELETE FROM default_circ_rules"); $dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )"); $dbh->do("DELETE FROM issues"); # Test homebranch = patron branch -C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); -C4::Context->set_preference('LocalHoldsPriorityItemControl', 'homebranch'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'homebranch'); C4::Context->clear_syspref_cache(); $dbh->do("DELETE FROM reserves"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -358,8 +359,8 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's home library"); # Test holdingbranch = patron branch -C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); -C4::Context->set_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'HomeLibrary'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); C4::Context->clear_syspref_cache(); $dbh->do("DELETE FROM reserves"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -375,8 +376,8 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); # Test holdingbranch = pickup branch -C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); -C4::Context->set_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'holdingbranch'); C4::Context->clear_syspref_cache(); $dbh->do("DELETE FROM reserves"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -392,8 +393,8 @@ $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); # Test homebranch = pickup branch -C4::Context->set_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); -C4::Context->set_preference('LocalHoldsPriorityItemControl', 'homebranch'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityPatronControl', 'PickupLibrary'); +t::lib::Mocks::mock_preference('LocalHoldsPriorityItemControl', 'homebranch'); C4::Context->clear_syspref_cache(); $dbh->do("DELETE FROM reserves"); $sth->execute( $borrower1->{borrowernumber}, $biblionumber, $branchcodes[0], 1 ); @@ -408,7 +409,7 @@ C4::HoldsQueue::CreateQueue(); $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} }); is( $holds_queue->[0]->{cardnumber}, $borrower3->{cardnumber}, "Holds queue giving priority to patron who's home library matches item's holding library"); -C4::Context->set_preference('LocalHoldsPriority', 0); +t::lib::Mocks::mock_preference('LocalHoldsPriority', 0); ## End testing of LocalHoldsPriority @@ -487,7 +488,7 @@ $dbh->do("DELETE FROM default_branch_item_rules"); $dbh->do("DELETE FROM default_circ_rules"); $dbh->do("DELETE FROM branch_item_rules"); -C4::Context->set_preference("UseTransportCostMatrix",1); +t::lib::Mocks::mock_preference("UseTransportCostMatrix",1); my $tc_rs = $schema->resultset('TransportCost'); $tc_rs->create({ frombranch => $library_A, tobranch => $library_B, cost => 0, disable_transfer => 1 }); @@ -518,7 +519,7 @@ is( @$holds_queue, 0, "Bug 15062 - Holds queue with Transport Cost Matrix will t # End Bug 15062 # Test hold_fulfillment_policy -C4::Context->set_preference( "UseTransportCostMatrix", 0 ); +t::lib::Mocks::mock_preference( "UseTransportCostMatrix", 0 ); $borrowernumber = $borrower3->{borrowernumber}; $library_A = $library1->{branchcode}; $library_B = $library2->{branchcode}; @@ -630,7 +631,7 @@ CancelReserve( { reserve_id => $reserve_id } ); # End testing hold_fulfillment_policy # Test hold itemtype limit -C4::Context->set_preference( "UseTransportCostMatrix", 0 ); +t::lib::Mocks::mock_preference( "UseTransportCostMatrix", 0 ); my $wrong_itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } })->{itemtype}; my $right_itemtype = $builder->build({ source => 'Itemtype', value => { notforloan => 0 } })->{itemtype}; $borrowernumber = $borrower3->{borrowernumber}; diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 7d0fae4..19a8a36 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -93,10 +93,10 @@ subtest 'General Add, Get and Del tests' => sub { is( $getitem->{location}, 'CART', "The location should have been set to CART" ); is( $getitem->{permanent_location}, $location, "The permanent_location should not have been set to CART" ); - C4::Context->set_preference('item-level_itypes', '1'); + t::lib::Mocks::mock_preference('item-level_itypes', '1'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, $itemtype->{itemtype}, "Itemtype set correctly when using item_level-itypes" ); - C4::Context->set_preference('item-level_itypes', '0'); + t::lib::Mocks::mock_preference('item-level_itypes', '0'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, undef, "Itemtype set correctly when not using item_level-itypes" ); diff --git a/t/db_dependent/Template/Plugin/Branches.t b/t/db_dependent/Template/Plugin/Branches.t index 24b6e6c..507cea9 100644 --- a/t/db_dependent/Template/Plugin/Branches.t +++ b/t/db_dependent/Template/Plugin/Branches.t @@ -22,6 +22,7 @@ use C4::Context; use Koha::Database; use t::lib::TestBuilder; +use t::lib::Mocks; BEGIN { use_ok('Koha::Template::Plugin::Branches'); @@ -63,7 +64,7 @@ C4::Context->set_userenv(123, 'userid', 'usercnum', 'First name', 'Surname', 'MY $library = $plugin->GetLoggedInBranchcode(); is($library, 'MYLIBRARY', 'GetLoggedInBranchcode() returns active library'); -C4::Context->set_preference( 'IndependentBranches', 0 ); +t::lib::Mocks::mock_preference( 'IndependentBranches', 0 ); my $libraries = $plugin->all(); ok( scalar(@$libraries) > 1, 'If IndependentBranches is not set, all libraries should be returned' ); is( grep ( { $_->{branchcode} eq 'MYLIBRARY' and $_->{selected} == 1 } @$libraries ), 1, 'Without selected parameter, my library should be preselected' ); @@ -74,7 +75,7 @@ is( grep ( { $_->{branchcode} eq 'ANOTHERLIB' and $_->{selected} == 1 } @$librar $libraries = $plugin->all( { selected => '' } ); is( grep ( { exists $_->{selected} } @$libraries ), 0, 'With selected parameter set to an empty string, no library should be preselected' ); -C4::Context->set_preference( 'IndependentBranches', 1 ); +t::lib::Mocks::mock_preference( 'IndependentBranches', 1 ); $libraries = $plugin->all(); is( scalar(@$libraries), 1, 'If IndependentBranches is set, only 1 library should be returned' ); $libraries = $plugin->all( { unfiltered => 1 } ); -- 2.7.4