Last Comment Bug 836404 - Provide better timezone support in jstests
: Provide better timezone support in jstests
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla23
Assigned To: André Bargull
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-30 11:02 PST by André Bargull
Modified: 2013-04-05 13:38 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Enhanced timezone support for ecma/shell.js (6.10 KB, patch)
2013-01-30 11:58 PST, André Bargull
jwalden+bmo: review+
Details | Diff | Splinter Review
Test results with and without patch for various timezones (63.42 KB, application/octet-stream)
2013-01-30 12:12 PST, André Bargull
no flags Details

Description André Bargull 2013-01-30 11:02:44 PST
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.
Comment 1 André Bargull 2013-01-30 11:58:17 PST
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()).
Comment 2 André Bargull 2013-01-30 12:12:09 PST
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.
Comment 3 Nicholas Nethercote [:njn] 2013-01-31 00:58:49 PST
Have you tested the Australia/Adelaide timezone?  It has a half-hour difference from the Australia/Sydney timezone, and so is an interesting case.
Comment 4 André Bargull 2013-01-31 04:26:09 PST
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 5 Jeff Walden [:Waldo] (remove +bmo to email) 2013-04-03 12:18:29 PDT
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.
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2013-04-04 14:52:13 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/695eb5588304
Comment 7 Ryan VanderMeulen [:RyanVM] 2013-04-05 13:38:02 PDT
https://hg.mozilla.org/mozilla-central/rev/695eb5588304

Note You need to log in before you can comment on or make changes to this bug.