RESOLVED FIXED 115724
Add layout test that lists all global constructors
https://bugs.webkit.org/show_bug.cgi?id=115724
Summary Add layout test that lists all global constructors
Chris Dumez
Reported 2013-05-07 07:05:12 PDT
We should have a layout test that lists all Constructor properties on the window object to better highlight changes to those.
Attachments
WIP Patch (9.41 KB, patch)
2013-05-07 07:13 PDT, Chris Dumez
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (494.48 KB, application/zip)
2013-05-07 07:42 PDT, Build Bot
no flags
Patch (24.17 KB, patch)
2013-05-07 07:52 PDT, Chris Dumez
haraken: review-
haraken: commit-queue-
WIP Patch (355.56 KB, patch)
2013-05-07 09:14 PDT, Chris Dumez
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (625.25 KB, application/zip)
2013-05-07 10:54 PDT, Build Bot
no flags
Patch (516.48 KB, patch)
2013-05-07 10:59 PDT, Chris Dumez
benjamin: review-
benjamin: commit-queue-
WIP Patch (521.29 KB, patch)
2013-05-08 00:14 PDT, Chris Dumez
no flags
Patch (527.84 KB, patch)
2013-05-08 04:27 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2013-05-07 07:13:45 PDT
Created attachment 200901 [details] WIP Patch Using ews to get mac results.
Build Bot
Comment 2 2013-05-07 07:42:44 PDT
Comment on attachment 200901 [details] WIP Patch Attachment 200901 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/338068 New failing tests: fast/dom/global-constructors-listing.html
Build Bot
Comment 3 2013-05-07 07:42:46 PDT
Created attachment 200904 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.2
Chris Dumez
Comment 4 2013-05-07 07:52:15 PDT
Kentaro Hara
Comment 5 2013-05-07 08:20:54 PDT
Comment on attachment 200905 [details] Patch Adding these tests looks reasonable. Would you remove (a part of) LayoutTests/fast/dom/constructed-objects-prototypes.html ? We don't want to manage two tests for this.
Chris Dumez
Comment 6 2013-05-07 09:14:49 PDT
Created attachment 200912 [details] WIP Patch
Ryosuke Niwa
Comment 7 2013-05-07 10:27:07 PDT
I thought we already had a test for this.
Chris Dumez
Comment 8 2013-05-07 10:30:25 PDT
(In reply to comment #7) > I thought we already had a test for this. It was removed when the global constructors were made not enumerable (as it broke the test). However, we can use Object.getOwnPropertyNames(window) to work around this.
Build Bot
Comment 9 2013-05-07 10:54:52 PDT
Comment on attachment 200912 [details] WIP Patch Attachment 200912 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/335148 New failing tests: fast/js/global-constructors-attributes.html
Build Bot
Comment 10 2013-05-07 10:54:54 PDT
Created attachment 200930 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.2
Chris Dumez
Comment 11 2013-05-07 10:59:34 PDT
Created attachment 200933 [details] Patch Improve existing test case instead of introducing a new one.
Benjamin Poulain
Comment 12 2013-05-07 14:30:35 PDT
Comment on attachment 200933 [details] Patch The test is a great idea. But the results have zero readability. If one line fail, you would have no idea which constructor is failing the test. The output needs to serialize the value of constructorName.
Chris Dumez
Comment 13 2013-05-08 00:14:30 PDT
Created attachment 201032 [details] WIP Patch Using ews to get mac results.
Chris Dumez
Comment 14 2013-05-08 04:27:53 PDT
Ryosuke Niwa
Comment 15 2013-05-08 11:44:24 PDT
Comment on attachment 201056 [details] Patch Looks good.
WebKit Commit Bot
Comment 16 2013-05-08 12:27:40 PDT
Comment on attachment 201056 [details] Patch Clearing flags on attachment: 201056 Committed r149758: <http://trac.webkit.org/changeset/149758>
WebKit Commit Bot
Comment 17 2013-05-08 12:27:44 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 19 2013-05-09 13:17:51 PDT
(In reply to comment #18) > Test fails on mac-lion: > http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r149820%20(12294)/fast/js/global-constructors-attributes-pretty-diff.html > > Is this to be expected? If you don't have NOTIFICATIONS|LEGACY_NOTIFICATIONS enabled, yes. You likely need a platform-specific baseline.
Chris Dumez
Comment 20 2013-05-09 13:24:20 PDT
(In reply to comment #19) > (In reply to comment #18) > > Test fails on mac-lion: > > http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r149820%20(12294)/fast/js/global-constructors-attributes-pretty-diff.html > > > > Is this to be expected? > > If you don't have NOTIFICATIONS|LEGACY_NOTIFICATIONS enabled, yes. You likely need a platform-specific baseline. FYI, the global baseline is the one from mac-ews. However, it seems the mac port may be enabled NOTIFICATIONS depending on the Mac OS version: Source/WebCore/Configurations/FeatureDefines.xcconfig:ENABLE_NOTIFICATIONS_macosx_1070 = ; Source/WebCore/Configurations/FeatureDefines.xcconfig:ENABLE_NOTIFICATIONS_macosx_1080 = ENABLE_NOTIFICATIONS; Source/WebCore/Configurations/FeatureDefines.xcconfig:ENABLE_NOTIFICATIONS_macosx_1090 = ENABLE_NOTIFICATIONS;
Roger Fong
Comment 21 2013-05-09 13:26:28 PDT
okay, i'll go ahead and rebaseline then, thanks!
Benjamin Poulain
Comment 22 2013-05-09 14:22:22 PDT
Late to the party but LGTM too :) The results look good now.
Note You need to log in before you can comment on or make changes to this bug.