Bug 115724 - Add layout test that lists all global constructors
Summary: Add layout test that lists all global constructors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 115714
  Show dependency treegraph
 
Reported: 2013-05-07 07:05 PDT by Chris Dumez
Modified: 2013-05-09 14:22 PDT (History)
9 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 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.
Comment 1 Chris Dumez 2013-05-07 07:13:45 PDT
Created attachment 200901 [details]
WIP Patch

Using ews to get mac results.
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Chris Dumez 2013-05-07 07:52:15 PDT
Created attachment 200905 [details]
Patch
Comment 5 Kentaro Hara 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.
Comment 6 Chris Dumez 2013-05-07 09:14:49 PDT
Created attachment 200912 [details]
WIP Patch
Comment 7 Ryosuke Niwa 2013-05-07 10:27:07 PDT
I thought we already had a test for this.
Comment 8 Chris Dumez 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.
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Chris Dumez 2013-05-07 10:59:34 PDT
Created attachment 200933 [details]
Patch

Improve existing test case instead of introducing a new one.
Comment 12 Benjamin Poulain 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.
Comment 13 Chris Dumez 2013-05-08 00:14:30 PDT
Created attachment 201032 [details]
WIP Patch

Using ews to get mac results.
Comment 14 Chris Dumez 2013-05-08 04:27:53 PDT
Created attachment 201056 [details]
Patch
Comment 15 Ryosuke Niwa 2013-05-08 11:44:24 PDT
Comment on attachment 201056 [details]
Patch

Looks good.
Comment 16 WebKit Commit Bot 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>
Comment 17 WebKit Commit Bot 2013-05-08 12:27:44 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Chris Dumez 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.
Comment 20 Chris Dumez 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;
Comment 21 Roger Fong 2013-05-09 13:26:28 PDT
okay, i'll go ahead and rebaseline then, thanks!
Comment 22 Benjamin Poulain 2013-05-09 14:22:22 PDT
Late to the party but LGTM too :)
The results look good now.