Task and task spec fixes

core/lib/tasks/migrations/assure_store_on_orders.rake → 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)

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

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

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

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 }
 

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

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

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 }
 

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