From 8d747a2508dab4fca4efece2792f77864cd37bcb Mon Sep 17 00:00:00 2001 From: Joseph Johansen Date: Wed, 21 Aug 2024 11:34:12 +0100 Subject: [PATCH 1/7] Enable coverage in base_spec_helper --- spec/base_spec_helper.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index b56cb83492..5579890b64 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -4,8 +4,9 @@ ENV["RAILS_ENV"] ||= 'test' +# for full configuration, see .simplecov require 'simplecov' if ENV["COVERAGE"] -require 'rubygems' + require 'pry' unless ENV['CI'] require 'view_component/test_helpers' @@ -27,6 +28,12 @@ end require 'knapsack_pro' KnapsackPro::Adapters::RSpecAdapter.bind +if ENV["COVERAGE"] && defined?(SimpleCov) + KnapsackPro::Hooks::Queue.before_queue do + SimpleCov.command_name("rspec_ci_node_#{KnapsackPro::Config::Env.ci_node_index}") + end +end + # Allow connections to selenium whilst raising errors when connecting to external sites require 'webmock/rspec' WebMock.enable! From 922b853e3a0a450a1249dcc66b0a528d70ad1905 Mon Sep 17 00:00:00 2001 From: Joseph Johansen Date: Wed, 21 Aug 2024 11:35:03 +0100 Subject: [PATCH 2/7] Define specs for rake task to combine results --- .../simplecov/customer/.resultset.json | 85 +++++++++++++++++++ .../simplecov/voucher/.resultset.json | 68 +++++++++++++++ spec/lib/tasks/simplecov_spec.rb | 30 +++++++ 3 files changed, 183 insertions(+) create mode 100644 spec/fixtures/simplecov/customer/.resultset.json create mode 100644 spec/fixtures/simplecov/voucher/.resultset.json create mode 100644 spec/lib/tasks/simplecov_spec.rb diff --git a/spec/fixtures/simplecov/customer/.resultset.json b/spec/fixtures/simplecov/customer/.resultset.json new file mode 100644 index 0000000000..130cfe0072 --- /dev/null +++ b/spec/fixtures/simplecov/customer/.resultset.json @@ -0,0 +1,85 @@ +{ + "RSpec": { + "coverage": { + "/Users/josephjohansen/code/open_food_foundation/openfoodnetwork/app/models/customer.rb": { + "lines": [ + null, + null, + null, + null, + null, + null, + null, + null, + null, + 1, + 1, + null, + 1, + null, + 1, + null, + 1, + 1, + 1, + 1, + 1, + 1, + 1, + null, + 1, + 1, + 1, + null, + 1, + 1, + 1, + null, + 1, + 1, + null, + null, + null, + null, + null, + 8, + 1, + 6, + null, + 2, + 2, + null, + 1, + null, + 1, + 0, + null, + null, + 1, + null, + 1, + 31, + null, + null, + 1, + 31, + null, + null, + 1, + 24, + null, + null, + 1, + 0, + 0, + 0, + null, + 0, + null, + null + ] + } + }, + "timestamp": 1724235565 + } +} diff --git a/spec/fixtures/simplecov/voucher/.resultset.json b/spec/fixtures/simplecov/voucher/.resultset.json new file mode 100644 index 0000000000..db9d793e0e --- /dev/null +++ b/spec/fixtures/simplecov/voucher/.resultset.json @@ -0,0 +1,68 @@ +{ + "RSpec": { + "coverage": { + "/Users/josephjohansen/code/open_food_foundation/openfoodnetwork/app/models/voucher.rb": { + "lines": [ + null, + null, + 1, + 1, + null, + 1, + null, + 1, + null, + null, + null, + 1, + null, + null, + null, + null, + 1, + null, + 1, + null, + 1, + 15, + null, + null, + null, + null, + null, + null, + null, + null, + 1, + null, + 3, + null, + null, + null, + null, + null, + null, + null, + null, + 3, + null, + null, + null, + 1, + 1, + null, + null, + 1, + 1, + null, + null, + 1, + 1, + null, + null + ] + } + }, + "timestamp": 1724239602 + } +} diff --git a/spec/lib/tasks/simplecov_spec.rb b/spec/lib/tasks/simplecov_spec.rb new file mode 100644 index 0000000000..e51ee4a1e2 --- /dev/null +++ b/spec/lib/tasks/simplecov_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rake' + +RSpec.describe "simplecov.rake" do + let(:output_dir) { "tmp/test_#{Process.pid}_coverage" } + + before(:all) do + Rake.application.rake_require("tasks/simplecov") + end + + describe "simplecov:collate_results" do + context "when there are reports to merge" do + let(:input_dir) { Rails.root.join("spec/fixtures/simplecov") } + + it "creates a new combined report" do + expect { + Rake.application.invoke_task "simplecov:collate_results[#{input_dir},#{output_dir}]" + }.to change { Dir.exist? output_dir }. + from(false). + to(true). + + and change { File.exist? File.join(output_dir, "index.html") }. + from(false). + to(true) + end + end + end +end From 5f9b14df9f4d552c5495ba17aadd819036ebad4b Mon Sep 17 00:00:00 2001 From: Joseph Johansen Date: Wed, 21 Aug 2024 11:35:20 +0100 Subject: [PATCH 3/7] Implement rake task to combine results --- lib/tasks/simplecov.rake | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 lib/tasks/simplecov.rake diff --git a/lib/tasks/simplecov.rake b/lib/tasks/simplecov.rake new file mode 100644 index 0000000000..bc190c1fe4 --- /dev/null +++ b/lib/tasks/simplecov.rake @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +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| + require "simplecov" + + path_to_results = args[:path_to_results].presence || "tmp/simple-cov" + coverage_dir = args[:coverage_dir].presence || "coverage" + + SimpleCov.collate Dir[File.join(path_to_results, "**", ".resultset.json")], "rails" do + formatter SimpleCov::Formatter::HTMLFormatter + + coverage_dir coverage_dir + end + end +end From a816814819980470058cfe013a89e3c453d2dc98 Mon Sep 17 00:00:00 2001 From: Joseph Johansen Date: Wed, 21 Aug 2024 11:37:07 +0100 Subject: [PATCH 4/7] Update CI workflow to upload results and call rake task --- .github/workflows/build.yml | 107 ++++++++++++++++++++++++++++++++---- .simplecov | 2 + lib/tasks/simplecov.rake | 2 +- 3 files changed, 98 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c5c7ba16f8..6b6db9ad81 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -3,7 +3,7 @@ name: Build on: workflow_dispatch: push: - branches-ignore: + branches-ignore: - 'dependabot/**' pull_request: @@ -47,7 +47,7 @@ jobs: - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -81,11 +81,19 @@ jobs: # RSpec split test files by test examples feature - it's optional # https://knapsackpro.com/faq/question/how-to-split-slow-rspec-test-files-by-test-examples-by-individual-it #KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES: true - KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/controllers/**/*_spec.rb}" + KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/controllers/**/*_spec.rb}" run: | git show --no-patch # the commit being tested (which is often a merge due to actions/checkout@v3) bin/rake knapsack_pro:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-controllers-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + models: runs-on: ubuntu-22.04 services: @@ -116,7 +124,7 @@ jobs: - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -141,10 +149,18 @@ jobs: # RSpec split test files by test examples feature - it's optional # https://knapsackpro.com/faq/question/how-to-split-slow-rspec-test-files-by-test-examples-by-individual-it #KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES: true - KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/models/**/*_spec.rb}" + KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/models/**/*_spec.rb}" run: | bin/rake knapsack_pro:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-models-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + system_admin: runs-on: ubuntu-22.04 services: @@ -175,7 +191,7 @@ jobs: - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -209,11 +225,19 @@ jobs: # RSpec split test files by test examples feature - it's optional # https://knapsackpro.com/faq/question/how-to-split-slow-rspec-test-files-by-test-examples-by-individual-it #KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES: true - KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/system/admin/**/*_spec.rb}" + KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/system/admin/**/*_spec.rb}" run: | bin/rake knapsack_pro:queue:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-system-admin-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + - name: Archive failed tests screenshots if: failure() uses: actions/upload-artifact@v4 @@ -247,13 +271,13 @@ jobs: ci_node_total: [12] # Indexes for parallel jobs (starting from zero). # E.g. use [0, 1] for 2 parallel jobs, [0, 1, 2] for 3 parallel jobs, etc. - ci_node_index: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] + ci_node_index: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] steps: - uses: actions/checkout@v3 - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -287,11 +311,19 @@ jobs: # RSpec split test files by test examples feature - it's optional # https://knapsackpro.com/faq/question/how-to-split-slow-rspec-test-files-by-test-examples-by-individual-it #KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES: true - KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/system/consumer/**/*_spec.rb}" + KNAPSACK_PRO_TEST_FILE_PATTERN: "{spec/system/consumer/**/*_spec.rb}" run: | bin/rake knapsack_pro:queue:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-system-consumer-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + - name: Archive failed tests screenshots if: failure() uses: actions/upload-artifact@v4 @@ -331,7 +363,7 @@ jobs: - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -371,6 +403,14 @@ jobs: run: | bin/rake knapsack_pro:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-engines-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + test_the_rest: runs-on: ubuntu-22.04 services: @@ -401,7 +441,7 @@ jobs: - name: Setup redis uses: supercharge/redis-github-action@1.4.0 - with: + with: redis-version: 6 - name: Set up Ruby @@ -439,6 +479,14 @@ jobs: run: | bin/rake knapsack_pro:rspec + - name: Save SimpleCov file + uses: actions/upload-artifact@v3 + with: + name: __simplecov-chunk-the-rest-${{ matrix.ci_node_index }} + path: coverage/*.* + retention-days: 2 # doesn't need to be long, because it's the combined results that matter + if-no-files-found: ignore + non_knapsack_jest_karma: runs-on: ubuntu-22.04 services: @@ -476,3 +524,38 @@ jobs: - name: Run jest tests run: yarn jest + + collate_simplecov_results: + runs-on: ubuntu-22.04 + needs: + - controllers + - models + - engines + - system_admin + - system_consumer + - test_the_rest + steps: + - uses: actions/checkout@v3 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true # runs 'bundle install' and caches installed gems automatically + + - name: Download individual results from individual runners + uses: actions/download-artifact@v3 + with: + pattern: __simplecov-chunk-* + path: tmp/simplecov + merge-multiple: true + + - name: collate results from each of the workers + run: bundle exec rake 'simplecov:collate_results[tmp/simplecov]' + + - name: Upload collated results + uses: actions/upload-artifact@v3 + with: + name: simplecov-combined-results + path: coverage/*.* + retention-days: 7 + if-no-files-found: ignore diff --git a/.simplecov b/.simplecov index 0094b3ddcd..8cf02c6b54 100755 --- a/.simplecov +++ b/.simplecov @@ -14,4 +14,6 @@ SimpleCov.start 'rails' do add_filter '/log' add_filter '/db' add_filter '/lib/tasks/sample_data/' + + formatter SimpleCov::Formatter::SimpleFormatter end diff --git a/lib/tasks/simplecov.rake b/lib/tasks/simplecov.rake index bc190c1fe4..b1a50737d7 100644 --- a/lib/tasks/simplecov.rake +++ b/lib/tasks/simplecov.rake @@ -6,7 +6,7 @@ namespace :simplecov do [:path_to_results, :coverage_dir] do |_t, args| require "simplecov" - path_to_results = args[:path_to_results].presence || "tmp/simple-cov" + path_to_results = args[:path_to_results].presence || "tmp/simplecov" coverage_dir = args[:coverage_dir].presence || "coverage" SimpleCov.collate Dir[File.join(path_to_results, "**", ".resultset.json")], "rails" do From 85385a198983557b62e594317aa13f27888051a4 Mon Sep 17 00:00:00 2001 From: Joseph Johansen Date: Fri, 23 Aug 2024 11:11:16 +0100 Subject: [PATCH 5/7] Rename uploads so combined report is listed first alphabetically --- .github/workflows/build.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6b6db9ad81..6358e6cd8d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -89,7 +89,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-controllers-${{ matrix.ci_node_index }} + name: simplecov-chunk-controllers-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -156,7 +156,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-models-${{ matrix.ci_node_index }} + name: simplecov-chunk-models-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -233,7 +233,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-system-admin-${{ matrix.ci_node_index }} + name: simplecov-chunk-system-admin-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -319,7 +319,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-system-consumer-${{ matrix.ci_node_index }} + name: simplecov-chunk-system-consumer-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -406,7 +406,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-engines-${{ matrix.ci_node_index }} + name: simplecov-chunk-engines-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -482,7 +482,7 @@ jobs: - name: Save SimpleCov file uses: actions/upload-artifact@v3 with: - name: __simplecov-chunk-the-rest-${{ matrix.ci_node_index }} + name: simplecov-chunk-the-rest-${{ matrix.ci_node_index }} path: coverage/*.* retention-days: 2 # doesn't need to be long, because it's the combined results that matter if-no-files-found: ignore @@ -545,7 +545,7 @@ jobs: - name: Download individual results from individual runners uses: actions/download-artifact@v3 with: - pattern: __simplecov-chunk-* + pattern: simplecov-chunk-* path: tmp/simplecov merge-multiple: true @@ -555,7 +555,7 @@ jobs: - name: Upload collated results uses: actions/upload-artifact@v3 with: - name: simplecov-combined-results + name: combined-simplecov-report path: coverage/*.* retention-days: 7 if-no-files-found: ignore From a696c66857d530e22226645d08a8f6535a50462f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 28 Aug 2024 11:27:42 +1000 Subject: [PATCH 6/7] Clean up tmp dir after test and avoid collisions Best viewed ignoring whitespace changes. --- spec/lib/tasks/simplecov_spec.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/spec/lib/tasks/simplecov_spec.rb b/spec/lib/tasks/simplecov_spec.rb index e51ee4a1e2..68e9d0bc5e 100644 --- a/spec/lib/tasks/simplecov_spec.rb +++ b/spec/lib/tasks/simplecov_spec.rb @@ -4,8 +4,6 @@ require 'spec_helper' require 'rake' RSpec.describe "simplecov.rake" do - let(:output_dir) { "tmp/test_#{Process.pid}_coverage" } - before(:all) do Rake.application.rake_require("tasks/simplecov") end @@ -15,15 +13,21 @@ RSpec.describe "simplecov.rake" do let(:input_dir) { Rails.root.join("spec/fixtures/simplecov") } it "creates a new combined report" do - expect { - Rake.application.invoke_task "simplecov:collate_results[#{input_dir},#{output_dir}]" - }.to change { Dir.exist? output_dir }. - from(false). - to(true). + Dir.mktmpdir do |tmp_dir| + output_dir = File.join(tmp_dir, "output") - and change { File.exist? File.join(output_dir, "index.html") }. - from(false). - to(true) + expect { + Rake.application.invoke_task( + "simplecov:collate_results[#{input_dir},#{output_dir}]" + ) + }.to change { Dir.exist?(output_dir) }. + from(false). + to(true). + + and change { File.exist?(File.join(output_dir, "index.html")) }. + from(false). + to(true) + end end end end From 64470e977a2ca644e7706fa2af81c9b0b1646617 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 28 Aug 2024 11:31:01 +1000 Subject: [PATCH 7/7] Avoid name clash in simplecov task A variable and a method were called the same. Also made method calls more obvious with parenthesis. --- lib/tasks/simplecov.rake | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/tasks/simplecov.rake b/lib/tasks/simplecov.rake index b1a50737d7..b0c896ecb7 100644 --- a/lib/tasks/simplecov.rake +++ b/lib/tasks/simplecov.rake @@ -7,12 +7,11 @@ namespace :simplecov do require "simplecov" path_to_results = args[:path_to_results].presence || "tmp/simplecov" - coverage_dir = args[:coverage_dir].presence || "coverage" + output_path = args[:coverage_dir].presence || "coverage" SimpleCov.collate Dir[File.join(path_to_results, "**", ".resultset.json")], "rails" do - formatter SimpleCov::Formatter::HTMLFormatter - - coverage_dir coverage_dir + formatter(SimpleCov::Formatter::HTMLFormatter) + coverage_dir(output_path) end end end