Provide better timezone support in jstests

RESOLVED FIXED in mozilla23

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: André Bargull, Assigned: André Bargull)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130116073211

Steps to reproduce:

tests/ecma/shell.js lacks support for timezones which do not observe daylight time saving or reside on the southern hemisphere.
(Assignee)

Updated

4 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Comment 1

4 years ago
Created attachment 708240 [details] [diff] [review]
Enhanced timezone support for ecma/shell.js

This patch adds (partial) support for timezones without DST resp. for timezones on the southern hemisphere. Except for the "IgnoreDaylightSaving()" function, the changes should be easy to understand:

Multiple tests in the "tests/ecma/Date" folder use dates near start of the epoch. These dates are converted to local time (using the helper functions in ecma/shell.js) and then compared to the output from the various Date.prototype.getXXX() functions. The conversion step may need to apply DST correction to obtain the correct result. But this should only take place when DST was actually observed at that time. You can use the "Australia/Sydney" timezone to observe this effect (33 failures without IgnoreDaylightSaving(), 0 failure with IgnoreDaylightSaving()).
Attachment #708240 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 2

4 years ago
Created attachment 708247 [details]
Test results with and without patch for various timezones

I've tested this patch with a few timezones (well, 457 timezones may no longer count as 'few') and found no regressions. That means any timezone which passed the tests without this patch, still passes the tests with the patch applied. (Note: I've only used the Date related tests in "tests/ecma/Date" to reduce the overall time needed to complete the test suite). The patches for bug 830304, bug 835850 and bug 836396 also need to be applied to get sensible results. The complete results can be found in the attachment.
Have you tested the Australia/Adelaide timezone?  It has a half-hour difference from the Australia/Sydney timezone, and so is an interesting case.
(Assignee)

Comment 4

4 years ago
Yes, Australia/Adelaide passes without failures (*). Only the following Australian timezones still produce failures: Brisbane (Queensland), Eucla, Lindeman, Lord_Howe (LHI) and Perth (West). 

Reasons:
- Perth, Eucla, Brisbane and Lindeman did not observe DST in 2000 (**), so they're marked as never observing DST
- Lord_Howe, let's just say 30 minutes DST rules don't work that well in the script...


(*) excluding date format failures, but that's another story
(**) 2000 is used as a reference point in ecma/shell.js
Comment on attachment 708240 [details] [diff] [review]
Enhanced timezone support for ecma/shell.js

Review of attachment 708240 [details] [diff] [review]:
-----------------------------------------------------------------

I think I understand this better than the previous patch, and it looks more reasonable than that one.  Not full grokking, but again, time demands say I should stamp this now.

I'll push both patches next time I have stuff to push, and after some tryservering for safety.
Attachment #708240 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/695eb5588304
Assignee: general → andrebargull
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/695eb5588304
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.