Merge pull request #418 from chadlwilson/minor-tidies

[chore] impove build/test reliabiliy across jruby versions

Author: chadlwilson

.github/workflows/maven.yml

@@ -9,7 +9,7 @@ on:
 env:
   # Default versions for canonical release build
   DEFAULT_JAVA_VERSION: '8'
-  DEFAULT_JRUBY_VERSION: '9.4.14.0' # Should match pom.xml <jruby.version> property (AND a version inside the test matrix)
+  DEFAULT_JRUBY_VERSION: '9.4.14.0' # Should match pom.xml <jruby.compat.version> property (AND a version inside the test matrix)
   DEFAULT_RACK_VERSION: '~> 2.2.0' # Should match Gemfile (AND a version inside the test matrix)
 
 jobs:
@@ -42,7 +42,7 @@ jobs:
         cache: maven
 
     - name: Build with Maven
-      run: ./mvnw -B install -Djruby.version=${{ matrix.jruby_version }}
+      run: ./mvnw -ntp install -Dstyle.color=always -Djruby.test.version=${{ matrix.jruby_version }}
       env:
         RACK_VERSION: ${{ matrix.rack_version }}
 
@@ -79,7 +79,6 @@ jobs:
 
     env:
       BUNDLE_GEMFILE: gemfiles/${{ matrix.appraisal }}.gemfile
-      JRUBY_VERSION: ${{ matrix.jruby_version }}
 
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

Rakefile

@@ -38,14 +38,14 @@ directory 'target/classes'
 
 desc "Compile classes"
 task(:compile => 'target/classes') do
-  sh "./mvnw compile #{ENV['JRUBY_VERSION'] ? "-Djruby.version=#{ENV['JRUBY_VERSION']}" : ""}"
+  sh "./mvnw -ntp -Dstyle.color=always compile"
 end
 
 directory 'target/test-classes'
 
 desc "Compile test classes"
 task(:test_prepare => ['target/classes', 'target/test-classes']) do
-  sh "./mvnw test-compile #{ENV['JRUBY_VERSION'] ? "-Djruby.version=#{ENV['JRUBY_VERSION']}" : ""}"
+  sh "./mvnw -ntp -Dstyle.color=always test-compile"
 end
 
 desc "Unpack the rack gem"
@@ -88,7 +88,7 @@ task :test_resources => ["target/test-classes"]
 namespace :resources do
   desc "Copy (and generate) resources"
   task :copy => :resources do
-    sh './mvnw process-resources -Dmdep.skip=true'
+    sh './mvnw -ntp process-resources -Dstyle.color=always -Dmdep.skip=true'
   end
   desc "Generate test resources"
   task :test => :test_resources
@@ -198,8 +198,7 @@ task :release_checks do
       "  git push origin :#{GEM_VERSION}" if ok
   end
 
-  pom_version = `./mvnw help:evaluate -Dexpression=project.version`.
-    split("\n").reject { |line| line =~ /[INFO]/ }.first.chomp
+  pom_version = `./mvnw help:evaluate -q --non-recursive -DforceStdout -Dexpression=project.version`
   if pom_version =~ /dev|SNAPSHOT/
     fail "Can't release a dev/snapshot version.\n" +
       "Please update pom.xml to the final release version, run `mvn install', and commit the result."
@@ -219,7 +218,7 @@ task :release => [:release_checks, :clean] do
   args = ''
   args << "-Dgpg.keyname=#{ENV['GPG_KEYNAME']} " if ENV['GPG_KEYNAME']
 
-  sh "./mvnw -Prelease #{args} -DupdateReleaseInfo=true clean deploy"
+  sh "./mvnw -ntp -Prelease #{args} -Dstyle.color=always -DupdateReleaseInfo=true clean deploy"
 
   sh "git tag #{GEM_VERSION}"
 

pom.xml

@@ -21,7 +21,11 @@
   <properties>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 
-    <jruby.version>9.4.14.0</jruby.version>
+    <!-- Version to target at compile time -->
+    <jruby.compat.version>9.4.14.0</jruby.compat.version>
+    <!-- Version to execute tests with -->
+    <jruby.test.version>${jruby.compat.version}</jruby.test.version>
+
     <jruby.maven.plugins.version>3.0.6</jruby.maven.plugins.version>
     <gem.home>${project.build.directory}/rubygems</gem.home>
     <slf4j.version>2.0.17</slf4j.version>
@@ -76,8 +80,10 @@
   <dependencies>
     <dependency>
       <groupId>org.jruby</groupId>
-      <artifactId>jruby</artifactId>
-      <version>${jruby.version}</version>
+      <!-- This has to be jruby-complete, as bundler-maven-plugin has special logic to exclude jruby-complete - otherwise
+           it conflicts with the bootclasspath if using different "target compile version" to "runtime version" -->
+      <artifactId>jruby-complete</artifactId>
+      <version>${jruby.compat.version}</version>
       <scope>provided</scope>
     </dependency>
     <dependency>
@@ -241,7 +247,7 @@
             <phase>test</phase>
             <goals><goal>install</goal></goals>
             <configuration>
-              <jrubyVersion>${jruby.version}</jrubyVersion>
+              <jrubyVersion>${jruby.test.version}</jrubyVersion>
               <local>false</local>
               <quiet>false</quiet>
             </configuration>
@@ -253,7 +259,7 @@
         <artifactId>rake-maven-plugin</artifactId>
         <version>${jruby.maven.plugins.version}</version>
         <configuration>
-          <jrubyVersion>${jruby.version}</jrubyVersion>
+          <jrubyVersion>${jruby.test.version}</jrubyVersion>
         </configuration>
         <executions>
           <execution>

src/main/java/org/jruby/rack/DefaultRackApplicationFactory.java

@@ -439,7 +439,7 @@ private void captureMessage(final RaiseException re) {
                 rubyException.callMethod(context, "capture");
                 rubyException.callMethod(context, "store");
             }
-            catch (Exception e) {
+            catch (Throwable e) {
                 rackContext.log(INFO, "failed to capture exception message", e);
                 // won't be able to capture anything
             }

src/spec/ruby/rack/application_spec.rb

@@ -476,6 +476,40 @@ def reset_config
         expect(e.message).to eql 'something went wrong'
       end
     end
+
+    it "swallows and logs errors during exception detail capturing" do
+      expect(@rack_config).to receive(:getRackup).and_return("raise 'something went wrong'")
+      expect_any_instance_of(Exception).to receive(:capture).and_raise java.lang.NoClassDefFoundError.new("missing class during exception capture")
+
+      app_factory = mocked_runtime_application_factory
+      app_factory.init @rack_context
+      app_object = app_factory.newApplication
+
+      raise_info_logged = 0
+      raise_error_logged = 0
+      allow(@rack_context).to receive(:log) do |level, msg, e|
+        if level.to_s == 'INFO'
+          expect(msg).to eql 'failed to capture exception message'
+          expect(e).to be_a java.lang.NoClassDefFoundError
+          raise_info_logged += 1
+        elsif level.to_s == 'ERROR'
+          expect(msg).to eql 'unable to initialize application'
+          expect(e).to be_a org.jruby.exceptions.RaiseException
+          raise_error_logged += 1
+        else
+          true
+        end
+      end
+
+      begin
+        app_object.init
+        fail "expected to raise"
+      rescue => e
+        expect(e.message).to eql 'something went wrong'
+      end
+
+      expect(raise_info_logged).to eql 1 # logs info message for exception capture
+    end
   end
 
   describe "getApplication" do

src/spec/ruby/rack/capture_spec.rb

@@ -13,15 +13,26 @@
   before :each do
     JRuby::Rack.context = nil
     $servlet_context = @servlet_context
-    allow(@servlet_context).to receive(:init_parameter_names).and_return []
+    allow(@servlet_context).to receive(:config).and_return Java::OrgJrubyRackEmbed::Config.new
   end
 
   it "captures environment information" do
     expect(@servlet_context).to receive(:log)
-    error = StandardError.new
+    error = StandardError.new "simulated rack start-up failed"
     error.capture
     error.store
     expect(error.output).to be_a StringIO
+    expect(error.output.string).to include "An exception happened during JRuby-Rack startup"
+    expect(error.output.string).to include "simulated rack start-up failed"
+    expect(error.output.string).to include "--- System"
+    expect(error.output.string).to include "jruby #{JRUBY_VERSION}"
+    expect(error.output.string).to include "--- Context Init Parameters:"
+    expect(error.output.string).to include "--- RubyGems"
+    expect(error.output.string).to include "Gem.path:"
+    expect(error.output.string).to include "--- Bundler"
+    expect(error.output.string).to include "Gemfile:"
+    expect(error.output.string).to include "--- JRuby-Rack Config"
+    expect(error.output.string).to include "logger_class_name"
   end
 
   it "captures exception backtrace" do
@@ -34,4 +45,5 @@
     end
   end
 
+
 end

src/spec/ruby/rack/config_spec.rb

@@ -1,6 +1,6 @@
 require File.expand_path('spec_helper', File.dirname(__FILE__) + '/..')
 
-describe org.jruby.rack.DefaultRackConfig do
+describe 'org.jruby.rack.DefaultRackConfig' do
 
   let(:config) do
     config = org.jruby.rack.DefaultRackConfig.new
@@ -23,7 +23,7 @@
 
 end
 
-describe org.jruby.rack.servlet.ServletRackConfig do
+describe 'org.jruby.rack.servlet.ServletRackConfig' do
 
   let(:config) do
     config = org.jruby.rack.servlet.ServletRackConfig.new(@servlet_context)

src/spec/ruby/spec_helper.rb

@@ -22,26 +22,21 @@
 
 module SharedHelpers
 
-  java_import 'org.jruby.rack.RackContext'
-  java_import 'org.jruby.rack.RackConfig'
-  java_import 'org.jruby.rack.servlet.ServletRackContext'
-  java_import 'javax.servlet.ServletContext'
-  java_import 'javax.servlet.ServletConfig'
-
   def mock_servlet_context
-    @servlet_context = ServletContext.impl {}
-    @rack_config ||= RackConfig.impl {}
-    @rack_context ||= ServletRackContext.impl {}
+    @servlet_context = Java::JavaxServlet::ServletContext.impl {}
+    @rack_config ||= Java::OrgJrubyRack::RackConfig.impl {}
+    @rack_context ||= Java::OrgJrubyRackServlet::ServletRackContext.impl {}
     [@rack_context, @servlet_context].each do |context|
       allow(context).to receive(:log)
       allow(context).to receive(:isEnabled).and_return nil
       allow(context).to receive(:getInitParameter).and_return nil
       allow(context).to receive(:getRealPath).and_return "/"
       allow(context).to receive(:getResource).and_return nil
       allow(context).to receive(:getContextPath).and_return "/"
+      allow(context).to receive(:init_parameter_names).and_return []
     end
     allow(@rack_context).to receive(:getConfig).and_return @rack_config
-    @servlet_config ||= ServletConfig.impl {}
+    @servlet_config ||= Java::JavaxServlet::ServletConfig.impl {}
     allow(@servlet_config).to receive(:getServletName).and_return "a Servlet"
     allow(@servlet_config).to receive(:getServletContext).and_return @servlet_context
     @servlet_context
@@ -151,8 +146,8 @@ def should_eval_as_not_nil(code)
 
   config.backtrace_exclusion_patterns = [
     /bin\//,
-    #/gems/,
-    /spec\/spec_helper\.rb/,
+    # /gems/,
+    # /spec\/spec_helper\.rb/,
   ]
 
 end