Skip to content

Make JRuby 10 a required CI target#573

Open
etagwerker wants to merge 1 commit into
mainfrom
jruby-compatibility
Open

Make JRuby 10 a required CI target#573
etagwerker wants to merge 1 commit into
mainfrom
jruby-compatibility

Conversation

@etagwerker

Copy link
Copy Markdown
Collaborator

What

RubyCritic already runs end-to-end on JRuby, but CI listed jruby-10.0 only as an experimental continue-on-error target, so it never actually had to pass. This PR makes JRuby 10 a required CI target and fixes the one thing that was keeping the suite from going green.

The bug

The application was fine. The test suite was the blocker. When flog/ruby_parser loaded before reek, Ruby 3.4's bundled_gems require shim clobbered Zeitwerk's implicit-namespace autoload on JRuby, and reek's dry-schema Macros namespace failed to load:

LoadError: cannot load such file -- .../dry-schema-1.16.0/lib/dry/schema/macros

It only showed up under the full rake test load order (a single test file passed on its own), which is why it looked flaky at first.

The fix

Force reek's schema to load up front in test_helper.rb, guarded by defined?(JRUBY_VERSION), before any flog/ruby_parser require. The application itself is untouched because it already loads reek before flog.

  • test/test_helper.rb: eager reek schema load on JRuby, with a comment explaining the ordering issue
  • .github/workflows/main.yml: move jruby-10.0 from the experimental include: block into the required matrix across all four jobs (Specs, Rubocop, Reek, Minitest); ruby-head stays experimental
  • README.md: note JRuby 10 support in the Compatibility section

Verification

Run locally on JRuby 10.0.4.0 (Ruby 3.4.5):

Check JRuby 10 MRI 4.0
rake test 164 runs, 0 failures 164 runs, 0 failures
rake spec 14 runs, 0 failures n/a
rubocop 0 offenses n/a
rake reek 0 warnings n/a
mdl clean clean

No MRI regression.

🤖 Generated with Claude Code

RubyCritic already ran end-to-end on JRuby, but its CI listed jruby-10.0
only as an experimental, continue-on-error target so it never had to pass.

The test suite was the blocker: when flog/ruby_parser loaded before reek,
Ruby 3.4's bundled_gems require shim clobbered Zeitwerk's implicit-namespace
autoload on JRuby, so reek's dry-schema `Macros` namespace failed to load.
Forcing reek's schema to load up front in test_helper (guarded by
defined?(JRUBY_VERSION)) sidesteps the ordering issue. The application is
unaffected because it loads reek before flog.

- test_helper: eager reek schema load on JRuby
- main.yml: promote jruby-10.0 from experimental include into the required
  matrix across all four jobs; ruby-head stays experimental
- README: note JRuby 10 support in the Compatibility section

Verified on JRuby 10: test (164), spec (14), rubocop (0 offenses), reek
(0 warnings), mdl; MRI 4.0 test (164) shows no regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@etagwerker etagwerker marked this pull request as ready for review June 9, 2026 16:58
@faisal

faisal commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

If we do this should we revert the change that made failing runs flag green but annotate problems?

@etagwerker

Copy link
Copy Markdown
Collaborator Author

@faisal Good question. I don't think we want to revert it entirely, since ruby-head is still experimental after this PR and relies on that same continue-on-error: ${{ matrix.experimental }} to stay non-blocking. ruby-head is a genuinely moving target, so I'd rather it annotate failures than block merges.

What this PR does is narrow that "green but annotated" behavior down to just ruby-head by graduating jruby-10.0 out of the experimental set into the required matrix. So the mechanism stays, but it only applies where we actually want it.

Were you thinking of something more specific that we'd want to clean up, like dropping the experimental include block altogether, or making ruby-head failures more visible than they are today? Happy to fold that in if so.

@faisal

faisal commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Before answering I reviewed the history of the changes here, and came to the conclusion that this PR is likely the right approach (so long as we're willing and able to keep things running on jruby).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants