Allow only stubbed changes to Spree::Config in specs

api/spec/models/spree/legacy_user_spec.rb

@@ -54,7 +54,7 @@ module Spree
 
           before {
             user.clear_spree_api_key!
-            Spree::Config.roles_for_auto_api_key = ['hobbit']
+            stub_spree_preferences roles_for_auto_api_key: ['hobbit']
           }
 
           it { expect { subject }.to change { user.reload.spree_api_key }.from(nil) }
@@ -69,7 +69,7 @@ module Spree
           before {
             user.clear_spree_api_key!
             other_user.clear_spree_api_key!
-            Spree::Config.generate_api_key_for_all_roles = true
+            stub_spree_preferences(generate_api_key_for_all_roles: true)
           }
 
           it { expect { subject }.to change { user.reload.spree_api_key }.from(nil) }
@@ -89,7 +89,7 @@ module Spree
           end
 
           it "grants an api key on create when set to true" do
-            Spree::Config.generate_api_key_for_all_roles = true
+            stub_spree_preferences(generate_api_key_for_all_roles: true)
 
             expect(user.spree_api_key).to eq(nil)
 

api/spec/requests/spree/api/checkouts_controller_spec.rb

@@ -6,7 +6,7 @@ module Spree
   describe Api::CheckoutsController, type: :request do
     before(:each) do
       stub_authentication!
-      Spree::Config[:track_inventory_levels] = false
+      stub_spree_preferences track_inventory_levels: false
       country_zone = create(:zone, name: 'CountryZone')
       @state = create(:state)
       @country = @state.country
@@ -17,10 +17,6 @@ module Spree
       @payment_method = create(:credit_card_payment_method)
     end
 
-    after do
-      Spree::Config[:track_inventory_levels] = true
-    end
-
     context "PUT 'update'" do
       let(:order) do
         order = create(:order_with_line_items)

api/spec/requests/spree/api/orders_controller_spec.rb

@@ -865,7 +865,7 @@ module Spree
 
       context "can cancel an order" do
         before do
-          Spree::Config[:mails_from] = "spree@example.com"
+          stub_spree_preferences(mails_from: "spree@example.com")
 
           order.completed_at = Time.current
           order.state = 'complete'

api/spec/requests/spree/api/products_controller_spec.rb

@@ -158,15 +158,13 @@ module Spree
       end
 
       context "tracking is disabled" do
-        before { Config.track_inventory_levels = false }
+        before { stub_spree_preferences(track_inventory_levels: false) }
 
         it "still displays valid json with total_on_hand Float::INFINITY" do
           get spree.api_product_path(product)
           expect(response).to be_ok
           expect(json_response[:total_on_hand]).to eq nil
         end
-
-        after { Config.track_inventory_levels = true }
       end
 
       context "finds a product by slug first then by id" do
@@ -296,7 +294,7 @@ module Spree
         end
 
         context "when tracking is disabled" do
-          before { Config.track_inventory_levels = false }
+          before { stub_spree_preferences(track_inventory_levels: false) }
 
           it "still displays valid json with total_on_hand Float::INFINITY" do
             post spree.api_products_path, params: {
@@ -310,8 +308,6 @@ module Spree
             expect(response.status).to eq(201)
             expect(json_response['total_on_hand']).to eq nil
           end
-
-          after { Config.track_inventory_levels = true }
         end
 
         it "puts the created product in the given taxon" do

api/spec/requests/spree/api/variants_controller_spec.rb

@@ -251,16 +251,14 @@ module Spree
 
       context "when tracking is disabled" do
         before do
-          Config.track_inventory_levels = false
+          stub_spree_preferences(track_inventory_levels: false)
           subject
         end
 
         it "still displays valid json with total_on_hand Float::INFINITY" do
           expect(response.status).to eq(200)
           expect(json_response['total_on_hand']).to eq nil
         end
-
-        after { Config.track_inventory_levels = true }
       end
     end
 

api/spec/spec_helper.rb

@@ -59,7 +59,6 @@
 
   config.before(:each) do
     Rails.cache.clear
-    reset_spree_preferences
     Spree::Api::Config[:requires_authentication] = true
   end
 

backend/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb

@@ -17,12 +17,12 @@
           let!(:checkout_zone) { create(:zone, name: "Checkout Zone", countries: [canada]) }
 
           before do
-            Spree::Config.checkout_zone = checkout_zone.name
+            stub_spree_preferences(checkout_zone: checkout_zone.name)
           end
 
           context "and default_country_iso of the Canada" do
             before do
-              Spree::Config.default_country_iso = Spree::Country.find_by!(iso: "CA").iso
+              stub_spree_preferences(default_country_iso: Spree::Country.find_by!(iso: "CA").iso)
             end
 
             it 'defaults the shipping address country to Canada' do
@@ -38,7 +38,7 @@
 
           context "and default_country_iso of the United States" do
             before do
-              Spree::Config.default_country_iso = Spree::Country.find_by!(iso: "US").iso
+              stub_spree_preferences(default_country_iso: Spree::Country.find_by!(iso: "US").iso)
             end
 
             it 'defaults the shipping address country to nil' do

backend/spec/controllers/spree/admin/orders_controller_spec.rb

@@ -140,7 +140,7 @@
         end
 
         context 'when order_bill_address_used is true' do
-          before { Spree::Config[:order_bill_address_used] = true }
+          before { stub_spree_preferences(order_bill_address_used: true) }
 
           it "should redirect to the customer details page" do
             get :edit, params: { id: order.number }
@@ -149,7 +149,7 @@
         end
 
         context 'when order_bill_address_used is false' do
-          before { Spree::Config[:order_bill_address_used] = false }
+          before { stub_spree_preferences(order_bill_address_used: false) }
 
           it "should redirect to the customer details page" do
             get :edit, params: { id: order.number }

backend/spec/controllers/spree/admin/stock_locations_controller_spec.rb

@@ -20,7 +20,7 @@ module Admin
         let(:country) { create :country, iso: "BR" }
 
         before do
-          Spree::Config[:default_country_iso] = country.iso
+          stub_spree_preferences(default_country_iso: country.iso)
         end
 
         it "can create a new stock location" do

backend/spec/features/admin/orders/customer_details_spec.rb

@@ -70,10 +70,10 @@
 
   context "editing an order" do
     before do
-      configure_spree_preferences do |config|
-        config.default_country_iso = country.iso
-        config.company = true
-      end
+      stub_spree_preferences(
+        default_country_iso: country.iso,
+        company: true
+      )
 
       visit spree.admin_path
       click_link "Orders"
@@ -153,9 +153,7 @@
 
       before do
         order.bill_address.country.destroy
-        configure_spree_preferences do |config|
-          config.default_country_iso = brazil.iso
-        end
+        stub_spree_preferences(default_country_iso: brazil.iso)
       end
 
       it "sets default country when displaying form" do

backend/spec/features/admin/orders/listing_spec.rb

@@ -114,12 +114,7 @@
 
       context "when pagination is really short" do
         before do
-          @old_per_page = Spree::Config[:orders_per_page]
-          Spree::Config[:orders_per_page] = 1
-        end
-
-        after do
-          Spree::Config[:orders_per_page] = @old_per_page
+          stub_spree_preferences(orders_per_page: 1)
         end
 
         # Regression test for https://github.com/spree/spree/issues/4004

backend/spec/features/admin/orders/new_order_spec.rb

@@ -208,12 +208,12 @@
 
     before do
       Spree::Country.update_all(states_required: true)
-      Spree::Config.checkout_zone = checkout_zone.name
+      stub_spree_preferences(checkout_zone: checkout_zone.name)
     end
 
     context 'and default_country_iso of the United States' do
       before do
-        Spree::Config.default_country_iso = Spree::Country.find_by!(iso: 'US').iso
+        stub_spree_preferences(default_country_iso: Spree::Country.find_by!(iso: 'US').iso)
       end
 
       it 'the shipping address country select includes only options for Canada' do
@@ -267,7 +267,7 @@
 
     context 'and default_country_iso of Canada' do
       before do
-        Spree::Config.default_country_iso = Spree::Country.find_by!(iso: 'CA').iso
+        stub_spree_preferences(default_country_iso: Spree::Country.find_by!(iso: 'CA').iso)
       end
 
       it 'defaults the shipping address country to Canada' do

backend/spec/features/admin/products/products_spec.rb

@@ -54,7 +54,7 @@ def build_option_type_with_values(name, values)
       context "currency displaying" do
         context "using Russian Rubles" do
           before do
-            Spree::Config[:currency] = "RUB"
+            stub_spree_preferences(currency: "RUB")
           end
 
           let!(:product) do
@@ -72,15 +72,15 @@ def build_option_type_with_values(name, values)
       end
       context "when none of the product prices are in the same currency as the default in the store" do
         before do
-          Spree::Config[:currency] = "MXN"
+          stub_spree_preferences(currency: "MXN")
         end
 
         let!(:product) do
           create(:product, name: "Just a product", price: 19.99)
         end
 
         it 'defaults it to Spree::Config.currency and sets the price as blank' do
-          Spree::Config[:currency] = "USD"
+          stub_spree_preferences(currency: "USD")
           visit spree.admin_product_path(product)
           within("#product_price_field") do
             expect(page).to have_content("USD")

backend/spec/features/admin/products/variant_spec.rb

@@ -32,7 +32,7 @@
     context "currency displaying" do
       context "using Russian Rubles" do
         before do
-          Spree::Config[:currency] = "RUB"
+          stub_spree_preferences(currency: "RUB")
         end
 
         let!(:variant) do

backend/spec/spec_helper.rb

@@ -89,7 +89,6 @@
 
   config.before do
     Rails.cache.clear
-    reset_spree_preferences
     if RSpec.current_example.metadata[:js] && page.driver.browser.respond_to?(:url_blacklist)
       page.driver.browser.url_blacklist = ['http://fonts.googleapis.com']
     end

core/lib/spree/testing_support/preferences.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'spree/deprecation'
+
 module Spree
   module TestingSupport
     module Preferences
@@ -10,6 +12,7 @@ module Preferences
       #   config.track_inventory_levels = false
       # end
       #
+      # @deprecated
       def reset_spree_preferences(&config_block)
         Spree::Config.instance_variables.each { |iv| Spree::Config.remove_instance_variable(iv) }
         Spree::Config.preference_store = Spree::Config.default_preferences
@@ -21,6 +24,8 @@ def reset_spree_preferences(&config_block)
         configure_spree_preferences(&config_block) if block_given?
       end
 
+      deprecate :reset_spree_preferences, deprecator: Spree::Deprecation
+
       def configure_spree_preferences
         yield(Spree::Config) if block_given?
       end
@@ -29,6 +34,63 @@ def assert_preference_unset(preference)
         find("#preferences_#{preference}")['checked'].should be false
         Spree::Config[preference].should be false
       end
+
+      # This is the preferred way for changing temporarily Spree preferences during
+      # tests via stubs, without changing the actual values stored in Spree::Config.
+      #
+      # By using stubs no global preference change will leak outside the lifecycle
+      # of each spec example, avoiding possible unpredictable side effects.
+      #
+      # This method  may be used for stubbing one or more different preferences
+      # at the same time.
+      #
+      # @param [Hash] preferences names and values to be stubbed
+      #
+      # @example Stubs `currency` and `track_inventory_levels` preferences
+      #   stub_spree_preferences(currency: 'EUR', track_inventory_levels: false)
+      #   expect(Spree::Config.currency).to eql 'EUR'
+      #
+      # @see https://github.com/solidusio/solidus/issues/3219
+      #   Solidus #3219 for more details and motivations.
+      def stub_spree_preferences(preferences)
+        preferences.each do |name, value|
+          if Spree::Config.method(:[]).owner >= Spree::Config.class
+            allow(Spree::Config).to receive(:[]).and_call_original
+          end
+          allow(Spree::Config).to receive(:[]).with(name) { value }
+          allow(Spree::Config).to receive(name) { value }
+        end
+      end
+
+      # This method allows to temporarily switch to an unfrozen Spree::Config preference
+      # store with all proper preferences values set.
+      #
+      # It should be used sparingly, only when `stub_spree_preferences` would not work.
+      #
+      # @example Temporarily switch to an unfrozen store and change some preferences:
+      #   with_unfrozen_spree_preference_store do
+      #     Spree::Config.currency = 'EUR'
+      #     Spree::Config.track_inventory_levels = false
+      #
+      #     expect(Spree::Config.currency).to eql 'EUR'
+      #   end
+      # @see Spree::TestingSupport::Preferences#stub_spree_preferences
+      def with_unfrozen_spree_preference_store
+        frozen_store = Spree::Config.preference_store
+        Spree::Config.preference_store = Spree::Config[:unfrozen_preference_store].dup
+        yield
+      ensure
+        Spree::Config.preference_store = frozen_store
+      end
     end
   end
 end
+
+RSpec.configure do |config|
+  config.before :suite do
+    # keep a copy of the original unfrozen preference_store for later use:
+    Spree::AppConfiguration.preference :unfrozen_preference_store, :hash
+    Spree::Config.unfrozen_preference_store = Spree::Config.preference_store.dup
+    Spree::Config.preference_store.freeze
+  end
+end

core/spec/helpers/base_helper_spec.rb

@@ -16,7 +16,7 @@
 
     context "with no checkout zone defined" do
       before do
-        Spree::Config[:checkout_zone] = nil
+        stub_spree_preferences(checkout_zone: nil)
       end
 
       it "return complete list of countries" do
@@ -33,7 +33,7 @@
         before do
           @country_zone = create(:zone, name: "CountryZone")
           @country_zone.members.create(zoneable: country)
-          Spree::Config[:checkout_zone] = @country_zone.name
+          stub_spree_preferences(checkout_zone: @country_zone.name)
         end
 
         it "return only the countries defined by the checkout zone" do
@@ -50,7 +50,7 @@
           state_zone = create(:zone, name: "StateZone")
           state = create(:state, country: country)
           state_zone.members.create(zoneable: state)
-          Spree::Config[:checkout_zone] = state_zone.name
+          stub_spree_preferences(checkout_zone: state_zone.name)
         end
 
         it "return complete list of countries" do