diff --git a/core/lib/tasks/migrations/assure_store_on_orders.rake b/core/lib/tasks/migrations/ensure_store_on_orders.rake similarity index 81% rename from core/lib/tasks/migrations/assure_store_on_orders.rake rename to core/lib/tasks/migrations/ensure_store_on_orders.rake index f3567915cce..8ec7cd95158 100644 --- a/core/lib/tasks/migrations/assure_store_on_orders.rake +++ b/core/lib/tasks/migrations/ensure_store_on_orders.rake @@ -1,6 +1,6 @@ namespace :solidus do namespace :migrations do - namespace :assure_store_on_orders do + namespace :ensure_store_on_orders do desc "Makes sure every order in the system has a store attached" task up: :environment do @@ -16,18 +16,18 @@ namespace :solidus do spree_store_count = Spree::Store.count if spree_store_count == 0 - abort "You do not have a store set up. Please create a store instance for your installation." + raise "You do not have a store set up. Please create a store instance for your installation." elsif spree_store_count > 1 - abort(<<-TEXT.squish) + raise(<<-TEXT.squish) You have more than one store set up. We can not be sure which store to attach your orders to. Please attach store ids to all your orders, and run this task again when you're finished. TEXT end - default_store = Store.where(default: true).first + default_store = Spree::Store.where(default: true).first unless default_store - abort "Your store is not marked as default. Please mark your one store as the default store and run this task again." + raise "Your store is not marked as default. Please mark your one store as the default store and run this task again." end Spree::Order.where(store_id: nil).update_all(store_id: Spree::Store.default.id) diff --git a/core/lib/tasks/upgrade.rake b/core/lib/tasks/upgrade.rake index c3f68a43502..dbffab7624b 100644 --- a/core/lib/tasks/upgrade.rake +++ b/core/lib/tasks/upgrade.rake @@ -2,7 +2,7 @@ namespace :solidus do namespace :upgrade do desc "Upgrade Solidus to version 1.3" task one_point_three: [ - 'solidus:migrations:assure_store_on_orders:up', + 'solidus:migrations:ensure_store_on_orders:up', 'solidus:migrations:migrate_shipping_rate_taxes:up', 'solidus:migrations:create_vat_prices' ] do diff --git a/core/spec/lib/tasks/dummy_task.rake b/core/spec/lib/tasks/dummy_task.rake new file mode 100644 index 00000000000..563655239d6 --- /dev/null +++ b/core/spec/lib/tasks/dummy_task.rake @@ -0,0 +1,10 @@ +# This is a dummy task used for generic rake task testing + +task dummy_task: :environment do + DummyTaskRunner.run +end + +class DummyTaskRunner + def self.run + end +end diff --git a/core/spec/lib/tasks/dummy_task_spec.rb b/core/spec/lib/tasks/dummy_task_spec.rb new file mode 100644 index 00000000000..1a71b268b9e --- /dev/null +++ b/core/spec/lib/tasks/dummy_task_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe 'dummy_task' do + include_context( + 'rake', + task_name: 'dummy_task', + task_path: Spree::Core::Engine.root.join('spec/lib/tasks/dummy_task.rake'), + ) + + it 'calls the dummy task exactly once' do + expect(DummyTaskRunner).to receive(:run).once + task.invoke + end + + # This tests: + # 1) that tasks get reenabled between examples + # 2) that tasks aren't loaded in the wrong way, causing them to execute + # an extra time for every example that's defined. + # We need at least two specs to trigger the error conditions and spec order is + # random so we just create the same spec twice. We could probably combine this + # with the generic spec above but this seems clearer. + 2.times do |i| + it "still calls the dummy task exactly once when more than one example is defined - #{i}" do + expect(DummyTaskRunner).to receive(:run).once + task.invoke + end + end +end diff --git a/core/spec/lib/tasks/exchanges_spec.rb b/core/spec/lib/tasks/exchanges_spec.rb index b8d471f6933..01d02398f9a 100644 --- a/core/spec/lib/tasks/exchanges_spec.rb +++ b/core/spec/lib/tasks/exchanges_spec.rb @@ -1,14 +1,11 @@ require 'spec_helper' describe "exchanges:charge_unreturned_items" do - let(:task) do - Rake::Task['exchanges:charge_unreturned_items'] - end - - before do - Rails.application.load_tasks - task.reenable - end + include_context( + 'rake', + task_name: 'exchanges:charge_unreturned_items', + task_path: Spree::Core::Engine.root.join('lib/tasks/exchanges.rake'), + ) subject { task } diff --git a/core/spec/lib/tasks/migrations/copy_shipped_shipments_to_cartons_spec.rb b/core/spec/lib/tasks/migrations/copy_shipped_shipments_to_cartons_spec.rb index 21dd99e902b..c69a131877d 100644 --- a/core/spec/lib/tasks/migrations/copy_shipped_shipments_to_cartons_spec.rb +++ b/core/spec/lib/tasks/migrations/copy_shipped_shipments_to_cartons_spec.rb @@ -1,16 +1,13 @@ require 'spec_helper' describe 'spree:migrations:copy_shipped_shipments_to_cartons' do - before do - Rails.application.load_tasks - task.reenable - end + include_context( + 'rake', + task_name: 'spree:migrations:copy_shipped_shipments_to_cartons:up', + task_path: Spree::Core::Engine.root.join('lib/tasks/migrations/copy_shipped_shipments_to_cartons.rake'), + ) describe 'up' do - let(:task) do - Rake::Task['spree:migrations:copy_shipped_shipments_to_cartons:up'] - end - # should generate a carton let!(:shipped_shipment) { shipped_order.shipments.first } # should not generate a carton because it's not shipped diff --git a/core/spec/lib/tasks/migrations/ensure_store_on_orders_spec.rb b/core/spec/lib/tasks/migrations/ensure_store_on_orders_spec.rb new file mode 100644 index 00000000000..b93f2412119 --- /dev/null +++ b/core/spec/lib/tasks/migrations/ensure_store_on_orders_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe 'solidus:migrations:ensure_store_on_orders' do + describe 'up' do + include_context( + 'rake', + task_path: Spree::Core::Engine.root.join('lib/tasks/migrations/ensure_store_on_orders.rake'), + task_name: 'solidus:migrations:ensure_store_on_orders:up', + ) + + context 'with no orders' do + it 'succeeds' do + expect { task.invoke }.to output( + "Everything is good, all orders in your database have a store attached.\n" + ).to_stdout + end + end + + context 'when all orders have store_ids' do + let!(:order) { create(:order, store: create(:store)) } + + it 'succeeds' do + expect { task.invoke }.to output( + "Everything is good, all orders in your database have a store attached.\n" + ).to_stdout + end + end + + context 'when some orders do not have store_ids' do + let!(:order_with_store) { create(:order, store: store) } + let!(:order_without_store) do + # due to a before_validation that adds a store when one is missing, + # we can't simply specify `store: nil` + create(:order, store: store).tap do |o| + o.update_columns(store_id: nil) + end + end + let!(:store) { create(:store) } + + it 'succeeds' do + expect { task.invoke }.to output( + "All orders updated with the default store.\n" + ).to_stdout + end + + context 'when there are no stores' do + before do + order_with_store.update_columns(store_id: nil) + # due to a before_validation that adds a store when one is missing, + # we can't simply specify `store: nil` + store.destroy + end + + it 'raises' do + expect { task.invoke }.to raise_error(/You do not have a store set up/) + end + end + + context 'when there are multiple stores' do + let!(:extra_store) { create(:store) } + + it 'raises' do + expect { task.invoke }.to raise_error(/You have more than one store set up/) + end + end + + context 'when there is no default store' do + before do + # The before_save 'ensure_default_exists_and_is_unique' means that we + # need to use update_columns to set up this scenario. + store.update_columns(default: false) + end + + it 'raises' do + expect { task.invoke }.to raise_error(/Your store is not marked as default/) + end + end + end + end +end diff --git a/core/spec/lib/tasks/order_capturing_spec.rb b/core/spec/lib/tasks/order_capturing_spec.rb index bdd0eb7e15f..5642d6cc3f0 100644 --- a/core/spec/lib/tasks/order_capturing_spec.rb +++ b/core/spec/lib/tasks/order_capturing_spec.rb @@ -1,14 +1,11 @@ require 'spec_helper' describe "order_capturing:capture_payments" do - let(:task) do - Rake::Task['order_capturing:capture_payments'] - end - - before do - Rails.application.load_tasks - task.reenable - end + include_context( + 'rake', + task_name: 'order_capturing:capture_payments', + task_path: Spree::Core::Engine.root.join('lib/tasks/order_capturing.rake'), + ) subject { task } diff --git a/core/spec/support/shared_contexts/rake.rb b/core/spec/support/shared_contexts/rake.rb new file mode 100644 index 00000000000..feac9878007 --- /dev/null +++ b/core/spec/support/shared_contexts/rake.rb @@ -0,0 +1,30 @@ +# +# Rake task spec setup. +# +shared_context "rake" do |task_path:, task_name:| + require 'rake' + + let(:task) do + Rake::Task[task_name] + end + + before(:each) do + # we need to reenable the task or else `task.invoke` will only run the task + # for the first example that runs. + task.reenable + end + + before(:all) do + Rake::Task.clear + # Note: Using `Rails.application.load_tasks` doesn't seem to work correctly + # in the specs. The tasks each run twice when invoked instead of once. + load task_path + # Many tasks require the 'environment' task, which isn't needed in specs + # since the environment is already loaded. So generate a fake one. + Rake::Task.define_task(:environment) + end + + after(:all) do + Rake::Task.clear + end +end