How can my boss take rails seriously with bugs like this?

The Time::next_week method is supposed to give the time of the start of
the next week. But look at this, it cocks up :

t=Time.parse “Monday October 16th 2006”
=> Mon Oct 16 00:00:00 BST 2006

t.next_week
=> Mon Oct 23 00:00:00 BST 2006

t.next_week.next_week
=> Tue Oct 24 00:00:00 BST 2006

t.next_week.next_week.next_week
=> Mon Oct 30 00:00:00 GMT 2006

Arrhhhgg!! Its just f*cked up my application data!

next_week is not advertised to be exact. It’s an approximation. That
said, this looks like a really nasty bug. Did you pass all your
tests? Your title implies that your boss is evaluating Rails based on
the kind/number of bugs present. What’s the alternative? Count
unfixed bugs in .Net or any other framework? At least this is one you
might be able to fix – it’s open source.

Chris R. wrote:

Arrhhhgg!! Its just f*cked up my application data!

I may be misunderstanding you, but judging by your reaction, it sounds
like you:

a. applied a change to a non-trivial number of records of live
production data
b. without testing the change first, and
c. without backing up the database beforehand?

If so, uh … sorry to hear it?

Yes, that actually does appear to be a bug in the next_week method (I
just verified it here). But still …

Chris R. wrote the following on 24.10.2006 18:04 :

=> Mon Oct 23 00:00:00 BST 2006
Arrhhhgg!! Its just f*cked up my application data!

This is most probably a timezone related bug and not easily solved IMHO.
It seems to me that as in UTC your original date is in fact in the
previous week at some point the next_week method forget to take the TZ
into account. next_week should probably enforce TZ being consistant
across the whole computations (in general this becomes quite tricky when
you mix several Time objects with various TZs in the computation).

This isn’t applicable to your specific problem, but I take the
opportunity to write it there:
The timezone problem being so complex, I only manipulate UTC Time
objects (try t.utc, t.utc.next_week, t.utc.next_week.next_week and
you’ll see the problem doesn’t show up) and convert to localtime only
when needed. Unfortunately, the next_week result is clearly TZ-dependant
so you can’t simply switch to UTC.

Lionel.

On Tue, 24 Oct 2006 18:04:35 +0200
Chris R. [email protected] wrote:

The Time::next_week method is supposed to give the time of the start of
the next week. But look at this, it cocks up :

I kind of like Chronic for this:

http://chronic.rubyforge.org/

It was demonstrated at RubyConf and totally rocked.


Zed A. Shaw, MUDCRAP-CE Master Black Belt Sifu

http://mongrel.rubyforge.org/
http://www.lingr.com/room/3yXhqKbfPy8 – Come get help.

Your close Lionel, actually its due to the switch from Daylight Savings
Time here in the U.S. to Standard Time.

t = Time.parse “Sun October 29 2006”
=> Sun October 29 00:00:00 Pacific Daylight Time 2006

t.since(1.day)
=> Sun October 29 23:00:00 Pacific Standard Time 2006

1.day is really just a shortcut for the number of seconds in a day:
86,400. The same goes for 1.week. It’s really just a shortcut for the
number of seconds in a week: 604,800.

So when you execute the method you did, next_week():

def next_week(day = :monday)
days_into_week = {:monday => 0, :tuesday => 1, :wednesday => 2,
:thursday => 3, :friday => 4, :saturday => 5, :sunday => 6}
since(1.week).beginning_of_week.since(days_into_week[:day].day).change(:hour
=> 0)
end

… with the given date of Monday, October 23rd, it tries to increment
it by adding a weeks worth of seconds to the given date. However, due
to the switch from DST to ST, a weeks worth of seconds only gets you to
11:00PM on Sunday instead of 12:00AM on Monday. Thus the incorrect
date.

I guess technically the bug is with our government (:slight_smile: one of many) in
their continuing this arcane thing called Daylight Savings Time.

However, I believe Ruby provides a method to convert a time to UTC and
then back to your local time. I suppose this might solve the problem by
taking stupid DST out of the equation. I’ll mess around with it and see
if I can create a patch.

Lionel B. wrote:

This is most probably a timezone related bug and not easily solved IMHO.
It seems to me that as in UTC your original date is in fact in the
previous week at some point the next_week method forget to take the TZ
into account. next_week should probably enforce TZ being consistant
across the whole computations (in general this becomes quite tricky when
you mix several Time objects with various TZs in the computation).

This isn’t applicable to your specific problem, but I take the
opportunity to write it there:
The timezone problem being so complex, I only manipulate UTC Time
objects (try t.utc, t.utc.next_week, t.utc.next_week.next_week and
you’ll see the problem doesn’t show up) and convert to localtime only
when needed. Unfortunately, the next_week result is clearly TZ-dependant
so you can’t simply switch to UTC.

Lionel.

Well, I posted about that a month ago, and nobody seemed to care…

I find this attitude quite disturbing actually. In fact, Rails just
doesn’t care about everything that is not-US.

For example, the validation error messages can be translated, but not
the “There was 3 errors trying to validate…”

Anyway, here is what I came with to fix the calculations problems. And
it works:

module ActiveSupport::CoreExtensions::Time::Calculations
def beginning_of_day
self.change(:hour=>0)
end

def beginning_of_week
temp = self.change(:hour=>12)
days_to_monday = temp.wday!=0 ? temp.wday-1 : 6
(temp - days_to_monday.days).beginning_of_day
end

def next_monday
(self.beginning_of_week+10.days).beginning_of_week
end

def monday_last_week
(self.beginning_of_week-3.days).beginning_of_week
end

def yesterday
((self.beginning_of_day)-12.hours).change(:hour=>self.hour,
:min=>self.min, :sec=>self.sec)
end

def tomorrow
((self.beginning_of_day)+36.hours).change(:hour=>self.hour,
:min=>self.min, :sec=>self.sec)
end

end

Zed A. Shaw wrote:

http://chronic.rubyforge.org/

It was demonstrated at RubyConf and totally rocked.


Zed A. Shaw, MUDCRAP-CE Master Black Belt Sifu
http://www.zedshaw.com/
http://mongrel.rubyforge.org/
http://www.lingr.com/room/3yXhqKbfPy8 – Come get help.

I liked chronic so much that I added support for it in the ruby-units
gem.

Now you can do things like…

irb>‘3 weeks’.from ‘today’
#=>Tue Nov 14 19:30:00 EST 2006

_Kevin

I created a patch to fix this problem.

http://dev.rubyonrails.org/ticket/6483

Nauhaie N. wrote:

Well, I posted about that a month ago, and nobody seemed to care…

The patch for the defect, http://dev.rubyonrails.org/ticket/6483, was
applied to Edge within 2 hours.

I find this attitude quite disturbing actually. In fact, Rails just
doesn’t care about everything that is not-US.

As far as I am aware most US states use DST, so this defect affected US
as well.

For example, the validation error messages can be translated, but not
the “There was 3 errors trying to validate…”

The application I am currently working on happily displays “Der blev
fundet 3 problemer i din indtastning”. This stuff is easy to modify
(hint: Noone is forcing you to use the (very basic) builtin helper).


Jakob S. - http://mentalized.net

I fixed the translation problem to, but I had to patch the helper, which
is quite dirty…

Anyway, I’m glad to see the DST problem fixed!

Nauhaie

So, I guess the thing for you to take to your boss - seriously - is the
speed with which this was fixed after you observed the problem. This
ain’t patch Tuesday, after all.

c.

Nauhaie N. wrote:

I fixed the translation problem to, but I had to patch the helper, which
is quite dirty…

Anyway, I’m glad to see the DST problem fixed!

Nauhaie

On Oct 24, 2006, at 6:03 PM, Marc L. wrote:

I created a patch to fix this problem.

http://dev.rubyonrails.org/ticket/6483

On Oct 25, 2006, at 10:51 AM, Jakob S. wrote:

As far as I am aware most US states use DST, so this defect

Jakob S. - http://mentalized.net

On Oct 25, 2006, at 2:11 PM, Cayce B. wrote:

is quite dirty…

Anyway, I’m glad to see the DST problem fixed!

Nauhaie

Sorry to have to jump in here, but I made a patch back in July for this:
http://dev.rubyonrails.org/ticket/5617

I’ve updated it to fix the problem correctly and broken out the test
that show the continued problem with next_week as “fixed” by ticket
6483.

I only hope that this ticket with its renewed attention gets swift
attention, too.

-Rob

Rob B. http://agileconsultingllc.com
[email protected]

On 10/30/06, Rob B. [email protected] wrote:

Nauhaie N. wrote:
as well.

Nauhaie N. wrote:
http://dev.rubyonrails.org/ticket/5617

I’ve updated it to fix the problem correctly and broken out the test
that show the continued problem with next_week as “fixed” by ticket
6483.

I only hope that this ticket with its renewed attention gets swift
attention, too.

It has been fixed:

http://dev.rubyonrails.org/changeset/5388