WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
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
Details
Patch
(24.17 KB, patch)
2013-05-07 07:52 PDT
,
Chris Dumez
haraken
: review-
haraken
: commit-queue-
Details
Formatted Diff
Diff
WIP Patch
(355.56 KB, patch)
2013-05-07 09:14 PDT
,
Chris Dumez
buildbot
: commit-queue-
Details
Formatted Diff
Diff
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
Details
Patch
(516.48 KB, patch)
2013-05-07 10:59 PDT
,
Chris Dumez
benjamin
: review-
benjamin
: commit-queue-
Details
Formatted Diff
Diff
WIP Patch
(521.29 KB, patch)
2013-05-08 00:14 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(527.84 KB, patch)
2013-05-08 04:27 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 200905
[details]
Patch
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
Created
attachment 201056
[details]
Patch
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.
Roger Fong
Comment 18
2013-05-09 13:12:54 PDT
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?
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.
Top of Page
Format For Printing
XML
Clone This Bug