Bug 42354 - Improve coverage of fast/dom/prototype-inheritance test
Summary: Improve coverage of fast/dom/prototype-inheritance test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kent Hansen
URL:
Keywords:
Depends on:
Blocks: 28771
  Show dependency treegraph
 
Reported: 2010-07-15 03:29 PDT by Kent Hansen
Modified: 2010-07-22 01:55 PDT (History)
0 users

See Also:


Attachments
Patch (60.83 KB, patch)
2010-07-15 03:39 PDT, Kent Hansen
sam: review+
Details | Formatted Diff | Diff
Patch v2 (skip jscprint property) (61.21 KB, patch)
2010-07-19 05:12 PDT, Kent Hansen
sam: review+
kent.hansen: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Hansen 2010-07-15 03:29:38 PDT
The fast/dom/prototype-inheritance test uses a for..in statement to enumerate the properties of the window object. An issue with that approach is that non-enumerable properties, in particular the built-in ECMA properties, are not enumerated, and hence not tested.

Since we now have the ES5 function Object.getOwnPropertyNames implemented, it could be used in place of for..in; Object.getOwnPropertyNames returns both enumerable and non-enumerable properties. This will increase the test coverage, and as a bonus the test won't be sensitive to changes in the enumerable flag of existing properties (see https://bugs.webkit.org/show_bug.cgi?id=28771). Case in point, currently the Chromium expected results don't include the *Error constructors due to them being non-enumerable in V8 (like the spec says), whereas in JSC they are enumerable (which is what I'd ultimately like to fix, without reducing test coverage).
Comment 1 Kent Hansen 2010-07-15 03:39:32 PDT
Created attachment 61632 [details]
Patch

Tested on Safari, Qt, Gtk and "manually" on Chromium (nightly build). I'm hoping the bot will catch missing properties, if any.
Comment 2 Kent Hansen 2010-07-19 05:12:16 PDT
Created attachment 61940 [details]
Patch v2 (skip jscprint property)

Identical to first patch, except "jscprint" has been added to the set of properties to skip since it only shows up in debug builds. I forgot to test that configuration in the first iteration.
Comment 3 Kent Hansen 2010-07-20 03:58:36 PDT
Tested on Chromium (Linux/Release) too now.
Comment 4 Sam Weinig 2010-07-21 08:14:26 PDT
Comment on attachment 61940 [details]
Patch v2 (skip jscprint property)

r=me
Comment 5 Kent Hansen 2010-07-22 01:55:42 PDT
Committed r63880: <http://trac.webkit.org/changeset/63880>