Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error using ActiveSupport::Testing::TestHelpers (7.1.1) on jruby-9.4.4.0 #7982

Closed
klobuczek opened this issue Oct 25, 2023 · 2 comments · Fixed by #7984
Closed

Error using ActiveSupport::Testing::TestHelpers (7.1.1) on jruby-9.4.4.0 #7982

klobuczek opened this issue Oct 25, 2023 · 2 comments · Fixed by #7984
Milestone
JRuby 9.4.5.0

Comments

@klobuczek
Copy link

Environment Information

Provide at least:

  • JRuby version 9.4.4.0
  • Operating system and platform (e.g. uname -a)
    Darwin anchorage.local 23.0.0 Darwin Kernel Version 23.0.0: Fri Sep 15 14:42:42 PDT 2023; root:xnu-10002.1.13~1/RELEASE_X86_64 x86_64

Other relevant info you may wish to add:

  • Installed or activated gems: acrivesupport 7.1.1

Consider the following script:

require 'active_support'
require 'active_support/testing/time_helpers'
require 'active_support/core_ext/string/conversions'

include ActiveSupport::Testing::TimeHelpers

travel_to(Time.new(2000)) { Time.current.utc.iso8601(9).to_time }

Expected Behavior

Result on ruby-3.1.3

=> 1999-12-31 23:00:00 +0000

Actual Behavior

Result on jruby-9.4.4.0:

/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:62: warning: method redefined; discarding old now
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:174: warning: previous definition of now was here
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:62: warning: method redefined; discarding old new
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:176: warning: previous definition of new was here
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:62: warning: method redefined; discarding old today
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:185: warning: previous definition of today was here
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:62: warning: method redefined; discarding old now
/Users/heinrich/.rvm/gems/jruby-9.4.3.0/gems/activesupport-7.1.1/lib/active_support/testing/time_helpers.rb:186: warning: previous definition of now was here
org.jruby.dist/org.jruby.RubyTime.initialize(RubyTime.java:1723): class org.jruby.RubyFixnum cannot be cast to class org.jruby.RubyHash (org.jruby.RubyFixnum and org.jruby.RubyHash are in module org.jruby.dist of loader 'app') (Java::JavaLang::ClassCastException)
	from org.jruby.dist/org.jruby.RubyTime$INVOKER$i$initialize.call(RubyTime$INVOKER$i$initialize.gen)
	from org.jruby.dist/org.jruby.internal.runtime.methods.JavaMethod$JavaMethodN.call(JavaMethod.java:847)
	from org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:90)
	from org.jruby.dist/org.jruby.RubyClass.newInstance(RubyClass.java:931)
	from org.jruby.dist/org.jruby.RubyClass$INVOKER$i$newInstance.call(RubyClass$INVOKER$i$newInstance.gen)
	from org.jruby.dist/org.jruby.internal.runtime.methods.AliasMethod.call(AliasMethod.java:133)
	from org.jruby.dist/org.jruby.RubyClass.finvokeWithRefinements(RubyClass.java:522)
	from org.jruby.dist/org.jruby.RubyBasicObject.send(RubyBasicObject.java:1703)
	from org.jruby.dist/org.jruby.RubyKernel.send(RubyKernel.java:2355)
	from org.jruby.dist/org.jruby.RubyKernel$INVOKER$s$send.call(RubyKernel$INVOKER$s$send.gen)
	from org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:446)
	from org.jruby.dist/org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:92)
	from org.jruby.dist/org.jruby.ir.instructions.CallBase.interpret(CallBase.java:561)
	from org.jruby.dist/org.jruby.ir.interpreter.InterpreterEngine.processCall(InterpreterEngine.java:367)
	from org.jruby.dist/org.jruby.ir.interpreter.StartupInterpreterEngine.interpret(StartupInterpreterEngine.java:66)
	from org.jruby.dist/org.jruby.ir.interpreter.Interpreter.INTERPRET_BLOCK(Interpreter.java:116)
	... 197 levels...

The error does not happen with activesupport-7.0.8
@headius
Copy link
Member

headius commented Oct 26, 2023

Thank you for the report!

I would say we are still working on Rails 7.1 support, but this is something broken in JRuby. I'll have a look.

@headius
Copy link
Member

headius commented Oct 26, 2023

Looks like it was me: 5565751#diff-a9084ae3fb1483bfff442818e0b133bfbb08c9a20393b3e432481013865321d8R1723

The exception is caused by a blind cast of the last argument passed into Time#initialize, under the assumption that if the thread-local keywords flag was set we can be sure it is a `Hash. For whatever reason, that assumption doesn't hold with this code from Rails:

https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/testing/time_helpers.rb#L176-L183

We will want to look into that failed expectation, but in this case the cast is not needed and can simply be dropped. I have a PR in progress.

headius added a commit to headius/jruby that referenced this issue Oct 26, 2023
@headius
Remove unnecessary cast of kwargs
74fa59e 
The intent here was to pass in what might be kwargs if the keyword
flag was set, indicating that kwargs were being passed in. This
assumption seems ok so I'm not sure why it doesn't hold. In any
case, the cast here is unnecessary and the IRubyObject form will
return null for non-RubyHashes.

The code that triggered this does deal directly with kwargs, but
as a kwrest, forwarding it through a block:

https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/testing/time_helpers.rb#L176-L183

This could indicate JRuby failing to pass along a keyword flag
when forwarding kwrest.

The additional changes here are:

* Fall back on non-kwargs behavior if last arg turns out not to be
  a hash.
* Remove a redundant reset of the `callInfo` that governs keyword-
  passing.

Fixes jruby#7982
@headius headius mentioned this issue Oct 26, 2023
Merged
@enebo enebo added this to the JRuby 9.4.5.0 milestone Nov 1, 2023
@kaylareopelle kaylareopelle mentioned this issue Nov 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.4.5.0
Development

Successfully merging a pull request may close this issue.

Remove unnecessary cast of kwargs headius/jruby
3 participants
@headius @enebo @klobuczek