From f532c4712e0cde1c84b948df0ae2c05ee21ce7cc Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 30 Jul 2025 16:20:06 +1000 Subject: [PATCH 1/3] Load rake tasks only once for code coverage Apparently, Rake's way of reloading the task code confuses the code coverage report. Code tested by rake task specs was not recognised as covered even though it was. --- .../lib/tasks/data/truncate_data_rake_spec.rb | 8 ++--- spec/lib/tasks/enterprises_rake_spec.rb | 10 ++---- .../tasks/import_product_images_rake_spec.rb | 16 +++------ spec/lib/tasks/reset_spec.rb | 8 ++--- spec/lib/tasks/sample_data_rake_spec.rb | 8 ++--- spec/lib/tasks/simplecov_spec.rb | 7 ++-- spec/lib/tasks/users_rake_spec.rb | 10 ++---- spec/support/shared_contexts/rake.rb | 35 +++++++++++++++++++ 8 files changed, 54 insertions(+), 48 deletions(-) create mode 100644 spec/support/shared_contexts/rake.rb diff --git a/spec/lib/tasks/data/truncate_data_rake_spec.rb b/spec/lib/tasks/data/truncate_data_rake_spec.rb index a04221200a..87a13616e5 100644 --- a/spec/lib/tasks/data/truncate_data_rake_spec.rb +++ b/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 diff --git a/spec/lib/tasks/enterprises_rake_spec.rb b/spec/lib/tasks/enterprises_rake_spec.rb index 2e98502f0e..89330dd47f 100644 --- a/spec/lib/tasks/enterprises_rake_spec.rb +++ b/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) diff --git a/spec/lib/tasks/import_product_images_rake_spec.rb b/spec/lib/tasks/import_product_images_rake_spec.rb index 7a6451b0f2..0e3ad6646f 100644 --- a/spec/lib/tasks/import_product_images_rake_spec.rb +++ b/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 diff --git a/spec/lib/tasks/reset_spec.rb b/spec/lib/tasks/reset_spec.rb index 99b0237bb5..43243e03ed 100644 --- a/spec/lib/tasks/reset_spec.rb +++ b/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) diff --git a/spec/lib/tasks/sample_data_rake_spec.rb b/spec/lib/tasks/sample_data_rake_spec.rb index f16a1e3be0..6cb9d509f4 100644 --- a/spec/lib/tasks/sample_data_rake_spec.rb +++ b/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 diff --git a/spec/lib/tasks/simplecov_spec.rb b/spec/lib/tasks/simplecov_spec.rb index 62204b12ef..f752ea1e84 100644 --- a/spec/lib/tasks/simplecov_spec.rb +++ b/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 @@ -17,7 +14,7 @@ RSpec.describe "simplecov.rake" do output_dir = File.join(tmp_dir, "output") expect { - Rake.application.invoke_task( + invoke_task( "simplecov:collate_results[#{input_dir},#{output_dir}]" ) }.to change { Dir.exist?(output_dir) }. diff --git a/spec/lib/tasks/users_rake_spec.rb b/spec/lib/tasks/users_rake_spec.rb index ebf5e8a5a1..b5b2b623e5 100644 --- a/spec/lib/tasks/users_rake_spec.rb +++ b/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 diff --git a/spec/support/shared_contexts/rake.rb b/spec/support/shared_contexts/rake.rb new file mode 100644 index 0000000000..3bb5891451 --- /dev/null +++ b/spec/support/shared_contexts/rake.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# Let this context take 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 From 2555a9e710fb6a9e0c58ee06087c5a8c7794a7fb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 5 Aug 2025 12:35:44 +1000 Subject: [PATCH 2/3] Ignore breaking code coverage for coverage spec When we test our code coverage compilation, it breaks the code coverage report for the current rspec process. By running that code separately, we gain a correct coverage report for the rest of the code again. So unfortunately, we can't report on the code coverage of this particular task and have to ignore it. But at least CI depends on the correct function of this task and would fail if it didn't work. --- .simplecov | 3 --- lib/tasks/simplecov.rake | 4 ++++ spec/lib/tasks/simplecov_spec.rb | 12 +++++++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.simplecov b/.simplecov index dd47f33e21..8f18990a6a 100755 --- a/.simplecov +++ b/.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 diff --git a/lib/tasks/simplecov.rake b/lib/tasks/simplecov.rake index cc7f5c95e6..369929c195 100644 --- a/lib/tasks/simplecov.rake +++ b/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 diff --git a/spec/lib/tasks/simplecov_spec.rb b/spec/lib/tasks/simplecov_spec.rb index f752ea1e84..e93b6324b7 100644 --- a/spec/lib/tasks/simplecov_spec.rb +++ b/spec/lib/tasks/simplecov_spec.rb @@ -13,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 { - 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). From 910ded1a8c66890f6246e78d2eaba3dda883f46f Mon Sep 17 00:00:00 2001 From: Maikel Date: Tue, 5 Aug 2025 13:49:44 +1000 Subject: [PATCH 3/3] Typo [skip ci] --- spec/support/shared_contexts/rake.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/shared_contexts/rake.rb b/spec/support/shared_contexts/rake.rb index 3bb5891451..4dcf5af07b 100644 --- a/spec/support/shared_contexts/rake.rb +++ b/spec/support/shared_contexts/rake.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Let this context take of Rake testing gotchas. +# Let this context take care of Rake testing gotchas. # # ```rb # RSpec.describe "my_task.rake" do