[webkit-reviews] review denied: [Bug 32596] Refactor Window object tests to use a shared list of property names : [Attachment 45055] Revised patch (use "var" keyword, update LayoutTests/platform/mac/objc-wrapper-identity)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Kent Hansen <kent.hansen at nokia.com>'s
request for review:
Bug 32596: Refactor Window object tests to use a shared list of property names
https://bugs.webkit.org/show_bug.cgi?id=32596

Attachment 45055: Revised patch (use "var" keyword, update
LayoutTests/platform/mac/objc-wrapper-identity)
https://bugs.webkit.org/attachment.cgi?id=45055&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
There's a problem with adding these "<script>" elements into automatically
generated test wrappers such as wrapper-identity.html. The next time someone
runs make-script-test-wrappers they will be overwritten. It would be better to
add JavaScript code to load the script element into the script-tests files. Or
you could add all of these into the exception list in the
make-script-test-wrappers perl script -- that seems like an ugly solution.

I'm not sure that automatically generating static-window-properties.js would be
a big win. Many test results still need to be updated any time the list
changes, so generating that file automatically would just reduce the number of
files to edit by one.

I think you should change fast/dom/Window/window-properties so that it there is
some text in the test suggesting an update of static-window-properties.js.
Someone might overlook that comment, but having the comment there gives them a
fighting chance. And the comment should be in the output too so it's in the
expected file as well.

review- because this breaks make-script-test-wrappers -- this otherwise seems
ready to go