Make JRuby 10 a required CI target#573
Conversation
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>
|
If we do this should we revert the change that made failing runs flag green but annotate problems? |
|
@faisal Good question. I don't think we want to revert it entirely, since 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 |
|
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). |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Also does this mean we are promoting jruby to "fully supported?" |
There was a problem hiding this comment.
Pull request overview
This PR makes JRuby 10 a required (non-experimental) CI target for RubyCritic and addresses a JRuby-only test-suite load-order failure by eagerly loading Reek’s configuration schema early in the test boot process.
Changes:
- Promote
jruby-10.0from an experimental to a required CI matrix entry across the main test/lint jobs. - Add a JRuby-only eager load in
test_helper.rbto prevent adry-schemaMacrosnamespace load failure under specific require ordering. - Document JRuby 10 support in the README and record the change in the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
test/test_helper.rb |
Eager-load Reek on JRuby to stabilize gem loading/namespace autoload behavior in the test environment. |
README.md |
Document JRuby 10 compatibility and that it’s covered by CI. |
CHANGELOG.md |
Note the CI policy change and accompanying JRuby test-suite fix. |
.github/workflows/main.yml |
Add jruby-10.0 to the required matrix for Specs, Rubocop, Reek, and Minitest jobs (keeping ruby-head experimental). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JRuby 10.1 targets Ruby 4.0 compatibility (the project's current MRI top target) and is the latest stable JRuby line. Add it across all four jobs as continue-on-error, mirroring ruby-head, so it can be promoted to the required matrix once it's confirmed green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jruby-10.1 has been green across all four jobs as an experimental target, so move it into the required matrix alongside jruby-10.0. Update the README compatibility section and CHANGELOG to note support for both JRuby 10.0 (Ruby 3.4 compat) and 10.1 (Ruby 4.0 compat). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@faisal Yes, I think so. |
What
RubyCritic already runs end-to-end on JRuby, but CI listed
jruby-10.0only as an experimentalcontinue-on-errortarget, 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_parserloaded beforereek, Ruby 3.4'sbundled_gemsrequire shim clobbered Zeitwerk's implicit-namespace autoload on JRuby, and reek'sdry-schemaMacrosnamespace failed to load:It only showed up under the full
rake testload 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 bydefined?(JRUBY_VERSION), before anyflog/ruby_parserrequire. 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: movejruby-10.0from the experimentalinclude:block into the required matrix across all four jobs (Specs, Rubocop, Reek, Minitest);ruby-headstays experimentalREADME.md: note JRuby 10 support in the Compatibility sectionVerification
Run locally on JRuby 10.0.4.0 (Ruby 3.4.5):
rake testrake specrubocoprake reekmdlNo MRI regression.
🤖 Generated with Claude Code