aetherknight / recursive-open-struct

OpenStruct subclass that returns nested hash attributes as RecursiveOpenStructs
Other
276 stars 54 forks source link

rspec fails on ruby-head #38

Closed Kris-LIBIS closed 8 years ago

Kris-LIBIS commented 8 years ago

Currently most of the tests fail on ruby-head. I'm currently not sure why, but I noticed this as my own gem started to fail on ruby-head recently.

What I observed is that it seems the recursive aspect is no longer working. There is nothing in the ruby changelog that I can pinpoint to be causing this, but I may be overlooking something.

If time permits, I'll investigate further myself but I'm not sure if that is going to happen any time soon.

Kris-LIBIS commented 8 years ago

Apparently ruby 2.2.3 has significantly refactored ostruct, in such a way that it breaks the current reverse-open-struct accessor method definitions. At least one factor that plays a role is the defintiion of respond_to_missing? in ostruct, causing the test "unless self.respond_to?(name)" to always return true. Changing the test into "unless self.methods.include?(name.to_sym)" improves things, but still several tests fail.

Kris-LIBIS commented 8 years ago

The remaining errors, seem to be related to the fact that initialize_copy no longer calls new_ostruct_member any more. I'll prepare a pull request.

Kris-LIBIS commented 8 years ago

Finally traced the issue down to the following commit: https://github.com/ruby/ruby/commit/3bf9b2f0473550caa73468908ac3e18e0f431b85. It's not part of the released 2.2.3, but as it is a commit of oct 15, 2015 it is part of 2.2.3dev, which is what Travis currently uses for ruby-head.

Kris-LIBIS commented 8 years ago

The issue with the initialize_copy has a couple of consequences:

We can go different ways here:

  1. call the new_ostruct_member for each key in the table in intialize_copy.
  2. patch the ROS to implement lazy creation of attributes, as implemented in the latest OpenStruct and let delete_field call remove_method, but catch and ignore the exception.

The first is the easiest one to implement, but generates overhead for ruby < 2.2.3dev. The thread in pull request 1033 indicates that there can be a significant performance gain by using lazy accessor creation, so I opted for the second solution.

Kris-LIBIS commented 8 years ago

@aetherknight thanks for the merge and release. I noticed my travis tests started passing on ruby-head this weekend ;)