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

peavo
Reported 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.
Attachments
Patch (2.42 KB, patch)
2013-11-27 12:24 PST, peavo 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
peavo
Comment 1 2013-11-27 12:24:22 PST
Created attachment 217963 [details] Patch
peavo
Comment 2 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.
peavo
Comment 3 2013-11-27 12:27:34 PST
Also see bug 25160.
EFL EWS Bot
Comment 4 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
peavo
Comment 5 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.
Brent Fulgham
Comment 6 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?
peavo
Comment 7 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 :)
Brent Fulgham
Comment 8 2013-11-30 19:04:26 PST
Comment on attachment 217963 [details] Patch R=me
WebKit Commit Bot
Comment 9 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>
WebKit Commit Bot
Comment 10 2013-11-30 19:30:59 PST
All reviewed patches have been landed. Closing bug.
Brent Fulgham
Comment 11 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!
Daniel Bates
Comment 12 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.
Radar WebKit Bug Importer
Comment 13 2014-01-23 09:45:36 PST
<rdar://problem/15892592>
Brent Fulgham
Comment 14 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?
Daniel Bates
Comment 15 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.
Daniel Bates
Comment 16 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>
Note You need to log in before you can comment on or make changes to this bug.