From 017a5802ec6e787ba646f63321647d011253bdcd Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 3 Oct 2025 17:57:20 +0200 Subject: [PATCH 1/6] Silence the puma server for admin specs (cherry picked from commit 58ddcfbf69e22daa0cc14abeeb18e8d9f8421dc9) --- admin/spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/admin/spec/spec_helper.rb b/admin/spec/spec_helper.rb index d69fa4e57b6..0bd65d5102e 100644 --- a/admin/spec/spec_helper.rb +++ b/admin/spec/spec_helper.rb @@ -53,6 +53,7 @@ Capybara.disable_animation = true Capybara.default_max_wait_time = ENV["DEFAULT_MAX_WAIT_TIME"].to_f if ENV["DEFAULT_MAX_WAIT_TIME"].present? Capybara.enable_aria_label = true +Capybara.server = :puma, {Silent: true} # A workaround for https://github.com/rspec/rspec-rails/issues/1897 # DATABASE CLEANER require "database_cleaner" From 753431bd6d85fcb82cde82a49d6dfb17cad72f96 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 3 Oct 2025 10:39:30 +0200 Subject: [PATCH 2/6] Test env: fix preview_paths FrozenError, speed up sqlite, fix dev container Three unrelated test-environment fixes that were blocking a local run. On Rails versions where `config.action_mailer.preview_paths` returns a frozen array, appending to it with `<<` raises FrozenError, so the dummy app builds a new array and assigns it instead. `fast_sqlite` is required for its side effect: it patches SQLite3::Database#initialize to set `PRAGMA synchronous = OFF` and `journal_mode = MEMORY`, which drops fsync and keeps the rollback journal in memory. That is unsafe for real data and fine for a suite that recreates its database; locally it took 500 inserts from ~105ms to ~7ms. The gem is declared `require: false` and only when DB is sqlite, hence the `rescue LoadError` guard. Note that it is a plain monkey patch on `#initialize`, so it takes effect whenever it is loaded; the placement at the top of the file is for visibility next to the other requires, not a load-order requirement. The dev container was still building on Ruby 3.1, which is below the `>= 3.2.0` in solidus_core.gemspec, so `bundle install` could not resolve inside it. Bump it to 3.4.6 to match the version the suite is developed against. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 2b1297ff85c7cf7f9c8d77e8eab5f7cda2d708fd) --- core/lib/spree/testing_support/dummy_app.rb | 17 ++++++++++++++++- docker-compose.yml | 6 +++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index e2be2ae170f..2489e2db051 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -3,6 +3,18 @@ ENV["RAILS_ENV"] = "test" ENV["DISABLE_DATABASE_ENVIRONMENT_CHECK"] = "1" +# Speed up the sqlite runs: `fast_sqlite` patches SQLite3::Database#initialize to +# set `PRAGMA synchronous = OFF` and `journal_mode = MEMORY`, trading durability we +# don't need for a database the suite recreates anyway. +# +# NOTE: The gem is declared `require: false`, and only when DB is sqlite, so it is +# genuinely absent for the mysql and postgres runs. +begin + require "fast_sqlite" +rescue LoadError + # Not running against sqlite, nothing to speed up. +end + require "rails" require "active_record/railtie" require "action_controller/railtie" @@ -106,7 +118,10 @@ class Application < ::Rails::Application # Set the preview path within the dummy app: if ActionMailer::Base.respond_to? :preview_paths # Rails 7.1+ - config.action_mailer.preview_paths << File.expand_path("dummy_app/mailer_previews", __dir__) + # Some Rails versions return a frozen array here; assign a new array + # to avoid FrozenError when augmenting the paths. + existing = Array(config.action_mailer.preview_paths) + config.action_mailer.preview_paths = existing + [File.expand_path("dummy_app/mailer_previews", __dir__)] else config.action_mailer.preview_path = File.expand_path("dummy_app/mailer_previews", __dir__) end diff --git a/docker-compose.yml b/docker-compose.yml index 7247adc0933..cf2b44666d6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,4 @@ -version: '3.7' +version: "3.7" services: mysql: @@ -18,12 +18,12 @@ services: - postgres:/var/lib/postgresql/data:cached app: - shm_size: '512mb' + shm_size: "512mb" build: context: .dockerdev dockerfile: Dockerfile args: - RUBY_VERSION: "3.1" + RUBY_VERSION: "3.4.6" PG_VERSION: 13 NODE_VERSION: 20 MYSQL_VERSION: "8.0" From d92804902c745be101c640330405744903bf5750 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 3 Oct 2025 17:57:57 +0200 Subject: [PATCH 3/6] Specs: make click_icon resilient to intercepted clicks When a floating element (tooltips/overlays) intercepts the click, scroll the target into view and dispatch the click via JS to keep specs stable. (cherry picked from commit 2d82d995ed7e213f56e361a7daffb9db05262733) --- .../lib/spree/testing_support/capybara_ext.rb | 10 ++- .../testing_support/capybara_ext_spec.rb | 76 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 core/spec/lib/spree/testing_support/capybara_ext_spec.rb diff --git a/core/lib/spree/testing_support/capybara_ext.rb b/core/lib/spree/testing_support/capybara_ext.rb index a567e23c819..0f3849f6796 100644 --- a/core/lib/spree/testing_support/capybara_ext.rb +++ b/core/lib/spree/testing_support/capybara_ext.rb @@ -4,7 +4,15 @@ module Spree module TestingSupport module CapybaraExt def click_icon(type) - find(".fa-#{type}").click + el = find(".fa-#{type}", visible: :all) + begin + el.click + rescue Selenium::WebDriver::Error::ElementClickInterceptedError + # When a floating element (eg. tooltips/overlays) intercepts the click, + # scroll the target into view and dispatch a click via JS to keep tests stable. + page.execute_script('arguments[0].scrollIntoView({block: "center"});', el.native) + page.execute_script("arguments[0].click();", el.native) + end end def eventually_fill_in(field, options = {}) diff --git a/core/spec/lib/spree/testing_support/capybara_ext_spec.rb b/core/spec/lib/spree/testing_support/capybara_ext_spec.rb new file mode 100644 index 00000000000..1d0390a69bf --- /dev/null +++ b/core/spec/lib/spree/testing_support/capybara_ext_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require "rails_helper" +require "selenium-webdriver" +require "spree/testing_support/capybara_ext" + +RSpec.describe Spree::TestingSupport::CapybaraExt do + subject(:page_object) do + Class.new do + include Spree::TestingSupport::CapybaraExt + + attr_reader :page + + def initialize(element:, page:) + @element = element + @page = page + end + + def find(*) = @element + end.new(element: element, page: page) + end + + let(:element) { instance_double(Capybara::Node::Element, native: :native_element) } + let(:page) { instance_double(Capybara::Session, execute_script: nil) } + + describe "#click_icon" do + it "clicks the icon it finds" do + allow(element).to receive(:click) + + page_object.click_icon(:edit) + + expect(element).to have_received(:click) + end + + context "when a floating element intercepts the click" do + before do + allow(element).to receive(:click) + .and_raise(Selenium::WebDriver::Error::ElementClickInterceptedError) + end + + it "scrolls the element into view and clicks it via JavaScript" do + page_object.click_icon(:edit) + + expect(page).to have_received(:execute_script) + .with('arguments[0].scrollIntoView({block: "center"});', :native_element).ordered + expect(page).to have_received(:execute_script) + .with("arguments[0].click();", :native_element).ordered + end + end + + context "when finding the icon is itself intercepted" do + subject(:page_object) do + Class.new do + include Spree::TestingSupport::CapybaraExt + + attr_reader :page + + def initialize(page:) + @page = page + end + + def find(*) + raise Selenium::WebDriver::Error::ElementClickInterceptedError + end + end.new(page: page) + end + + # The fallback needs an element to scroll to, so a failure to find one has to + # surface as itself rather than as a NoMethodError on nil. + it "lets the error through instead of retrying without an element" do + expect { page_object.click_icon(:edit) } + .to raise_error(Selenium::WebDriver::Error::ElementClickInterceptedError) + end + end + end +end From 82252ae6e0e96059297b72a3fb9d995d33ecfd5d Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Sun, 2 Aug 2026 17:11:23 +0200 Subject: [PATCH 4/6] CI: install libvips with apt-get instead of a cached action All twelve `activestorage` matrix jobs were failing with an opaque `undefined method 'new' for nil`, raised where ActiveStorage resolves `ImageProcessing.const_get(ActiveStorage.variant_processor.to_s.camelize)`. The constant was nil because libvips was never installed. `awalsh128/cache-apt-pkgs-action@v1` was reporting a cache hit and then restoring nothing: Cache hit for: cache-apt-pkgs_c6a33a4bc2050d519bb8c70e7705bd4e Restoring 0 packages from cache... The same key had restored 79 packages on previously green runs, so the cache entry had gone bad. Because the action still exits successfully, a poisoned entry is indistinguishable from a working one until the specs fail for a seemingly unrelated reason. Bumping the action's `version` input would mint a fresh key and restore green, but it re-arms the same trap. Install the package directly instead, matching what solidus_installer.yml already does, and run `vips --version` so a bad install fails at this step rather than deep in the suite. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 019551bc3025aa0af9b4948de2798d1b2a694262) --- .github/workflows/test_solidus.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test_solidus.yml b/.github/workflows/test_solidus.yml index a1818a5b63e..268fb3b29ce 100644 --- a/.github/workflows/test_solidus.yml +++ b/.github/workflows/test_solidus.yml @@ -83,11 +83,17 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true - - uses: awalsh128/cache-apt-pkgs-action@v1 + # NOTE: Install libvips with plain apt-get rather than a caching action. A + # cached install can report a cache hit while restoring zero packages, which + # leaves libvips missing and surfaces much later as an opaque + # "undefined method 'new' for nil" from ImageProcessing::Vips inside the + # specs. `vips --version` makes a bad install fail here instead. + - name: Install libvips if: ${{ matrix.storage == 'activestorage' }} - name: Install libvips - with: - packages: libvips-dev + run: | + sudo apt-get update + sudo apt-get install -yq libvips-dev + vips --version - name: Setup coverage id: setup-coverage if: ${{ inputs.coverage && matrix.rails == '8.0' && matrix.ruby == '3.4' }} From 937c4318d76e2a6e8ce23325f0f8b11301925503 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Sun, 2 Aug 2026 17:16:30 +0200 Subject: [PATCH 5/6] CI: run the RSpec matrix on ubuntu-24.04 for libvips 8.13+ With libvips actually installing again, every Rails 8.0 and 8.1 job failed at load time, before a single example ran: libvips's unfuzzed operations are not safe to use with untrusted content, and Active Storage cannot disable them. Disabling them requires libvips 8.13 or later and ruby-vips 2.2.1 or later. ActiveStorage 8.1 raises this from active_storage/vips.rb while its engine is being required, so the whole suite dies in `rake test_app` regardless of which specs would have run. Rails 7.2 has no such check, which is why only the 8.x rows were affected. ubuntu-22.04 ships libvips 8.12.1, one version below the floor. ubuntu-24.04 ships 8.15, and is already what install_dummy_app.yml and solidus_installer.yml use. The poisoned apt cache had been masking this: with no libvips present, ruby-vips never loaded and the version check never ran. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit fc0ab3c1c78323b08d4660a933d8d0aa39ae29f6) --- .github/workflows/test_solidus.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test_solidus.yml b/.github/workflows/test_solidus.yml index 268fb3b29ce..2b80ffc7c51 100644 --- a/.github/workflows/test_solidus.yml +++ b/.github/workflows/test_solidus.yml @@ -16,7 +16,10 @@ on: jobs: RSpec: name: Rails ${{ matrix.rails }}, Ruby ${{ matrix.ruby }}, ${{ matrix.database }}, ${{ matrix.storage }} - runs-on: ubuntu-22.04 + # NOTE: ubuntu-24.04 is required for libvips 8.13+. ActiveStorage 8.1 refuses to + # load with older libvips, because it cannot disable the unfuzzed operations that + # are unsafe on untrusted content. ubuntu-22.04 only ships libvips 8.12.1. + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: From 8d5d0734f7f5b09584b1891cc1e0181b8f859e79 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Sun, 2 Aug 2026 19:55:43 +0200 Subject: [PATCH 6/6] CI: skip libvips packages the suite does not need apt pulls libvips-doc and nip2, a GUI image editor, as recommendations of libvips-dev. ruby-vips binds libvips.so through FFI at runtime rather than compiling against it, so none of that is needed and the download drops from 67.4MB to 47.0MB. libvips-tools has to be named explicitly, since it is only recommended by libvips-dev but provides the `vips` binary the verification step runs. Worth being honest about the payoff: across six jobs each, the step averaged 33.6s before and 29.0s after, but individual samples ranged from 21s to 48s. Mirror throughput dominates, so the ~5s is real but small against the noise. `apt-get update` stays. Skipping it and only refreshing on failure looked tempting, but the runner's apt index is stale often enough to 404 on a transitive dependency mid-install. Co-Authored-By: Claude Opus 5 (1M context) (cherry picked from commit 002f3843f57ac5fb0c647285bcd9deb3bd10fb45) --- .github/workflows/test_solidus.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test_solidus.yml b/.github/workflows/test_solidus.yml index 2b80ffc7c51..8471354814d 100644 --- a/.github/workflows/test_solidus.yml +++ b/.github/workflows/test_solidus.yml @@ -91,11 +91,15 @@ jobs: # leaves libvips missing and surfaces much later as an opaque # "undefined method 'new' for nil" from ImageProcessing::Vips inside the # specs. `vips --version` makes a bad install fail here instead. + # + # ruby-vips binds libvips.so through FFI at runtime, so the docs and the GUI + # tools apt recommends are dead weight; libvips-tools is requested explicitly + # because it provides the `vips` binary used by the check below. - name: Install libvips if: ${{ matrix.storage == 'activestorage' }} run: | sudo apt-get update - sudo apt-get install -yq libvips-dev + sudo apt-get install -yq --no-install-recommends libvips-dev libvips-tools vips --version - name: Setup coverage id: setup-coverage