test_treeview_date.xul fails for the rest of the day when daylight saving time (summer time) ends

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Toolkit
Places
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: dbaron, Assigned: mak)

Tracking

({intermittent-failure})

Trunk
mozilla1.9.3a1
intermittent-failure
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [orange:time-bomb])

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
Since daylight saving time ended this morning, chrome://mochikit/content/chrome/browser/components/places/tests/chrome/test_treeview_date.xul has been failing with:

40 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/browser/components/places/tests/chrome/test_treeview_date.xul | Date format is correct - got "11/1/09 12:00 AM", expected "12:00 AM"
45 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/browser/components/places/tests/chrome/test_treeview_date.xul | Date format is correct - got "11/1/09 12:00 AM", expected "12:00 AM"

This makes me suspect that the date formatting code places is using is doing a "time is same day" check before adjusting for any timezone changes that happened during that day.
Whiteboard: [orange]
(Assignee)

Comment 1

8 years ago
hm, then i'd expect this to fail always, and not randomly.
(Assignee)

Comment 2

8 years ago
looks like the failure has stopped by itself, it's probably a calculation error around the hour when the DST change happens.
(Assignee)

Comment 3

8 years ago
as Neil pointed out the day when DST changes has a different duration, and that hurts our code assuming the classic 24 hours day. So it's probably this piece of code in treeview.js
Assignee: nobody → mak77
(Assignee)

Comment 4

8 years ago
and yeah, it's for 1 day, not 1 hour as i initially pointed
(Assignee)

Comment 5

8 years ago
Created attachment 409709 [details] [diff] [review]
patch v1.0

the problem is that midnight can have a different timezoneOffset from the current time, so the comparison happens with an unwanted offset. This should correct that behavior, tested on 31-1-2 October 2009 PDT, 13-14-15 March 2010 PDT, 24-25-26 Oct 2009 GMT, 27-28-29 March 2010 GMT, and on different timezones on current time.
Testing automatically is hard since the test is based on current time, but the test will catch eventual problems day by day (as happened yesterday).
(Assignee)

Updated

8 years ago
Attachment #409709 - Flags: review?(dietrich)
(Assignee)

Comment 6

8 years ago
and notice the day after (today) everything works fine because both midnight and current time have the same timezoneOffset and the algorithm is based on diffs.
Attachment #409709 - Flags: review?(dietrich) → review+
Comment on attachment 409709 [details] [diff] [review]
patch v1.0

looks ok, i suppose we can't make a system call to change the clock before and after the test is run? ;)
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> (From update of attachment 409709 [details] [diff] [review])
> looks ok, i suppose we can't make a system call to change the clock before and
> after the test is run? ;)

i can't even imagine which kind of random oranges that could cause!
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/8e93f7413849
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
(Reporter)

Updated

8 years ago
Whiteboard: [orange] → [orange][orange:time-bomb]
Keywords: intermittent-failure
Whiteboard: [orange][orange:time-bomb] → [orange:time-bomb]
You need to log in before you can comment on or make changes to this bug.