TimeWithZone should not accept unrecognized time zones #927

Closed
lighthouse-import opened this Issue May 16, 2011 · 5 comments

Comments

Projects
None yet
1 participant

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/6573
Created by Marc-André Lafortune - 2011-03-14 14:53:36 UTC

Currently, the following doesn't raise any error:

t = Time.now.in_time_zone("There is no such timezone"); t.utc

Many other methods of TimeWithZone will raise a NoMethodError as soon as they try to access the timezone (e.g. t.inspect).

It should not be possible to create a TimeWithZone with an invalid timezone.

Imported from Lighthouse.
Comment by Josh Kalderimis - 2011-03-15 11:05:49 UTC

Hi Marc-André,

I agree with this. Are you able to whip up a patch with tests?

Thanks,

Josh

Imported from Lighthouse.
Comment by Marc-André Lafortune - 2011-03-15 13:41:34 UTC

Sure, will do.

Imported from Lighthouse.
Comment by Marc-André Lafortune - 2011-03-16 19:20:38 UTC

Here is my patch.

{Date}Time#in_time_zone, Time.use_zone and Time.zone= now raise an ArgumentError on invalid timezone arguments.

Note that I changed test_time_zone_setter_with_invalid_zone which used to imply that Time.zone = "foo" was a no-op. There was no test for in_time_zone nor for use_zone.

Thanks

Imported from Lighthouse.
Comment by Josh Kalderimis - 2011-03-16 19:52:14 UTC

Hey Marc,

I think the patch and tests look good.

+1 from me, I prefer having ActiveSupport raise an error if the timezone is incorrect.

Santiago, what do you think?

Josh

Attachments saved to Gist: http://gist.github.com/971815

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