From 313089fd49546669c42bc1d0cd613eadd34cc535 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Fri, 7 Dec 2018 18:00:34 +0100 Subject: [PATCH 01/23] Enable partial double verification for tests in Core RSpec 3 introduced partial double verification that checks arguments and methods existence also for partial doubles. This prevents having a double and its real object out of sync, which usually happens when refactoring existing code and the tests continue to pass, even if the refactored method or attribute has been renamed or removed. --- core/spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/core/spec/spec_helper.rb b/core/spec/spec_helper.rb index 36eee9bc627..054739b81fd 100644 --- a/core/spec/spec_helper.rb +++ b/core/spec/spec_helper.rb @@ -19,6 +19,7 @@ end config.mock_with :rspec do |c| c.syntax = :expect + c.verify_partial_doubles = true end config.before :each do From 644b7f27244a27862b41219c4753a80fd1fc31b3 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Fri, 7 Dec 2018 18:21:26 +0100 Subject: [PATCH 02/23] Remove regression tests for inexistent cases Its implementation has been refactored here: https://github.com/solidusio/solidus/pull/1033 --- core/spec/models/spree/payment_spec.rb | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index d482826c2b6..e2cb5b77cde 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -207,22 +207,6 @@ expect { payment.process! }.to raise_error(Spree::Core::GatewayError) expect(payment.state).to eq('invalid') end - - # Regression test for https://github.com/spree/spree/issues/4598 - it "should allow payments with a gateway_customer_profile_id" do - payment.source.update!(gateway_customer_profile_id: "customer_1", brand: 'visa') - expect(payment.payment_method.gateway_class).to receive(:supports?).with('visa').and_return(false) - expect(payment).to receive(:started_processing!) - payment.process! - end - - # Another regression test for https://github.com/spree/spree/issues/4598 - it "should allow payments with a gateway_payment_profile_id" do - payment.source.update!(gateway_payment_profile_id: "customer_1", brand: 'visa') - expect(payment.payment_method.gateway_class).to receive(:supports?).with('visa').and_return(false) - expect(payment).to receive(:started_processing!) - payment.process! - end end describe "#authorize!" do From 4dfafa89ba582f112f422865d6a19a4626076962 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 10:46:23 +0100 Subject: [PATCH 03/23] Remove unimplemented Spree::Order#finalize! tests First removed example: `Spree::StockLocation` doesn't implement `#decrease_stock_variant`. It was successful because the order object used in the example didn't contain any associated shipments, hence it never checked the expectation. Second removed example: `Spree::InventoryUnit` doesn't implement `.sell_units`. It was successful because it was checking that the method isn't called when finalizing the order. --- core/spec/models/spree/order/finalizing_spec.rb | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/core/spec/models/spree/order/finalizing_spec.rb b/core/spec/models/spree/order/finalizing_spec.rb index ad6548bded0..c5840559891 100644 --- a/core/spec/models/spree/order/finalizing_spec.rb +++ b/core/spec/models/spree/order/finalizing_spec.rb @@ -26,13 +26,6 @@ order.finalize! end - it "should decrease the stock for each variant in the shipment" do - order.shipments.each do |shipment| - expect(shipment.stock_location).to receive(:decrease_stock_for_variant) - end - order.finalize! - end - it "should change the shipment state to ready if order is paid" do Spree::Shipment.create(order: order) order.shipments.reload @@ -43,12 +36,6 @@ expect(order.shipment_state).to eq('ready') end - it "should not sell inventory units if track_inventory_levels is false" do - Spree::Config.set track_inventory_levels: false - expect(Spree::InventoryUnit).not_to receive(:sell_units) - order.finalize! - end - it "should send an order confirmation email" do mail_message = double "Mail::Message" expect(Spree::OrderMailer).to receive(:confirm_email).with(order).and_return mail_message From 7d531ab1a611635098f5187c683e1cf80bef1e13 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 11:11:25 +0100 Subject: [PATCH 04/23] Fix initial order object for Spree::Order#finalize! specs The order object used for the `Spree::Order#finalize!` specs didn't contain shipment objects. This changes the initial order factory to ensure it covers all the test cases. --- core/spec/models/spree/order/finalizing_spec.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/core/spec/models/spree/order/finalizing_spec.rb b/core/spec/models/spree/order/finalizing_spec.rb index c5840559891..d76a02cee62 100644 --- a/core/spec/models/spree/order/finalizing_spec.rb +++ b/core/spec/models/spree/order/finalizing_spec.rb @@ -3,11 +3,8 @@ require 'rails_helper' RSpec.describe Spree::Order, type: :model do - let(:order) { stub_model("Spree::Order") } - context "#finalize!" do - let!(:store) { create(:store) } - let(:order) { Spree::Order.create(email: 'test@example.com', store: store) } + let(:order) { create(:order_ready_to_complete) } before do order.update_column :state, 'complete' @@ -27,9 +24,6 @@ end it "should change the shipment state to ready if order is paid" do - Spree::Shipment.create(order: order) - order.shipments.reload - allow(order).to receive_messages(paid?: true, complete?: true) order.finalize! order.reload # reload so we're sure the changes are persisted From 187499e6e50b6c60398597a4b9506f0e20dd37eb Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 11:53:16 +0100 Subject: [PATCH 05/23] Fix a method name in a Return Item test example --- core/spec/models/spree/return_item_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/spec/models/spree/return_item_spec.rb b/core/spec/models/spree/return_item_spec.rb index 686f6936e5a..1f585ed234b 100644 --- a/core/spec/models/spree/return_item_spec.rb +++ b/core/spec/models/spree/return_item_spec.rb @@ -294,7 +294,7 @@ end it 'does not decrease inventory' do - expect(return_item).to_not receive(:process_inventory_unit) + expect(return_item).to_not receive(:process_inventory_unit!) subject end From 397cbb7456a9c97c11836e39e8676585bf78004a Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 13:02:23 +0100 Subject: [PATCH 06/23] Fix update shipments mock in a OrderUpdater test Actually, `Spree::OrderUpdater#update_shipments` doesn't accept any arguments. --- core/spec/models/spree/order_updater_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/spec/models/spree/order_updater_spec.rb b/core/spec/models/spree/order_updater_spec.rb index 8783ee06b60..be220640123 100644 --- a/core/spec/models/spree/order_updater_spec.rb +++ b/core/spec/models/spree/order_updater_spec.rb @@ -521,7 +521,7 @@ def create_adjustment(label, amount) allow(shipments).to receive_messages shipped: [] allow(updater).to receive(:update_totals) # Otherwise this gets called and causes a scene - expect(updater).not_to receive(:update_shipments).with(order) + expect(updater).not_to receive(:update_shipments) updater.update end end From 7d4ed886b024713b5a210dda5bf0a73fcc9e3ce6 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 13:11:41 +0100 Subject: [PATCH 07/23] Fix a mock in a OrderUpdater test example --- core/spec/models/spree/order_updater_spec.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/core/spec/models/spree/order_updater_spec.rb b/core/spec/models/spree/order_updater_spec.rb index be220640123..41734cd5bb5 100644 --- a/core/spec/models/spree/order_updater_spec.rb +++ b/core/spec/models/spree/order_updater_spec.rb @@ -513,13 +513,8 @@ def create_adjustment(label, amount) it "doesnt update each shipment" do shipment = stub_model(Spree::Shipment) - shipments = [shipment] - allow(order).to receive_messages shipments: shipments - allow(shipments).to receive_messages states: [] - allow(shipments).to receive_messages ready: [] - allow(shipments).to receive_messages pending: [] - allow(shipments).to receive_messages shipped: [] - + order.shipments = [shipment] + allow(order.shipments).to receive_messages(states: [], ready: [], pending: [], shipped: []) allow(updater).to receive(:update_totals) # Otherwise this gets called and causes a scene expect(updater).not_to receive(:update_shipments) updater.update From bf3aa76577c50f1f121bc4f172869dfadd17c51b Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 13:32:47 +0100 Subject: [PATCH 08/23] Fix method name typo in Spree::Payment specs --- core/spec/models/spree/payment_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index e2cb5b77cde..8fe2bdfc8a9 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -9,7 +9,7 @@ let(:gateway) do gateway = Spree::PaymentMethod::BogusCreditCard.new(active: true, name: 'Bogus gateway') - allow(gateway).to receive_messages source_required: true + allow(gateway).to receive_messages(source_required?: true) gateway end From 2d0984a38975b559d3fd197cbaf755af7c39cb1c Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Thu, 13 Dec 2018 17:19:26 +0100 Subject: [PATCH 09/23] Disable partial double check for OrderMerger specs Allows mocking user provided custom line item comparison hooks. --- core/spec/models/spree/order_merger_spec.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/spec/models/spree/order_merger_spec.rb b/core/spec/models/spree/order_merger_spec.rb index 448ab4c6cb8..add5cfcc835 100644 --- a/core/spec/models/spree/order_merger_spec.rb +++ b/core/spec/models/spree/order_merger_spec.rb @@ -68,7 +68,6 @@ module Spree context "merging using extension-specific line_item_comparison_hooks" do before do Spree::Order.register_line_item_comparison_hook(:foos_match) - allow(Spree::Variant).to receive(:price_modifier_amount).and_return(0.00) end after do @@ -83,7 +82,9 @@ module Spree end specify do - expect(order_1).to receive(:foos_match).with(@line_item_1, kind_of(Hash)).and_return(true) + without_partial_double_verification do + expect(order_1).to receive(:foos_match).with(@line_item_1, kind_of(Hash)).and_return(true) + end subject.merge!(order_2) expect(order_1.line_items.count).to eq(1) @@ -95,7 +96,9 @@ module Spree context "2 different line items" do before do - allow(order_1).to receive(:foos_match).and_return(false) + without_partial_double_verification do + allow(order_1).to receive(:foos_match).and_return(false) + end order_1.contents.add(variant, 1, foos: {}) order_2.contents.add(variant, 1, foos: { bar: :zoo }) From ce76c3cbe8fed158ded8b821227542ae2cb1c2e2 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 14:52:21 +0100 Subject: [PATCH 10/23] Add RSpec tag to disble partial double verification Adds `partial_double_verification` RSpec meta-tag, that, when set to false, disables the partial double verification for the example. Useful when testing unimplemented mocks like user-defined hooks. --- .../partial_double_verification_example_group.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 core/spec/support/example_groups/partial_double_verification_example_group.rb diff --git a/core/spec/support/example_groups/partial_double_verification_example_group.rb b/core/spec/support/example_groups/partial_double_verification_example_group.rb new file mode 100644 index 00000000000..a22cc716b58 --- /dev/null +++ b/core/spec/support/example_groups/partial_double_verification_example_group.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module PartialDoubleVerificationExampleGroup + extend ActiveSupport::Concern + + included do + around do |example| + without_partial_double_verification { example.run } + end + end +end + +RSpec.configure do |config| + config.include PartialDoubleVerificationExampleGroup, + partial_double_verification: false +end From b64fb1dd51168ea73ab59cd5a70a0f3eecdc3202 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 15:00:38 +0100 Subject: [PATCH 11/23] Upgrade rspec-activemodel-mocks to ~>1.1 Upgrades `RSpec::ActiveModel::Mocks` to version `~>1.1` which includes the support for `inversed_from` when assigning an association for a mock. See: rspec/rspec-activemodel-mocks#32 --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 7aaf0557262..6c25b50248b 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,7 @@ end gem 'database_cleaner', '~> 1.3', require: false gem 'factory_bot_rails', '~> 4.8', require: false gem 'i18n-tasks', '~> 0.9', require: false -gem 'rspec-activemodel-mocks', '~>1.0.2', require: false +gem 'rspec-activemodel-mocks', '~>1.1', require: false gem 'rspec-rails', '~> 3.7', require: false gem 'simplecov', require: false gem 'with_model', require: false From 6a3896f8bed738a8430b91c275469fde4b0a7f34 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 15:09:15 +0100 Subject: [PATCH 12/23] Remove unimplemented stub from a InventoryUnit test --- core/spec/models/spree/inventory_unit_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/spec/models/spree/inventory_unit_spec.rb b/core/spec/models/spree/inventory_unit_spec.rb index 23b3e7410eb..186c08d0fb1 100644 --- a/core/spec/models/spree/inventory_unit_spec.rb +++ b/core/spec/models/spree/inventory_unit_spec.rb @@ -97,8 +97,6 @@ shipment.stock_location = stock_location shipment.shipping_methods << create(:shipping_method) shipment.order = other_order - # We don't care about this in this test - allow(shipment).to receive(:ensure_correct_adjustment) shipment.tap(&:save!) end @@ -149,8 +147,6 @@ end describe "#current_or_new_return_item" do - before { allow(inventory_unit).to receive_messages(total_excluding_vat: 100.0) } - subject { inventory_unit.current_or_new_return_item } context "associated with a return item" do From cc19bb400bb4556bf140da5610492ea4d24fdae8 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 15:26:04 +0100 Subject: [PATCH 13/23] Disable double verification for some test examples Disables partial double verification when testing mocked external user-defined methods like hooks, for example. --- core/spec/models/spree/order/updating_spec.rb | 2 +- core/spec/models/spree/order_spec.rb | 4 ++-- core/spec/models/spree/product_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/spec/models/spree/order/updating_spec.rb b/core/spec/models/spree/order/updating_spec.rb index 1a7d0e7efa2..0fc840216f8 100644 --- a/core/spec/models/spree/order/updating_spec.rb +++ b/core/spec/models/spree/order/updating_spec.rb @@ -6,7 +6,7 @@ let(:order) { create(:order) } context "#update!" do - context "when there are update hooks" do + context "when there are update hooks", partial_double_verification: false do before { Spree::Order.register_update_hook :foo } after { Spree::Order.update_hooks.clear } it "should call each of the update hooks" do diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 251983882cc..a1ed3c99f7e 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -324,7 +324,7 @@ def merge!(other_order, user = nil) end end - context "add_update_hook" do + context "add_update_hook", partial_double_verification: false do before do Spree::Order.class_eval do register_update_hook :add_awesome_sauce @@ -712,7 +712,7 @@ def merge!(other_order, user = nil) expect(order.find_line_item_by_variant(mock_model(Spree::Variant))).to be_nil end - context "match line item with options" do + context "match line item with options", partial_double_verification: false do before do Spree::Order.register_line_item_comparison_hook(:foos_match) end diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb index f26296ae8a8..35d10db3176 100644 --- a/core/spec/models/spree/product_spec.rb +++ b/core/spec/models/spree/product_spec.rb @@ -27,7 +27,7 @@ class Extension < Spree::Base expect(clone.images.size).to eq(product.images.size) end - it 'calls #duplicate_extra' do + it 'calls #duplicate_extra', partial_double_verification: false do expect_any_instance_of(Spree::Product).to receive(:duplicate_extra) do |product, old_product| product.name = old_product.name.reverse end From 4971602a32aa44ea3aabffd5536c54b374e54c3a Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 15:30:30 +0100 Subject: [PATCH 14/23] Fix stock estimator mocked sorter class initializer By enabling `verify_partial_doubles` for RSpec, this test example was failing because the `TestSorter#new` implementation was not accepting any arguments. Fixed, by mocking a custom `#initialize` method. --- core/spec/models/spree/stock/estimator_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/spec/models/spree/stock/estimator_spec.rb b/core/spec/models/spree/stock/estimator_spec.rb index 9f88c2bd1d2..399a9333a23 100644 --- a/core/spec/models/spree/stock/estimator_spec.rb +++ b/core/spec/models/spree/stock/estimator_spec.rb @@ -205,11 +205,14 @@ def find_default end it 'uses the configured shipping rate sorter' do - class Spree::Stock::TestSorter; end; + class Spree::Stock::TestSorter + def initialize(_rates) + end + end Spree::Config.shipping_rate_sorter_class = Spree::Stock::TestSorter sorter = double(:sorter, sort: nil) - allow(Spree::Stock::TestSorter).to receive(:new).and_return(sorter) + allow(Spree::Stock::TestSorter).to receive(:new) { sorter } subject.shipping_rates(package) From bf60389c6657ff935d93095983a2c444854941f9 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:32:22 +0100 Subject: [PATCH 15/23] Disable double verification in order checkout specs Since `verify_partial_doubles` has been set to true for the entire project, state machine defined dynamically methods aren't supported. This disables the partial doubles check, for test cases dependent on the state machine methods. --- core/spec/models/spree/order/checkout_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/spec/models/spree/order/checkout_spec.rb b/core/spec/models/spree/order/checkout_spec.rb index cc0856bc45b..8adbc03b618 100644 --- a/core/spec/models/spree/order/checkout_spec.rb +++ b/core/spec/models/spree/order/checkout_spec.rb @@ -36,7 +36,7 @@ def assert_state_changed(order, from, to) Spree::Order.checkout_flow(&@old_checkout_flow) end - it '.remove_transition' do + it '.remove_transition', partial_double_verification: false do options = { from: transitions.first.keys.first, to: transitions.first.values.first } allow(Spree::Order).to receive(:next_event_transition).and_return([options]) expect(Spree::Order.remove_transition(options)).to be_truthy @@ -238,7 +238,7 @@ def assert_state_changed(order, from, to) order.ship_address = ship_address end - context 'when order has default selected_shipping_rate_id' do + context 'when order has default selected_shipping_rate_id', partial_double_verification: false do let(:shipment) { create(:shipment, order: order) } let(:shipping_method) { create(:shipping_method) } let(:shipping_rate) { @@ -277,7 +277,7 @@ def assert_state_changed(order, from, to) end end - context "from delivery" do + context "from delivery", partial_double_verification: false do let(:ship_address) { FactoryBot.create(:ship_address) } before do @@ -537,7 +537,7 @@ def assert_state_changed(order, from, to) end end - context "default credit card" do + context "default credit card", partial_double_verification: false do before do order.user = FactoryBot.create(:user) order.store = FactoryBot.create(:store) @@ -568,7 +568,7 @@ def assert_state_changed(order, from, to) end end - context "a payment fails during processing" do + context "a payment fails during processing", partial_double_verification: false do before do order.user = FactoryBot.create(:user) order.email = 'spree@example.org' @@ -656,7 +656,7 @@ def assert_state_changed(order, from, to) assert_state_changed(order, 'cart', 'complete') end - it "does not attempt to process payments" do + it "does not attempt to process payments", partial_double_verification: false do order.email = 'user@example.com' allow(order).to receive(:ensure_promotions_eligible).and_return(true) allow(order).to receive(:ensure_line_item_variants_are_not_deleted).and_return(true) From fd0ff058443c3671420a52b2b65dc80913d5319a Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:37:51 +0100 Subject: [PATCH 16/23] Remove unimplemented mocked methods for some tests --- core/spec/models/spree/order_spec.rb | 6 +----- core/spec/models/spree/shipment_spec.rb | 3 +-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index a1ed3c99f7e..291e4bcd598 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -15,10 +15,6 @@ end let(:code) { promotion.codes.first } - before do - allow(Spree::LegacyUser).to receive_messages(current: mock_model(Spree::LegacyUser, id: 123)) - end - context '#store' do it { is_expected.to respond_to(:store) } @@ -1072,7 +1068,7 @@ def generate context 'an old-style refund exists' do let(:order) { create(:order_ready_to_ship) } - let(:payment) { order.payments.first.tap { |p| allow(p).to receive_messages(profiles_supported: false) } } + let(:payment) { order.payments.first.tap { |p| allow(p).to receive_messages(profiles_supported?: false) } } let!(:refund_payment) { build(:payment, amount: -1, order: order, state: 'completed', source: payment).tap do |p| allow(p).to receive_messages(profiles_supported?: false) diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 4a864f8d2ce..f460788e05f 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -512,7 +512,6 @@ let(:subject) { shipment_with_inventory_units.ship! } before do allow(order).to receive(:update!) - allow(shipment_with_inventory_units).to receive_messages(require_inventory: false, update_order: true) end it 'unstocks them items' do @@ -525,7 +524,7 @@ context "from #{state}" do before do allow(order).to receive(:update!) - allow(shipment).to receive_messages(require_inventory: false, update_order: true, state: state) + allow(shipment).to receive_messages(state: state) end it "finalizes adjustments" do From 9ae0e0a65ed69e57ba6d0346c2ad2e0e9fee3fb5 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:43:03 +0100 Subject: [PATCH 17/23] Reuse already defined object in a Promotion test Reuse an already initialized Promotion object, instead of creating a new one. --- core/spec/models/spree/promotion_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/spec/models/spree/promotion_spec.rb b/core/spec/models/spree/promotion_spec.rb index 24cc7248318..579136dc87f 100644 --- a/core/spec/models/spree/promotion_spec.rb +++ b/core/spec/models/spree/promotion_spec.rb @@ -632,9 +632,12 @@ end context "with 'any' match policy" do - let(:promotion) { Spree::Promotion.create(name: "Promo", match_policy: 'any') } let(:promotable) { double('Promotable') } + before do + promotion.match_policy = 'any' + end + it "should have eligible rules if any of the rules are eligible" do allow_any_instance_of(Spree::PromotionRule).to receive_messages(applicable?: true) true_rule = Spree::PromotionRule.create(promotion: promotion) From bd0d91e59da8731ddcbdda9fb901d7159d757bd8 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:46:28 +0100 Subject: [PATCH 18/23] Refactor some Spree::Promotion specs By enabling the `verify_partial_doubles` in RSpec, it's not allowed to stub unimplemented methods. In these cases `Spree::Promotion#rules` was stubbed to return a plain `Array` object, instead it should return an `ActiveRecord::Associations::CollectionProxy` object. --- core/spec/models/spree/promotion_spec.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/core/spec/models/spree/promotion_spec.rb b/core/spec/models/spree/promotion_spec.rb index 579136dc87f..24b225e89f4 100644 --- a/core/spec/models/spree/promotion_spec.rb +++ b/core/spec/models/spree/promotion_spec.rb @@ -639,11 +639,9 @@ end it "should have eligible rules if any of the rules are eligible" do - allow_any_instance_of(Spree::PromotionRule).to receive_messages(applicable?: true) - true_rule = Spree::PromotionRule.create(promotion: promotion) - allow(true_rule).to receive_messages(eligible?: true) - allow(promotion).to receive_messages(rules: [true_rule]) - allow(promotion).to receive_message_chain(:rules, :for).and_return([true_rule]) + true_rule = mock_model(Spree::PromotionRule, eligible?: true, applicable?: true) + promotion.promotion_rules = [true_rule] + allow(promotion.rules).to receive(:for) { promotion.rules } expect(promotion.eligible_rules(promotable)).to eq [true_rule] end @@ -671,13 +669,13 @@ describe '#line_item_actionable?' do let(:order) { double Spree::Order } let(:line_item) { double Spree::LineItem } - let(:true_rule) { double Spree::PromotionRule, eligible?: true, applicable?: true, actionable?: true } - let(:false_rule) { double Spree::PromotionRule, eligible?: true, applicable?: true, actionable?: false } + let(:true_rule) { mock_model Spree::PromotionRule, eligible?: true, applicable?: true, actionable?: true } + let(:false_rule) { mock_model Spree::PromotionRule, eligible?: true, applicable?: true, actionable?: false } let(:rules) { [] } before do - allow(promotion).to receive(:rules) { rules } - allow(rules).to receive(:for) { rules } + promotion.promotion_rules = rules + allow(promotion.rules).to receive(:for) { rules } end subject { promotion.line_item_actionable? order, line_item } From 0b8553a208d1f72609b0d5b536976fd1e9fe1bd2 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:53:22 +0100 Subject: [PATCH 19/23] Refactor test for Shipment assoc. to InventoryUnit It was stubbed to return a plain `Array`, instead it must return a `ActiveRecord::Associations::CollectionProxy` object. --- core/spec/models/spree/shipment_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index f460788e05f..bcda20c6e05 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -267,8 +267,8 @@ before do allow(line_item).to receive(:order) { order } - allow(shipment).to receive(:inventory_units) { inventory_units } - allow(inventory_units).to receive_message_chain(:includes, :joins).and_return inventory_units + shipment.inventory_units = inventory_units + allow(shipment.inventory_units).to receive_message_chain(:includes, :joins).and_return inventory_units end it 'should use symbols for states when adding contents to package' do From cff9df5e03ec8a8356940b8c2f40e2c4fd23b33c Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 16:58:29 +0100 Subject: [PATCH 20/23] Disable double verification for a Shipment test By enabling `verify_partial_doubles` in RSpec, dynamic defined methods by State Machine are not supported. This disables the partial double verification for these cases. --- core/spec/models/spree/shipment_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index bcda20c6e05..b572575b347 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -413,7 +413,9 @@ allow(shipment.order).to receive(:update!) shipment.state = 'pending' - expect(shipment).to receive(:after_cancel) + without_partial_double_verification do + expect(shipment).to receive(:after_cancel) + end shipment.cancel! expect(shipment.state).to eq 'canceled' end From f7d6814c7ca4efe3b1c3ae428eda393079b145da Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 17:04:01 +0100 Subject: [PATCH 21/23] Fix a Payment::Cancellation test case Disables partial double verification, since it stubs unimplemented methods. Fixes the expectation for `#cancel` call check example. --- core/spec/models/spree/payment/cancellation_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/spec/models/spree/payment/cancellation_spec.rb b/core/spec/models/spree/payment/cancellation_spec.rb index 49a419708aa..05be547eb84 100644 --- a/core/spec/models/spree/payment/cancellation_spec.rb +++ b/core/spec/models/spree/payment/cancellation_spec.rb @@ -63,14 +63,15 @@ end end - context 'if the payment_method does not respond to `try_void`' do + context 'if the payment_method does not respond to `try_void`', partial_double_verification: false do before do allow(payment_method).to receive(:respond_to?) { false } - expect(payment_method).to receive(:cancel) { double } - expect(payment).to receive(:handle_void_response) + allow(payment_method).to receive(:cancel) { double } + allow(payment).to receive(:handle_void_response) end it 'calls cancel instead' do + expect(payment_method).to receive(:cancel) Spree::Deprecation.silence { subject } end From c4e5055256cab87d1cab680c3c6d9bef429ff498 Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 17:22:16 +0100 Subject: [PATCH 22/23] Disable double verification in some helpers specs By enabling `verify_partial_doubles`, when stubbing controller helpers, the partial double check must be disabled, see: rspec/rspec-rails#1076 --- core/spec/helpers/products_helper_spec.rb | 4 +++- core/spec/helpers/taxons_helper_spec.rb | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/spec/helpers/products_helper_spec.rb b/core/spec/helpers/products_helper_spec.rb index da38bf267d5..16996f0cf03 100644 --- a/core/spec/helpers/products_helper_spec.rb +++ b/core/spec/helpers/products_helper_spec.rb @@ -15,7 +15,9 @@ module Spree end before do - allow(helper).to receive(:current_pricing_options) { pricing_options } + without_partial_double_verification do + allow(helper).to receive(:current_pricing_options) { pricing_options } + end end context "#variant_price_diff" do diff --git a/core/spec/helpers/taxons_helper_spec.rb b/core/spec/helpers/taxons_helper_spec.rb index b058998b93d..b24de8d1a67 100644 --- a/core/spec/helpers/taxons_helper_spec.rb +++ b/core/spec/helpers/taxons_helper_spec.rb @@ -8,7 +8,9 @@ Spree::Config.pricing_options_class.new(currency: currency) end before do - allow(helper).to receive(:current_pricing_options) { pricing_options } + without_partial_double_verification do + allow(helper).to receive(:current_pricing_options) { pricing_options } + end end describe "#taxon_preview" do From 11e3ca48cc370d8a62d972dbdeee010e42b5bfed Mon Sep 17 00:00:00 2001 From: Dumitru Ceban Date: Mon, 17 Dec 2018 17:26:52 +0100 Subject: [PATCH 23/23] Allow stub of unimplmented helper method --- core/spec/lib/spree/core/controller_helpers/auth_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/spec/lib/spree/core/controller_helpers/auth_spec.rb b/core/spec/lib/spree/core/controller_helpers/auth_spec.rb index f7d1ef82932..577030adc52 100644 --- a/core/spec/lib/spree/core/controller_helpers/auth_spec.rb +++ b/core/spec/lib/spree/core/controller_helpers/auth_spec.rb @@ -55,11 +55,15 @@ def index describe '#try_spree_current_user' do it 'calls spree_current_user when define spree_current_user method' do - expect(controller).to receive(:spree_current_user) + without_partial_double_verification do + expect(controller).to receive(:spree_current_user) + end controller.try_spree_current_user end it 'calls current_spree_user when define current_spree_user method' do - expect(controller).to receive(:current_spree_user) + without_partial_double_verification do + expect(controller).to receive(:current_spree_user) + end controller.try_spree_current_user end it 'returns nil' do