Skip to content

Conversation

tillsc
Copy link
Contributor

@tillsc tillsc commented Jul 17, 2025

This PR fixes the previously broken integration test setup and ensures it now runs successfully with JRuby 9.4 and JRuby 10.

  • Integration tests now pass for JRuby 9.4 and 10
  • Migrated test app to Rails 7.1
  • Updated and cleaned up Ruby-related tooling:
    • Added missing Gemfile, bundle install, and rakefile fixes
    • Pinned bundler version in Mavenfile to avoid install drift
  • Maven project structure improvements:
    • Switched <packaging> to pom to avoid generating unnecessary .jar/.war artifacts
    • Adjusted for manual target/ directory handling before warble build
  • Upgraded Jetty:
    • Replaced Jetty 6.1 (Servlet API 2.5) with Jetty 8.1 (Servlet API 3.0)
    • Jetty 8.1 is the last version that allows background startup using <daemon>true</deamon>
    • Newer Jetty Maven plugins from Eclipse (versions 9.x–11.x) do not support background execution and are not suitable for this use case

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Looks reasonable, I'll start the tests!

@headius
Copy link
Member

headius commented Jul 18, 2025

Woah nice!

@mjansing
Copy link
Contributor

mjansing commented Jul 18, 2025

I don't know what's happening on CI.

The test using Java 21 with JRuby 10 (https://github.com/jruby/warbler/actions/runs/16350097899/job/46216486292?pr=567) passes without issues on my local machine.

However, the other integration test fails with Java 8 and JRuby 9.4 on my development machine as well. The error appears to be caused by the ScriptingContainer.class being compiled with Java 21 (class file version 65), which is incompatible with Java 8 (max class file version 52). So the issue likely stems from using a JRuby JAR (jruby-core-*.jar) that was built with a newer Java version than the runtime used during test execution. I assume that every new version of JRuby is built with something newer than Java 8, so how to fix this?

@tillsc tillsc force-pushed the update_test_app branch from 6015f8e to b779ade Compare July 22, 2025 07:16
@tillsc
Copy link
Contributor Author

tillsc commented Jul 22, 2025

Removed gem 'bundler', '2.4.22' from the Mavenfile again. After adding it, regular specs started failing under JRuby 10 — even though all other changes were limited to the /integration directory. So the most likely cause was this bundler override.

@olleolleolle
Copy link
Member

The (21, jruby-9.4, integration) AND the (8, jruby-9.4, integration) fail on: "ERROR: Error installing /home/runner/.m2/repository/rubygems/bundler/2.7.1/bundler-2.7.1.gem: bundler-2.7.1 requires Ruby version >= 3.2.0. The current ruby version is 3.1.4."

@tillsc
Copy link
Contributor Author

tillsc commented Jul 22, 2025

I've now tried adding Gemfile.lock files to each existing Gemfile. I'm not entirely sure this will work, but since the output shows that virtus is being installed — and that gem isn't referenced anywhere in the integration subdirectory — it seems like the issue must be caused somewhere outside the integration tests.

@tillsc
Copy link
Contributor Author

tillsc commented Jul 23, 2025

That didn’t work either. The only thing that seems to have any effect is adding gem 'bundler', '${bundler.version}' to the Mavenfile. Without it, some Maven plugin apparently tries to install the latest Bundler version during the JRuby 9.4 integration tests — and fails immediately. For some reason, I haven’t been able to reproduce this behavior locally.

However, adding gem 'bundler', '${bundler.version}' breaks one of the regular spec tests for JRuby 10 that previously passed — and I have no idea why.

Also, as @mjansing already pointed out, the JRuby 9.4 integration tests still fail when run with Java 8.

So unfortunately, I can't get all tests to pass at the same time — sorry!

@mjansing
Copy link
Contributor

mjansing commented Jul 23, 2025

I was able to reproduce it locally (Java 8, JRuby 9.4), but for some reason Maven invoked something (I guess gem-maven-plugin via gemspec( :jar => 'warbler_jar.jar', :source => 'ext' )??) that tries to install bundler 2.7.1 in any case, which fails due to compatibility issues.

This is really annoying. Older builds (e.g., this one) were green simply because Bundler 2.7 wasn't released when the action ran.

@headius
Copy link
Member

headius commented Jul 23, 2025

I'm out of the office quite a bit this week but once I'm back at work I'll see if I can help iron out these remaining issues.

tillsc added 8 commits July 24, 2025 10:01
This prevents Maven from producing unused JAR or WAR files during the build.
However, we now need to check in the target directories manually, as they are no longer created automatically before warbling.
The old Jetty 6.1 plugin used an outdated Servlet API, which caused compatibility issues with modern WAR files.
Jetty 8.1 includes a more recent API version and still allows background startup via the legacy Mortbay plugin.

Newer Jetty Maven plugins maintained by Eclipse (e.g. jetty-maven-plugin 9.x–11.x) no longer support background (daemon) mode,
which makes them unsuitable for integration tests in this setup.
@tillsc tillsc force-pushed the update_test_app branch from 606476c to fb06736 Compare July 24, 2025 08:08
@tillsc
Copy link
Contributor Author

tillsc commented Jul 24, 2025

All tests are passing except for the Java 8 one. I haven’t yet figured out what’s triggering the compilation or download of a class that isn’t Java 8 compatible.

tillsc added 4 commits July 24, 2025 14:20
Since the `gem-maven-plugin` attempts to install the latest Bundler version by default, we need to explicitly freeze it for compatibility with older ruby versions
@tillsc tillsc force-pushed the update_test_app branch from fb06736 to 4087ae0 Compare July 24, 2025 12:34
@tillsc
Copy link
Contributor Author

tillsc commented Jul 24, 2025

Sorry for the repeated force pushes — I just didn’t want to mess up the Git history.
I was on the wrong track initially: committing the Gemfile.lock files fixed some surface issues but not the actual problem.
So I started over and explicitly pinned jruby-jars and bundler versions in both the Mavenfile and integration/pom.xml.

As of now, the integration tests are always run with Bundler 2.6.3 and JRuby 9.4, even if JRuby 10 is used externally.
That’s intentional for the time being and can easily be changed later — the versions are controlled via Maven properties, so it’s just a matter of configuration.

Since the integration tests don’t have any Gemfile.lock files, this setup should be fine for now.

@tillsc
Copy link
Contributor Author

tillsc commented Jul 24, 2025

I added logic to ensure that the currently active JRuby and Bundler versions are also used for the dependencies in the integration tests. That’s good enough for now from my side.

@headius had removed support for running tests with different Bundler versions in the GitHub Actions configuration — I’ve restored that and added an example entry using Bundler 2.7.1 with JRuby 10.

Since we’ve experienced issues in some of our projects when using Bundler versions other than 2.6.3, this setup might help us reproduce those problems during testing. For now, though, everything is green.

Copy link
Member

@kares kares left a comment

Choose a reason for hiding this comment

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

💟 awesome work!

@mjansing
Copy link
Contributor

Is there anything blocking this merge so we can have a working infrastructure? It would really help to proceed with further work.

@olleolleolle olleolleolle merged commit ede2e17 into jruby:master Aug 25, 2025
8 checks passed
@olleolleolle
Copy link
Member

I read the change again, and refreshed my memory and merged this change.
Thanks, @mjansing, for the reminder.

@mjansing
Copy link
Contributor

Thank you. 🎉

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.

5 participants