Merge pull request #13447 from mkllnk/cover-rake

Report code coverage on rake tasks

.simplecov

@@ -7,8 +7,5 @@ SimpleCov.start 'rails' do
   add_filter '/script'
   add_filter '/db'
 
-  # We haven't managed to make simplecov recognise rake coverage accurately.
-  add_filter '/lib/tasks/'
-
   formatter SimpleCov::Formatter::SimpleFormatter
 end

lib/tasks/simplecov.rake

@@ -4,6 +4,9 @@ namespace :simplecov do
   desc "Collates all result sets produced during parallel test runs"
   task :collate_results, # rubocop:disable Rails/RakeEnvironment doesn't need the full env
        [:path_to_results, :coverage_dir] do |_t, args|
+    # This code is covered by a spec but trying to measure the code coverage of
+    # the spec breaks the coverage report. We need to ignore it to avoid warnings.
+    # :nocov:
     require "simplecov"
     require "undercover/simplecov_formatter"
 
@@ -19,5 +22,6 @@ namespace :simplecov do
       formatter(SimpleCov::Formatter::Undercover)
       coverage_dir(output_path)
     end
+    # :nocov:
   end
 end

spec/lib/tasks/data/truncate_data_rake_spec.rb

@@ -1,15 +1,13 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe 'truncate_data.rake' do
+  include_context "rake"
+
   describe ':truncate' do
     context 'when months_to_keep is specified' do
       it 'truncates order cycles closed earlier than months_to_keep months ago' do
-        Rake.application.rake_require 'tasks/data/truncate_data'
-        Rake::Task.define_task(:environment)
-
         highline = instance_double(HighLine, agree: true)
         allow(HighLine).to receive(:new).and_return(highline)
 
@@ -27,7 +25,7 @@ RSpec.describe 'truncate_data.rake' do
         create(:order, order_cycle: recent_order_cycle)
 
         months_to_keep = 6
-        Rake.application.invoke_task "ofn:data:truncate[#{months_to_keep}]"
+        invoke_task "ofn:data:truncate[#{months_to_keep}]"
 
         expect(OrderCycle.all).to contain_exactly(recent_order_cycle)
       end

spec/lib/tasks/enterprises_rake_spec.rb

@@ -1,13 +1,9 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe 'enterprises.rake' do
-  before(:all) do
-    Rake.application.rake_require("tasks/enterprises")
-    Rake::Task.define_task(:environment)
-  end
+  include_context "rake"
 
   describe ':remove_enterprise' do
     context 'when the enterprises exists' do
@@ -15,7 +11,7 @@ RSpec.describe 'enterprises.rake' do
         enterprise = create(:enterprise)
 
         expect {
-          Rake.application.invoke_task "ofn:remove_enterprise[#{enterprise.id}]"
+          invoke_task "ofn:remove_enterprise[#{enterprise.id}]"
         }.to change { Enterprise.count }.by(-1)
       end
     end
@@ -32,7 +28,7 @@ RSpec.describe 'enterprises.rake' do
         enterprise_diff.connected_apps.create
 
         expect {
-          Rake.application.invoke_task(
+          invoke_task(
             "ofn:enterprises:activate_connected_app_type[affiliate_sales_data]"
           )
         }.to change { ConnectedApps::AffiliateSalesData.count }.by(1)

spec/lib/tasks/import_product_images_rake_spec.rb

@@ -1,23 +1,15 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe 'ofn:import:product_images' do
-  before(:all) do
-    Rake.application.rake_require("tasks/import_product_images")
-    Rake::Task.define_task(:environment)
-  end
-
-  before do
-    Rake::Task['ofn:import:product_images'].reenable
-  end
+  include_context "rake"
 
   describe 'task' do
     context "filename is blank" do
       it 'raises an error' do
         expect {
-          Rake.application.invoke_task('ofn:import:product_images')
+          invoke_task('ofn:import:product_images')
         }.to raise_error(RuntimeError,
                          'Filename required')
       end
@@ -28,7 +20,7 @@ RSpec.describe 'ofn:import:product_images' do
         allow(CSV).to receive(:read).and_return(CSV::Table.new([]))
 
         expect {
-          Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
+          invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
         }.to raise_error(RuntimeError, 'CSV columns reqired: ["producer", "name", "image_url"]')
       end
     end
@@ -74,7 +66,7 @@ RSpec.describe 'ofn:import:product_images' do
         OUTPUT
 
         expect {
-          Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
+          invoke_task('ofn:import:product_images["path/to/csv/file.csv"]')
         }.to output(expected_output).to_stdout
       end
     end

spec/lib/tasks/reset_spec.rb

@@ -1,13 +1,9 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe "reset.rake" do
-  before(:all) do
-    Rake.application.rake_require("tasks/reset")
-    Rake::Task.define_task(:environment)
-  end
+  include_context "rake"
 
   it "clears job queues" do
     job_class = Class.new do
@@ -18,7 +14,7 @@ RSpec.describe "reset.rake" do
     queue = Sidekiq::Queue.all.first # rubocop:disable Rails/RedundantActiveRecordAllMethod
 
     expect {
-      Rake.application.invoke_task "ofn:reset_sidekiq"
+      invoke_task "ofn:reset_sidekiq"
     }.to change {
       queue.count
     }.to(0)

spec/lib/tasks/sample_data_rake_spec.rb

@@ -1,13 +1,9 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe 'sample_data.rake' do
-  before(:all) do
-    Rake.application.rake_require 'tasks/sample_data'
-    Rake::Task.define_task(:environment)
-  end
+  include_context "rake"
 
   before do
     # Create seed data required by the sample data.
@@ -16,7 +12,7 @@ RSpec.describe 'sample_data.rake' do
   end
 
   it "creates some sample data to play with" do
-    Rake.application.invoke_task "ofn:sample_data"
+    invoke_task "ofn:sample_data"
 
     expect(EnterpriseGroup.count).to eq 1
     expect(Customer.count).to eq 2

spec/lib/tasks/simplecov_spec.rb

@@ -1,12 +1,9 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'rake'
 
 RSpec.describe "simplecov.rake" do
-  before(:all) do
-    Rake.application.rake_require("tasks/simplecov")
-  end
+  include_context "rake"
 
   describe "simplecov:collate_results" do
     context "when there are reports to merge" do
@@ -16,10 +13,16 @@ RSpec.describe "simplecov.rake" do
         Dir.mktmpdir do |tmp_dir|
           output_dir = File.join(tmp_dir, "output")
 
+          task_name = "simplecov:collate_results[#{input_dir},#{output_dir}]"
+
           expect {
-            Rake.application.invoke_task(
-              "simplecov:collate_results[#{input_dir},#{output_dir}]"
-            )
+            if ENV["COVERAGE"]
+              # Start task in a new process to not mess with our coverage report.
+              `bundle exec rake #{task_name}`
+            else
+              # Use the quick standard invocation in dev.
+              invoke_task(task_name)
+            end
           }.to change { Dir.exist?(output_dir) }.
             from(false).
             to(true).

spec/lib/tasks/users_rake_spec.rb

@@ -4,18 +4,14 @@ require 'spec_helper'
 require 'rake'
 
 RSpec.describe 'users.rake' do
-  before do
-    Rake.application.rake_require 'tasks/users'
-    Rake::Task.define_task(:environment)
-    Rake::Task['ofn:remove_enterprise_limit'].reenable
-  end
+  include_context "rake"
 
   describe ':remove_enterprise_limit' do
     context 'when the user exists' do
       let(:user) { create(:user) }
 
       it 'sets the enterprise_limit to the maximum integer' do
-        Rake.application.invoke_task "ofn:remove_enterprise_limit[#{user.id}]"
+        invoke_task "ofn:remove_enterprise_limit[#{user.id}]"
 
         expect(user.reload.enterprise_limit).to eq(2_147_483_647)
       end
@@ -24,7 +20,7 @@ RSpec.describe 'users.rake' do
     context 'when the user does not exist' do
       it 'raises' do
         expect {
-          Rake.application.invoke_task "ofn:remove_enterprise_limit[123]"
+          invoke_task "ofn:remove_enterprise_limit[123]"
         }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end

spec/support/shared_contexts/rake.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+# Let this context take care of Rake testing gotchas.
+#
+# ```rb
+# RSpec.describe "my_task.rake" do
+#   include_context "rake"
+#   # ..
+# ```
+#
+shared_context "rake" do
+  before(:all) do
+    # Make sure that Rake tasks are only loaded once.
+    # Otherwise we lose code coverage data.
+    if Rake::Task.tasks.empty?
+      Openfoodnetwork::Application.load_tasks
+      Rake::Task.define_task(:environment)
+    end
+  end
+
+  # Use the same task string as you would on the command line.
+  #
+  # ```rb
+  # invoke_task "example:task[arg1,arg2]"
+  # ```
+  #
+  # This helper makes sure that you can run a task multiple times,
+  # even within the same test example.
+  def invoke_task(task_string)
+    Rake.application.invoke_task(task_string)
+  ensure
+    name, _args = Rake.application.parse_task_string(task_string)
+    Rake::Task[name].reenable
+  end
+end