Bug 54459 - Web Inspector: implement DOM agent tests.
Summary: Web Inspector: implement DOM agent tests.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-15 07:13 PST by Pavel Feldman
Modified: 2011-02-15 10:21 PST (History)
13 users (show)

See Also:


Attachments
Patch (40.46 KB, patch)
2011-02-15 08:07 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (40.27 KB, patch)
2011-02-15 08:49 PST, Pavel Feldman
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-02-15 07:13:21 PST
Patch to follow.
Comment 1 Pavel Feldman 2011-02-15 08:07:07 PST
Created attachment 82457 [details]
Patch
Comment 2 Yury Semikhatsky 2011-02-15 08:33:13 PST
Comment on attachment 82457 [details]
Patch

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.
Comment 3 Pavel Feldman 2011-02-15 08:40:10 PST
(In reply to comment #2)
> (From update of attachment 82457 [details])
> 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.
> 

Done.

> > LayoutTests/http/tests/inspector/inspector-test.js:142
> > +        if (/^test/.exec(symbol) && typeof testSuite[symbol] === "function")
> 
> test() should be used instead of exec() here.
> 

Done.

> > LayoutTests/http/tests/inspector/inspector-test.js:153
> > +        testSuiteTests.splice(0, 1);
> 
> Just use Array.shift() instead of these two lines.
> 

Done.

> > 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.
> 

This is a de-facto standard tested in many javascript layout tests.

> > 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.

I think it reflects what is happening now better this way.
Comment 4 Pavel Feldman 2011-02-15 08:49:27 PST
Created attachment 82465 [details]
Patch
Comment 5 Pavel Feldman 2011-02-15 09:41:40 PST
Committed r78576: <http://trac.webkit.org/changeset/78576>
Comment 6 WebKit Review Bot 2011-02-15 10:21:44 PST
http://trac.webkit.org/changeset/78576 might have broken Qt Linux Release
The following tests are not passing:
inspector/elements/mutate-unknown-node.html