Bug 124946

Summary: [Win] Some JavaScript date tests are failing in non-California Timezones
Product: WebKit Reporter: peavo
Component: Web Template FrameworkAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bfulgham, cmarcelo, commit-queue, dbates, eflews.bot, ggaren, gyuyoung.kim, msaboff, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=250553
Bug Depends on:    
Bug Blocks: 124450    
Attachments:
Description Flags
Patch none

Description peavo 2013-11-27 12:17:13 PST
When I run the javascript tests on Windows, I get the errors listed below.

I have traced this down to the method calculateDSTOffset in wtf/DateMath.cpp.

On Windows, I think it makes sense to use the native functions for this, instead of using the function getLocalTime, which uses the localtime_s function in the C runtime library.

** The following Mozilla test failures have been introduced:
        ecma/Date/15.9.5.10-1.js
        ecma/Date/15.9.5.10-10.js
        ecma/Date/15.9.5.10-11.js
        ecma/Date/15.9.5.10-12.js
        ecma/Date/15.9.5.10-13.js
        ecma/Date/15.9.5.10-2.js
        ecma/Date/15.9.5.10-3.js
        ecma/Date/15.9.5.10-4.js
        ecma/Date/15.9.5.10-5.js
        ecma/Date/15.9.5.10-6.js
        ecma/Date/15.9.5.10-7.js
        ecma/Date/15.9.5.10-8.js
        ecma/Date/15.9.5.10-9.js
        ecma/Date/15.9.5.12-1.js
        ecma/Date/15.9.5.12-2.js
        ecma/Date/15.9.5.12-3.js
        ecma/Date/15.9.5.12-4.js
        ecma/Date/15.9.5.12-5.js
        ecma/Date/15.9.5.12-6.js
        ecma/Date/15.9.5.12-7.js
        ecma/Date/15.9.5.28-1.js
        ecma/Date/15.9.5.29-1.js
        ecma/Date/15.9.5.35-1.js
        ecma/Date/15.9.5.8.js
        ecma/String/15.5.4.6-2.js

Results for Mozilla tests:
    25 regressions found.
    0 tests fixed.
Comment 1 peavo 2013-11-27 12:24:22 PST
Created attachment 217963 [details]
Patch
Comment 2 peavo 2013-11-27 12:25:38 PST
These are the results with the patch included:

Results for Mozilla tests:
    0 regressions found.
    0 tests fixed.
    OK.
Comment 3 peavo 2013-11-27 12:27:34 PST
Also see bug 25160.
Comment 4 EFL EWS Bot 2013-11-27 12:58:34 PST
Comment on attachment 217963 [details]
Patch

Attachment 217963 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/38598062
Comment 5 peavo 2013-11-27 13:22:06 PST
The efl-wk2 build seems to fail with an internal compiler error in a different file, so I don't think that is related to this patch.
Comment 6 Brent Fulgham 2013-11-28 09:22:44 PST
Do these same failures happen under Apple's Windows port? It looks like there are 11 jscore errors, but I can't see what those errors arson the buildbots. Still, they can' t be the same just based on counts.

Should we only use this code path for the WinCairo port?
Comment 7 peavo 2013-11-30 00:52:42 PST
(In reply to comment #6)
> Do these same failures happen under Apple's Windows port? It looks like there are 11 jscore errors, but I can't see what those errors arson the buildbots. Still, they can' t be the same just based on counts.
> 
> Should we only use this code path for the WinCairo port?

Thanks for looking into this :)

Yes, I get the exact same errors when running WinApple.

The strange thing is that, when I change the timezone to e.g. California-time (UTC - 8.00, I think), there are no errors, both with, and without the patch.
It seems the results depends on which timezone you are in, which might explain why we are seeing different results.

With the patch, I get no errors in both timezones.

To be on the safe side, I can make this patch WinCairo-only. Your call :)
Comment 8 Brent Fulgham 2013-11-30 19:04:26 PST
Comment on attachment 217963 [details]
Patch

R=me
Comment 9 WebKit Commit Bot 2013-11-30 19:30:57 PST
Comment on attachment 217963 [details]
Patch

Clearing flags on attachment: 217963

Committed r159892: <http://trac.webkit.org/changeset/159892>
Comment 10 WebKit Commit Bot 2013-11-30 19:30:59 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Brent Fulgham 2013-11-30 22:52:05 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Do these same failures happen under Apple's Windows port? It looks like there are 11 jscore errors, but I can't see what those errors arson the buildbots. Still, they can' t be the same just based on counts.
> > 
> Thanks for looking into this :)
> 
> Yes, I get the exact same errors when running WinApple.
> 
> The strange thing is that, when I change the timezone to e.g. California-time (UTC - 8.00, I think), there are no errors, both with, and without the patch.
> It seems the results depends on which timezone you are in, which might explain why we are seeing different results.
> 
> With the patch, I get no errors in both timezones.
> 

Very interesting. I wonder if this indicates a general weakness in our test coverage. All of our test bots live in California, and use that time zone.

Thanks for catching this!
Comment 12 Daniel Bates 2014-01-23 09:44:49 PST
(In reply to comment #9)
> (From update of attachment 217963 [details])
> Clearing flags on attachment: 217963
> 
> Committed r159892: <http://trac.webkit.org/changeset/159892>

Using the data on the Flakiness Dashboard (*) as a starting point (**) and comparing the Windows {Debug, Release} expected results diffs, this commit regressed test LayoutTests/js/dom/date-big-constructor.html

(*) <http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=js%2Fdom%2Fdate-big-constructor.html>

(**) The Flakiness Dashboard only has data that shows the test passed in r159891 and failed since r159894. Looking at the commits in [r159891, r159894], <http://trac.webkit.org/changeset/159892> seemed relevant given that it's the only patch that touched code related to date logic. Moreover, the description of this bug is with regards to JavaScript date tests.
Comment 13 Radar WebKit Bug Importer 2014-01-23 09:45:36 PST
<rdar://problem/15892592>
Comment 14 Brent Fulgham 2014-01-23 09:47:07 PST
Interesting!  So we fixed ecma problems, but somehow broke a layout test?

What is the layout test failure? Could it be that we had bad expectations checked in?
Comment 15 Daniel Bates 2014-01-23 09:49:02 PST
(In reply to comment #12)
> (In reply to comment #9)
> > (From update of attachment 217963 [details] [details])
> > Clearing flags on attachment: 217963
> > 
> > Committed r159892: <http://trac.webkit.org/changeset/159892>
> 
> Using the data on the Flakiness Dashboard (*) as a starting point (**) and comparing the Windows {Debug, Release} expected results diffs, this commit regressed test LayoutTests/js/dom/date-big-constructor.html
> 
> [...]

Filed bug #127492 for the regression.
Comment 16 Daniel Bates 2014-01-23 09:51:10 PST
(In reply to comment #14)
> Interesting!  So we fixed ecma problems, but somehow broke a layout test?
> 

Yes, according to the Flakiness Dashboard.

> What is the layout test failure? Could it be that we had bad expectations checked in?

Windows disagrees with the cross-platform expected results for the test.

Windows Debug bot diff:
<http://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/r162609%20(57442)/js/dom/date-big-constructor-pretty-diff.html>

Window Release Bot diff (identical to the Windows Debug bot diff):
<http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r162610%20(41968)/js/dom/date-big-constructor-pretty-diff.html>