Fix jstest failures in non-PST timezones

RESOLVED FIXED in mozilla36

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
9 months ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 8512098 [details] [diff] [review]
Patch

Outside the Pacific time zone, the following jstests have been perma-fail since forever:

    ecma_3/Date/15.9.5.5.js
    ecma_3/Date/15.9.5.6.js
    ecma_3/Date/15.9.5.7.js
    js1_5/extensions/toLocaleFormat-01.js
    js1_5/Regress/regress-58116.js

This patch fixes them as follows:

(*) It adds a tz-pacific flag to the jsreftest harness. This is similar to the jit-test flag with the same name. Like the jit-test harness, it sets the TZ=PST8PDT environment variable.

(*) For the first two tests, I also had to change toLocaleDateString() to toLocaleDateString("en-US") to get a string that Date.parse can parse correctly.

I only changed tasks_unix.py. tasks_win.py seems to use worker threads so it's more complicated: updating os.environ could race with other threads.
Attachment #8512098 - Flags: review?(terrence)
Comment on attachment 8512098 [details] [diff] [review]
Patch

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

Looks good, but be sure to try-server first. We basically just write the test header verbatim into the jstests.list that reftests parse and I'm not sure how it will deal with random, unknown keywords. If it errors out on the new keyword, I'd also be fine just overriding TZ unconditionally.
Attachment #8512098 - Flags: review?(terrence) → review+
(Assignee)

Comment 2

4 years ago
Created attachment 8512571 [details] [diff] [review]
Patch v2

You were right, the jsreftest harness wasn't happy. This patch just sets the environment variable in the main() function. A bit simpler and should hopefully also work on Windows/Cygwin this way.
Attachment #8512098 - Attachment is obsolete: true
Attachment #8512571 - Flags: review?(terrence)
Attachment #8512571 - Flags: review?(terrence) → review+
Any chance this might fix bug 610183?  If so, that would be great!

(I guess we'll find out this weekend.)
(Assignee)

Comment 6

4 years ago
(In reply to Daniel Holbert [:dholbert] from comment #4)
> Any chance this might fix bug 610183?  If so, that would be great!
> 
> (I guess we'll find out this weekend.)

I replied on IRC, but for others following along: this patch only affects the shell harness and jsreftests run in the browser on tinderbox.
https://hg.mozilla.org/mozilla-central/rev/99072302a3f4
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
/me wonders if he can now safely set the timezone on his Linux desktop machine to something other than Pacific time...
(In reply to Nicholas Nethercote [:njn] from comment #8)
> /me wonders if he can now safely set the timezone on his Linux desktop
> machine to something other than Pacific time...

Were you able to? In particular, is bug 515254 still relevant?
Flags: needinfo?(n.nethercote)
> > /me wonders if he can now safely set the timezone on his Linux desktop
> > machine to something other than Pacific time...
> 
> Were you able to? In particular, is bug 515254 still relevant?

I tried and got no failures, so I closed bug 515254. Thanks for the reminder.
Flags: needinfo?(n.nethercote)

Updated

9 months ago
Duplicate of this bug: 345840

Updated

9 months ago
Duplicate of this bug: 183817

Updated

9 months ago
Duplicate of this bug: 395501

Updated

9 months ago
Duplicate of this bug: 530974

Updated

9 months ago
Duplicate of this bug: 934238
You need to log in before you can comment on or make changes to this bug.