Bug 115724

Summary: Add layout test that lists all global constructors
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Tools / TestsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, commit-queue, gyuyoung.kim, haraken, laszlo.gombos, rakuco, rniwa, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 115714    
Attachments:
Description Flags
WIP Patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Patch
haraken: review-, haraken: commit-queue-
WIP Patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion
none
Patch
benjamin: review-, benjamin: commit-queue-
WIP Patch
none
Patch none

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.