Skip to content

Fix a few test environment issues - #6532

Merged
elia merged 6 commits into
mainfrom
elia/test-env-fixes
Aug 3, 2026
Merged

Fix a few test environment issues#6532
elia merged 6 commits into
mainfrom
elia/test-env-fixes

Conversation

@elia

@elia elia commented Aug 2, 2026

Copy link
Copy Markdown
Member

Summary

Test-environment and CI fixes, extracted from #6329 so that PR can stay focused on
Spree.admin_user_class. No production code is touched — everything here is
testing support, CI configuration, or local tooling.

The two CI commits fix the currently-red activestorage matrix. They turned out to
be a chain: the first failure was masking the second, so both are needed to get
green.

What's in here

CI: libvips

Install libvips with apt-get instead of a cached action — every activestorage
job was 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 reported a cache hit and then restored 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 entry had
gone bad. The action still exits 0, which makes a poisoned cache 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 — so this installs the package directly, matching what
solidus_installer.yml already does, and runs vips --version so a bad install
fails loudly at that step.

Run the RSpec matrix on ubuntu-24.04 — 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.1, and is already what install_dummy_app.yml and solidus_installer.yml use.

The poisoned cache had been hiding this: with no libvips present, ruby-vips never
loaded and the version check never ran.

Skip packages the suite does not need--no-install-recommends drops
libvips-doc and nip2 (a GUI image editor), taking the download from 67.4MB to
47.0MB. libvips-tools is then requested explicitly, since it is only a
recommendation of libvips-dev but provides the vips binary the check runs.

On the cost of dropping the cache

Being upfront, since it is a real regression and you have to live with the choice.
Timing the install step across six jobs each:

download mean range
cached action (when the cache worked) ~6.5s single sample
plain apt-get 67.4MB 33.6s 20.9–47.5s
plain apt-get, no recommends 47.0MB 29.0s 21.5–38.1s

So this trades roughly 22s per job for an install that cannot silently no-op. Those
jobs run in parallel and each takes ~7 minutes, so pipeline latency grows by about
22s once, not 28×. Mirror throughput dominates the variance, which is why trimming
20MB only buys ~5s.

If you would rather have the time back, caching is reasonable — the request is that
any cache layer be guarded by something like vips --version || <install>, so a
stale entry degrades loudly at that step instead of resurfacing as
undefined method 'new' for nil deep in the specs.

Test environment

Silence the puma server for admin specs — sets Capybara.server = :puma, {Silent: true} in admin/spec/spec_helper.rb, matching what
core/lib/spree/testing_support/capybara_ext.rb already does. Works around
rspec-rails#1897, which
otherwise floods admin spec output with server logs.

Fix the ActionMailer preview_paths FrozenError, speed up sqlite, fix the dev
container
— on Rails versions where config.action_mailer.preview_paths returns a
frozen array, << raises FrozenError, so the dummy app assigns a new array
instead.

fast_sqlite is required for its side effect: it patches
SQLite3::Database#initialize to set PRAGMA synchronous = OFF and
journal_mode = MEMORY, dropping fsync for a database the suite recreates anyway
(locally, 500 inserts went from ~105ms to ~7ms). It is declared require: false and
only when DB is sqlite, hence the rescue LoadError.

And docker-compose.yml was still building the dev container on Ruby 3.1, below the
>= 3.2.0 in solidus_core.gemspec, so bundle install could not resolve inside
it; it now matches the version the suite is developed against.

Make click_icon resilient to intercepted clicks — when a floating element
(tooltip/overlay) sits over the target, Selenium raises
ElementClickInterceptedError. The helper now rescues it, scrolls the target
into view, and dispatches the click via JS. The rescue is scoped to the click
itself: covering the find as well meant that when finding the icon raised, the
fallback ran with a nil element and died with undefined method 'native' for nil,
hiding the real error. Comes with a unit spec covering both branches.

How to test

CI is the test for the libvips commits: the activestorage RSpec jobs pass on this
branch, and the Install libvips step logs vips-8.15.1. Before these commits, 12
of them failed with undefined method 'new' for nil.

bin/rake lint:rb is clean. The test-environment changes are exercised implicitly —
dummy_app.rb and capybara_ext.rb are loaded by every gem's specs, and
spec_helper.rb by the admin specs:

cd core && bundle exec rake test_app && bundle exec rspec
cd admin && bundle exec rake test_app && bundle exec rspec

On the red codecov/project check

It reports 92.14% (-2.94%), but nothing here reduced coverage — Codecov's own
file-level comparison lists zero files with a coverage change, and codecov/patch
is at 100%. The totals it is comparing are not the same shape:

  • base (72f9fcd): 7,534 / 7,924 lines across 460 files
  • head: 19,488 / 21,149 lines across 1,035 files

72f9fcd is the merge commit for #6509 and never ran the Test workflow, so it has no
coverage report of its own and the baseline falls back to older, partial data
covering about a third of the codebase. This branch uploads the full set because all
seven coverage jobs pass, which tripled the denominator and dropped the percentage.

Worth a maintainer's glance, since it will keep flagging on PRs until a full report
lands on main.

For maintainers to decide

mini_magick is still a declared dependency in core/solidus_core.gemspec, while
the test environment is effectively vips-only (dummy_app.rb defaults
variant_processor to :vips). Whether Solidus wants to stay vips-only in test, or
grow a graceful fallback when neither binary is present, seemed like a maintainer
call rather than something to decide inside a CI fix. Worth noting that the failure
mode when the processor is missing is the unhelpful undefined method 'new' for nil
shown above.

Also note test_solidus_rubocops.yml still runs on ubuntu-22.04. It does not touch
libvips, so it is unaffected and left alone.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • I have added tests: core/spec/lib/spree/testing_support/capybara_ext_spec.rb covers both click_icon branches. The remaining changes are CI configuration and test-harness plumbing exercised by the existing suites.

@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.14%. Comparing base (72f9fcd) to head (002f384).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6532      +/-   ##
==========================================
- Coverage   95.07%   92.14%   -2.94%     
==========================================
  Files         460     1035     +575     
  Lines        7924    21149   +13225     
==========================================
+ Hits         7534    19488   +11954     
- Misses        390     1661    +1271     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@elia
elia force-pushed the elia/test-env-fixes branch 4 times, most recently from f534455 to 88c6bcc Compare August 2, 2026 18:17
elia and others added 5 commits August 3, 2026 00:21
…tainer

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) <noreply@anthropic.com>
When a floating element (tooltips/overlays) intercepts the click, scroll
the target into view and dispatch the click via JS to keep specs stable.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@elia
elia force-pushed the elia/test-env-fixes branch from 88c6bcc to 002f384 Compare August 2, 2026 22:22
@elia elia self-assigned this Aug 2, 2026

@mamhoff mamhoff left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Makes the test suite green again!

@elia
elia merged commit ccc5ad8 into main Aug 3, 2026
38 of 39 checks passed
@elia
elia deleted the elia/test-env-fixes branch August 3, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants