Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #10594 (closed defect: fixed)

Opened 4 months ago

Last modified 4 months ago

[PATCH][TINY] (Ruby 1.9 compat) fix raise call and constant name resolution in Duration

Reported by: fxn Assigned to: bitsweat
Priority: low Milestone: 2.x
Component: ActiveSupport Version: edge
Severity: minor Keywords:
Cc:

Description

Since Duration is a BasicObject it does not inherit raise, and does not see the ArgumentError constant.

Attachments

fix_raise_call_and_constant_in_duration_for_ruby19.diff (1.3 kB) - added by fxn on 12/22/07 19:23:25.
fix_raise_call_and_constant_in_duration_for_ruby19_take2.diff (1.3 kB) - added by fxn on 12/28/07 22:42:18.
fix_constant_in_duration_and_add_raise_to_as_basic_object.diff (1.9 kB) - added by fxn on 12/30/07 01:05:04.

Change History

12/22/07 18:04:42 changed by bitsweat

Needs to a test case demonstrating it on 1.9.

12/22/07 18:56:55 changed by fxn

Sure, there's an interesting gotcha for assert_raise here to show it does not work in 1.9.

It turns out that dependencies.rb is triggered and raises, well, ArgumentError :-). So the obvious test does not depict the problem. I'll figure out a way to make the test fail with the original code, and pass with the fix.

12/22/07 19:16:56 changed by fxn

OK, I've finally resorted to test the message. It is a little unusual so I've explained the intention in the assertion message. The patch has been updated.

12/22/07 19:23:25 changed by fxn

  • attachment fix_raise_call_and_constant_in_duration_for_ruby19.diff added.

12/28/07 16:54:03 changed by chuyeow

  • owner changed from core to bitsweat.

Re-assigning to bitsweat (he said to assign 1.9 compatibility-related tickets to him).

As I mentioned on #rails-contrib, if you could swap the order of the 1st 2 arguments to assert_equal here:

assert_equal e.message, 'expected a time or date, got ""', "ensure ArgumentError is not being raised by dependencies.rb" 

so that the failure message is not confusing, that'd be great.

12/28/07 22:42:18 changed by fxn

  • attachment fix_raise_call_and_constant_in_duration_for_ruby19_take2.diff added.

12/28/07 22:43:40 changed by fxn

Absolutely, a new patch has been attached.

12/29/07 02:33:14 changed by chuyeow

+1 Thanks!

12/29/07 04:57:29 changed by bitsweat

How about having Duration inherit ActiveSupport::BasicObject instead, and adding support for raise?

12/29/07 05:04:22 changed by bitsweat

Hmm, I see there is some controversy regarding AS::BasicObject, actually ;)

12/30/07 00:47:51 changed by fxn

In fact it does already inherit from AS::BasicObject because the bare constant BasicObject is evaluated inside module ActiveSupport:

  $ ~/ruby-1.9.0/bin/irb
  irb(main):001:0> RUBY_VERSION
  => "1.9.0"
  irb(main):002:0> require "./active_support.rb"
  => true
  irb(main):003:0> ActiveSupport::Duration.superclass
  => ActiveSupport::BasicObject

I have not enough perspective to have an opinion about adding raise in AS::BasicObject, but sounds fine in the sense that it is OK for me not to be able to puts from a BasicObject, but being able to raise an exception is another issue. I'll write a patch in a minute.

12/30/07 01:05:04 changed by fxn

  • attachment fix_constant_in_duration_and_add_raise_to_as_basic_object.diff added.

12/30/07 01:05:30 changed by fxn

Done!

01/02/08 09:08:18 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.

(In [8523]) Ruby 1.9 compat: add #raise to AS::BasicObject, fixup Duration argument error. Closes #10594.