Bug 139138

Summary: Several JavaScriptCore date tests are flaky, because they expect time to be frozen during execution
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, mark.lam
Priority: P2 Keywords: MakingBotsRed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed fix mark.lam: review+

Alexey Proskuryakov
Reported 2014-12-01 11:24:49 PST
At least these tests are affected: Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.1.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-1.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-2.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-3.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-4.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-5.js Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-6.js mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: Detected failures: mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: 15.9.2.2 The Date Constructor Called as a Function mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: Date(2031,11,31,23,59,59) = Sun Nov 30 2014 23:48:48 GMT-0800 (PST) FAILED! expected: Sun Nov 30 2014 23:48:47 GMT-0800 (PST) mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: Date(2032,0,1,0,0,0) = Sun Nov 30 2014 23:48:48 GMT-0800 (PST) PASSED! mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: Date(2032,0,1,0,0,1) = Sun Nov 30 2014 23:48:48 GMT-0800 (PST) PASSED! mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases: Date(2031,11,31,16,0,0,0) = Sun Nov 30 2014 23:48:48 GMT-0800 (PST) PASSED! FAIL: mozilla-tests.yaml/ecma/Date/15.9.2.2-6.js.mozilla-dfg-eager-no-cjit-validate-phases The tests seem bad - not even Firefox freezes the current time during script execution. The tests have an unused TOLERANCE variable, so maybe whoever wrote them understood the problem.
Attachments
proposed fix (30.15 KB, patch)
2014-12-01 12:32 PST, Alexey Proskuryakov
mark.lam: review+
Alexey Proskuryakov
Comment 1 2014-12-01 11:25:33 PST
This is a whole lot of flakiness for tests that do pretty much nothing (all they verify is that arguments are ignored!)
Alexey Proskuryakov
Comment 2 2014-12-01 11:27:28 PST
Looks like Mozilla folks fixed this long ago: <https://bugzilla.mozilla.org/show_bug.cgi?id=404351>.
Alexey Proskuryakov
Comment 3 2014-12-01 12:32:58 PST
Created attachment 242328 [details] proposed fix Manually merged the fix, because our version of shell.js is obsolete even compared to what Mozilla had in 2007. Perhaps we should update to trunk? I actually tried that just for laughs, but it wasn't easy enough.
Mark Lam
Comment 4 2014-12-01 12:54:35 PST
Comment on attachment 242328 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=242328&action=review > Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-2.js:72 > + array[item++] = new TestCase( SECTION, "Date(2000,0,1,0,0,0)", true, d2 - d1 <= 1000); I was concerned about all the copy and pasting, and was wondering if it is better to write a utility function to eliminate the cut and paste like so: function makeTestCase(dateExpr) { var d1 = new Date(); var d2 = eval(dateExpr); return new TestCase(SECTION, dateExpr, true, d2 - d1 <= 1000); } array[item++] = makeTestCase("Date(2000,0,1,0,0,0)"); Here, we're seeing a discrepancy between the reported and the evaluated expressions. I see that the discrepancy is also in the original test code (which is a bug). Regardless, I think it'd be better to use a utility function instead. > Source/JavaScriptCore/tests/mozilla/ecma/Date/15.9.2.2-2.js:76 > + array[item++] = new TestCase( SECTION, "Date(2000,0,1,0,0,1)", true, d2 - d1 <= 1000) Ditto. Another discrepancy.
Mark Lam
Comment 5 2014-12-01 12:58:37 PST
Comment on attachment 242328 [details] proposed fix r=me for the sake of keeping the test code consistent with Mozilla's and easily mergeable. However, please fix the 2 discrepancies. That shouldn't make merging that much harder in the future. Thanks.
Alexey Proskuryakov
Comment 6 2014-12-01 13:14:25 PST
Note You need to log in before you can comment on or make changes to this bug.