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

Hash inspection not working as in MRI for sprintf %p #2161

Closed
headius opened this issue Nov 9, 2014 · 5 comments
Closed

Hash inspection not working as in MRI for sprintf %p #2161

headius opened this issue Nov 9, 2014 · 5 comments
Labels
core JRuby 9000
Milestone
JRuby 9.1.8.0

Comments

@headius
Copy link
Member

headius commented Nov 9, 2014

The test that's failing in mri/ruby/test_sprintf.rb:

  def test_hash
    options = {:capture=>/\d+/}
    assert_equal("with options {:capture=>/\\d+/}", sprintf("with options %p" % options))
  end

The failure:

ArgumentError: positional args mixed with named args
    org/jruby/RubyString.java:1221:in `%'
    /Users/headius/projects/jruby/test/mri/ruby/test_sprintf.rb:173:in `test_hash'

We do have code that raises this error around Sprintf.java:154 in Sprintf::Args#next, but I was not able to figure out the equivalent code in MRI (it doesn't seem to be there in the GETNEXTARG macro in sprintf.c nor in the logic for case 'p' in the main switch) nor does the test itself indicate where/when the change to sprintf was actually made.
@headius headius added the core label Nov 9, 2014
@headius headius added this to the JRuby 9.0.0.0 milestone Nov 9, 2014
headius added a commit that referenced this issue Nov 9, 2014
@headius
Exclude failing sprintf %p hash-inspection test. #2161.
b3d1367
@enebo enebo modified the milestone: JRuby 9.0.0.0 Jul 14, 2015
@uchagani
Copy link

This is still failing as of jruby 9.0.5.0 (2.2.3).

@denyago
Copy link

denyago commented Feb 3, 2017

This is still failing as of jruby 9.1.7.0 (2.3.1) 😿

@enebo enebo closed this as completed in 86a1058 Feb 10, 2017
@enebo enebo added this to the JRuby 9.1.8.0 milestone Feb 10, 2017
@enebo
Copy link
Member

enebo commented Feb 10, 2017

Added test to spec in e66f638 ... I think @eregon will still merge this back upstream?

@headius
Copy link
Member Author

headius commented Feb 13, 2017

@enebo That's a good question. @eregon: Do we need to take over upstreaming our ruby/spec changes?

@eregon
Copy link
Member

eregon commented Feb 14, 2017

@enebo @headius I can keep doing it, no worries.
It's convenient since I merge from all repos and then update them all.
I did the last merge about 2 weeks ago.
I might ask your help for tagging new failures if it's not trivial though :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core JRuby 9000
Projects
None yet
Milestone
JRuby 9.1.8.0
Development

No branches or pull requests
5 participants
@headius @enebo @eregon @denyago @uchagani