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.

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

Avoid using block_given in the presence of aliases #45

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

headius
Copy link
Contributor

@headius headius commented Mar 29, 2022

In #41 ostruct.rb was modified to not define block_given! on JRuby because it would not be recognized as being the same as block_given? and would not work properly (and we would also warn about this situation).

Unfortunately, each_pair still expects block_given! to be defined:

https://github.com/ruby/ostruct/blob/master/lib/ostruct.rb#L200

This leads to two failures running on JRuby:

Failure: test_each_pair(TC_OpenStruct):
  <#<OpenStruct name="John Smith", age=70, pension=300>>
  with id <4020> was expected to be equal? to
  <#<Enumerator: #<OpenStruct name="John Smith", age=70, pension=300>:each_pair>>
  with id <4024>.
/Users/headius/projects/ostruct/test/ostruct/test_ostruct.rb:167:in `test_each_pair'
     164:   def test_each_pair
     165:     h = {name: "John Smith", age: 70, pension: 300}
     166:     os = OpenStruct.new(h)
  => 167:     assert_same os, os.each_pair{ }
     168:     assert_equal '#<Enumerator: #<OpenStruct name="John Smith", age=70, pension=300>:each_pair>', os.each_pair.inspect
     169:     assert_equal [[:name, "John Smith"], [:age, 70], [:pension, 300]], os.each_pair.to_a
     170:     assert_equal 3, os.each_pair.size

Failure: test_initialize(TC_OpenStruct)
/Users/headius/projects/ostruct/test/ostruct/test_ostruct.rb:10:in `test_initialize'
      7:   def test_initialize
      8:     h = {name: "John Smith", age: 70, pension: 300}
      9:     assert_equal h, OpenStruct.new(h).to_h
  => 10:     assert_equal h, OpenStruct.new(OpenStruct.new(h)).to_h
     11:     assert_equal h, OpenStruct.new(Struct.new(*h.keys).new(*h.values)).to_h
     12:   end
     13: 

This can be fixed by either using the un-aliased block_given? on JRuby, or just using defined?(yield) which does not require block_given? to be available. I have done the latter here.

defined?(yield) bypasses the block_given? method (or any aliases
to it) and always does the right thing.
headius added a commit to headius/jruby that referenced this pull request Mar 29, 2022
The two existing failures were resolved:

* test_method_missing depends on the native method_missing not
  appearing in NoMethodError backtrace.
* test_to_h_with_block was likely fixed by updating ostruct.

Two new failures are due to ruby/ostruct#41 and will be fixed by
ruby/ostruct#45: test_each_pair, test_initialize
headius added a commit to headius/jruby that referenced this pull request Mar 29, 2022
The two existing failures were resolved:

* test_method_missing depends on the native method_missing not
  appearing in NoMethodError backtrace.
* test_to_h_with_block was likely fixed by updating ostruct.

Two new failures are due to ruby/ostruct#41 and will be fixed by
ruby/ostruct#45: test_each_pair, test_initialize
@headius
Copy link
Contributor Author

headius commented Mar 31, 2022

@marcandre This issue is causing ostruct each_pair to be broken on JRuby, can we get a merge and release soon?

@marcandre marcandre merged commit 4c38fe6 into ruby:master Mar 31, 2022
@marcandre
Copy link
Member

Thanks for the reminder! Released as 0.5.5, thanks :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants