From c3c5b0f48e9efa5eea6a3e48facf5a09ef490422 Mon Sep 17 00:00:00 2001 From: benjamin wil Date: Thu, 25 Jul 2024 16:02:54 -0700 Subject: [PATCH 1/2] Test `Spree.deprecator` functionality These tests were written to aid in debugging an issue: solidusio/solidus#5766 Here we expose an issue using a pending test, with the ENV that allows overriding of the default deprecation behavior, which gets lost during the initialization of the `DummyApp`. Co-authored-by: Chris Todorov --- core/spec/lib/spree/deprecator_spec.rb | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 core/spec/lib/spree/deprecator_spec.rb diff --git a/core/spec/lib/spree/deprecator_spec.rb b/core/spec/lib/spree/deprecator_spec.rb new file mode 100644 index 00000000000..381e90bcbc2 --- /dev/null +++ b/core/spec/lib/spree/deprecator_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe "Spree.deprecator" do + Dummy = Class.new do + def deprecated_method + "foo" + end + deprecate :deprecated_method, deprecator: Spree.deprecator + end + + context "by default" do + it "returns a valid deprecator" do + expect(Spree.deprecator).to have_attributes( + deprecation_horizon: "5.0", + gem_name: "Solidus" + ) + end + + it "does not raise an error unless overridden by environment" do + if ENV["SOLIDUS_RAISE_DEPRECATIONS"] + pending "fix for deprecation behaviour override" + expect { Dummy.new.deprecated_method }.to raise_error(ActiveSupport::DeprecationException) + else + expect { Dummy.new.deprecated_method }.not_to raise_error + end + end + end + + context "when the behavior has been changed to :raise" do + around do |example| + behavior_name = ActiveSupport::Deprecation::DEFAULT_BEHAVIORS.detect { |_behavior_name, behavior_proc| + behavior_proc == Spree.deprecator.behavior.first + }.first + + Spree.deprecator.behavior = :raise + + example.run + + Spree.deprecator.behavior = behavior_name + end + + it "raises an error when a deprecated method is called" do + expect { Dummy.new.deprecated_method } + .to raise_error(ActiveSupport::DeprecationException) + end + end +end From 586c5196f0cb9539a7168718bbdcb56a1a5b83e4 Mon Sep 17 00:00:00 2001 From: Chris Todorov Date: Wed, 7 Aug 2024 14:41:41 -0700 Subject: [PATCH 2/2] Resolve load order issue with deprecator behaviour This change fixes issue solidusio/solidus#5766 which seems to be caused by the deprecator behaviour being lost when the dummy app is initialized. By moving the code that sets the behaviour to happen after the initialization of the dummy app we can avoid this issue. Co-authored-by: Benjamin Willems --- core/lib/spree/testing_support/dummy_app.rb | 11 ++++++----- core/spec/lib/spree/deprecator_spec.rb | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 39a246fb416..f9e9ed64f8b 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -41,6 +41,12 @@ def self.setup(gem_root:, lib_name:, auto_migrate: true) DummyApp::Application.config.root = root DummyApp::Application.initialize! unless DummyApp::Application.initialized? + # Raise on deprecation warnings. + # NOTE: This needs to happen after the application is initialized. + if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present? + Spree.deprecator.behavior = :raise + end + if auto_migrate DummyApp::Migrations.auto_migrate end @@ -151,8 +157,3 @@ class Application < ::Rails::Application config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' end end - -# Raise on deprecation warnings -if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present? - Spree.deprecator.behavior = :raise -end diff --git a/core/spec/lib/spree/deprecator_spec.rb b/core/spec/lib/spree/deprecator_spec.rb index 381e90bcbc2..1007b074f4c 100644 --- a/core/spec/lib/spree/deprecator_spec.rb +++ b/core/spec/lib/spree/deprecator_spec.rb @@ -20,7 +20,6 @@ def deprecated_method it "does not raise an error unless overridden by environment" do if ENV["SOLIDUS_RAISE_DEPRECATIONS"] - pending "fix for deprecation behaviour override" expect { Dummy.new.deprecated_method }.to raise_error(ActiveSupport::DeprecationException) else expect { Dummy.new.deprecated_method }.not_to raise_error