[webkit-reviews] review denied: [Bug 54459] Web Inspector: implement DOM agent tests. : [Attachment 82457] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 15 08:33:12 PST 2011
Yury Semikhatsky <yurys at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 54459: Web Inspector: implement DOM agent tests.
https://bugs.webkit.org/show_bug.cgi?id=54459
Attachment 82457: Patch
https://bugs.webkit.org/attachment.cgi?id=82457&action=review
------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82457&action=review
> LayoutTests/http/tests/inspector/elements-test.js:164
> + if (children.length)
This will fail if children is undefined. Prefer early return when children is
undefined above. r- for this.
> LayoutTests/http/tests/inspector/inspector-test.js:142
> + if (/^test/.exec(symbol) && typeof testSuite[symbol] === "function")
test() should be used instead of exec() here.
> LayoutTests/http/tests/inspector/inspector-test.js:153
> + testSuiteTests.splice(0, 1);
Just use Array.shift() instead of these two lines.
> LayoutTests/inspector/elements/insert-node-collapsed.html:23
> + function callback(node)
You shouldn't rely on the order of the properties in for/in loop. If you need
the tests to be executed in a particular order you should use array instead of
object.
> LayoutTests/inspector/elements/mutate-unknown-node.html:31
> + InspectorTest.addResult("DOMAgent event fired. Should only happen
once for output node: " + type + " " + event.target.nodeName + "#" +
event.target.getAttribute("id"));
Please, move description of correct result to the static section below in the
body.
More information about the webkit-reviews
mailing list