From bcfb46296fbab66410c3d099fbfcfd2d27f1dddd Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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
---
 .../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.1.4