ci: exercise the vips driver on JRuby - #243
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe test workflow now applies longer job and retry timeouts to JRuby matrix cells. It also selects ChangesCI runtime configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The CI workflow now exercises the vips driver on JRuby while preserving existing coverage elsewhere; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideConfigures CI to explicitly exercise the vips screenshot driver on JRuby by setting SCREENSHOT_DRIVER per matrix cell, ensuring the vips integration path is driven end-to-end without changing behavior for MRI runs. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
CI update — the JRuby cells did not pass. Marking this PR as not-ready pending diagnosis. All CRuby cells are green, including From For reference, the same job on master with chunky_png finishes the whole suite in about three minutes: Two things this is not:
So something environment-specific is stalling on the Linux runner that does not reproduce on macOS/arm64. Running the full suite locally on JRuby + vips now to establish whether it is slow-but-finite or genuinely hung; will post the timing. Do not merge as-is. |
The JRuby cells install libvips and ruby-vips already -- the setup action
installs libvips unconditionally and gems.rb declares ruby-vips with no
platform guard -- so `Drivers.detect_available` has always reported
`vips, chunky_png` there, and the vips-gated unit tests have always run.
What never ran on JRuby was the integration path: `bin/rake test` leaves
SCREENSHOT_DRIVER unset, and test/system_test_case.rb defaults that to
chunky_png. So capture -> compare -> annotate, plus the vips cache flush
in that file's teardown, were MRI-only.
Point the JRuby cells at vips. Non-JRuby cells get chunky_png spelled
out, which is the default they already had.
Verified locally on JRuby 10.0.6.0 + libvips 8.18.5:
test/unit/drivers/vips_driver_test.rb
50 runs, 104 assertions, 0 failures, 0 errors, 0 skips
test/integration (cuprite, SCREENSHOT_DRIVER=vips)
28 runs, 45 assertions, 0 failures, 0 errors, 1 skips
Identical to the MRI 4.0.6 control on the same machine (28 runs, 45
assertions, 0 failures, 0 errors, 1 skips).
Separate from the vips switch and a fix for a failure already on master: every JRuby cell of run 32638878557 (master b1d01af, chunky_png, no change from this branch) burns all three attempts on `Timeout of 420000ms hit` and is killed at the 20-minute job cap. The cells have stopped gating anything. The suite outgrew the budget -- it is ~600 tests now. Measured on JRuby 10.0.6.0 locally, same machine, same suite, only SCREENSHOT_DRIVER differing: vips 598 runs, 1683 assertions, 0 failures, 0 errors -- 390.4s chunky_png 598 runs, 1683 assertions, 0 failures, 0 errors -- 434.4s vips is the faster of the two, so this is not driver cost. Raise the per-attempt budget to 15 minutes and the job cap to 25 so one attempt plus setup fits with headroom. A passing cell still costs ~8 minutes; a failing one costs 25 instead of 20, but produces a result rather than a cancellation.
|
Diagnosed — the JRuby timeout is pre-existing on master and has nothing to do with vips. Rebased onto current master and added the budget fix; this PR is now two commits. The controlRun 32638878557, master The JRuby cells have stopped gating anything on master. My first push just inherited that. And vips is the faster driverJRuby 10.0.6.0, same machine, same full suite, only
vips is 10% faster and equally correct. The suite simply outgrew the 7-minute per-attempt budget — it is ~600 tests now, up from the 445 that used to finish in about three minutes. Second commit
It is a separate concern from the vips switch, so it is a separate commit — happy to split it into its own PR and rebase this on top if you would rather land the budget fix independently, since it fixes master either way. |
vips on JRuby is proven green on real CI
8m17s. The 2.1 chunky_png removal does not drop JRuby support. Three JRuby cells were still cancelled — pre-existing, not vips
~230 of 600 tests in 15 minutes, against 600 in 8m17s in the cell that passed. Different seed, same code. So there is an intermittent, seed-dependent hang in the JRuby suite. It predates this branch. Master That is a separate defect and I have not tried to fix it here. Worth its own issue — a timeout cannot fix a hang. Why the second commit staysNot papering over the hang: 7 minutes was simply too small. The cell that passed cleanly needed 8m17s. At the old budget even a healthy JRuby run could not finish, which is why every cell on master is cancelled rather than failing with a result. Summary
|
The retry budget was larger than the job cap on both engines, so the last attempt was always killed partway and the cell reported `cancelled`. Measured on master run 32643567648 (post-#243), the 5 JRuby cells: jruby-10.0 rails81 suite 713s, no hang, 1 attempt -> 12m02 pass jruby-10.0 rails71 suite 545s + 6m hang -> 15m t/o; attempt 2 clean at 543s -> 24m20 pass jruby-10.0 rails80 attempt 1 hung -> 15m t/o; attempt 2 killed at the 25m cap -> cancelled jruby-10.0 rails72 same -> cancelled jruby-head rails81 same -> cancelled 3 of 5 JRuby cells gate nothing. The cause is arithmetic, not the driver: max_attempts 3 x timeout_minutes 15 = 45 min against timeout-minutes 25. Attempt 3 could never start, and attempt 2 had only 25 - 15 - 0.7 = 9.3 min to finish a run that measures 9-12 min -- so whether a cell survived a hang came down to which gemfile it drew. MRI has the same shape, smaller: 3x3 = 9 against a cap of 8. - JRuby job cap 25 -> 31, so 1 + 15 + 15 fits. - max_attempts 3 -> 2 on both engines, since 3 was never reachable. A doubly-hung JRuby cell now costs 31 min instead of 25, but today's 25 min buys no verdict at all. Coverage, drivers and the full-ci/cron opt-in model are unchanged. Also adds the concurrency group Lint never had, so superseded PR pushes stop running the linter to completion.
1 participant
Why
Ahead of 2.1 removing the chunky_png driver, we needed to know whether
ruby-vipsactually works on JRuby — otherwise that removal silently drops JRuby support.It works. This PR makes CI prove it instead of assuming it.
What was already true
Nothing was missing from the JRuby cells:
.github/actions/setup-ruby-and-dependenciesinstalls libvips unconditionally (cached path or apt fallback), including for JRuby.gems.rbdeclaresgem "ruby-vips", require: falsewith no platform guard, so JRuby installs it. (oily_pngis the only gem withplatform: :ruby.)So
SnapDiff::Drivers.detect_availablehas always returned[:vips, :chunky_png]on JRuby, and everyskip ... unless defined?(Vips)unit test has always run there. Job log from master run 32590002736,Test Ruby & Rails (jruby-10.0, rails81_gems.rb):test/test_helper.rbwould have aborted the whole run viaDriverCoverage.missing_for_ciif vips had not loaded, so those green cells are themselves proof thatrequire "vips"succeeds on JRuby on Linux.What was missing
The integration path.
bin/rake testleavesSCREENSHOT_DRIVERunset, andtest/system_test_case.rb:24defaults it tochunky_png. So on JRuby the vips FFI binding was only ever loaded, never driven — no capture → compare → annotate, and never theVips.cache_set_maxflush in that file'steardown.Test Driverscovers both drivers end to end, but only on CRuby 4.0.The change
One env line on the
Test Ruby & Railsjob. Non-JRuby cells getchunky_pngspelled out, which is the default they already had — no behaviour change there. No new cells, no matrix growth, no extra Actions minutes.Evidence (empirical, local)
JRuby 10.0.6.0 (3.4.5) on OpenJDK 26, ruby-vips 2.3.0, libvips 8.18.5.
Driver contract tests:
Other vips-touching unit tests (
image_compare,annotation_service,compare_api,screenshoter,drivers,drivers/utils,image_preprocessor):Integration suite, cuprite + vips, with CI's
JRUBY_OPTS:MRI 4.0.6 control, same machine, same suite, same driver:
Identical. No JRuby-specific breakage in the vips path — no FFI pointer trouble, no
Vips::Imagefinalization trouble, no threading trouble.One unrelated thing found
Without
JRUBY_OPTS="--dev -J-Djruby.thread.pool.enabled=true", the integration suite completes (Ruby.tearDownis reached, so minitest has already reported) but then hangs forever joining a leftoverpuma reactorthread and aferrum/utils/thread.rbthread. With CI'sJRUBY_OPTSit exits cleanly. Not caused by vips, and CI already sets those options — noting it so nobody rediscovers it as a vips bug.Scope
2.0-shaped: additive, no chunky_png removal (that is 2.1's job), no version bump, no CHANGELOG edit.
standardrbclean.full-cilabel applied so the matrix job actually runs on this PR.Summary by Sourcery
Run the vips driver through the JRuby integration suite and adjust its CI time budget so JRuby support is continuously validated.
New Features:
Enhancements:
CI:
Tests:
Summary by CodeRabbit