Bug 55306 – Web Inspector: add first network test, improve harness.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Pavel Feldman]

Patch to follow.

[Pavel Feldman]

Created attachment 84021 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 84021 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84021&action=review

> LayoutTests/http/tests/inspector/inspector-test.js:308
>      layoutTestController.closeWebInspector();

Please delete watchdog timer here to make sure we don't close inspector twice.

> LayoutTests/http/tests/inspector/network/network-size.html:33
> +        var resource1 = WebInspector.panels.network.resources[0];

Image order might change. Can you sort the results to avoid inconsistent output? r- for this.

> LayoutTests/http/tests/inspector/network/network-timing.html:34
> +        InspectorTest.addResult(resource1.url);

Results' order should not depend on network activity.

> LayoutTests/http/tests/inspector/network/network-timing.html:36
> +        InspectorTest.assertGreaterOrEqual(300, resource1.duration * 1000, "Duration of the first resource");

I don't like that we deliberately slow down layout tests. Can we use mocks/something else to avoid this. Inspector tests are already notorious for being quite slow.

> LayoutTests/http/tests/inspector/network/network-timing.html:41
> +        InspectorTest.assertGreaterOrEqual(100, resource2.latency * 1000, "Latency of the second resource");

I think we should print something in case of success.

> LayoutTests/http/tests/inspector/network/resources/resource.php:42
> +    var now = new Date();

Just use Date.now(). This way you won't need Number(now) cast below.

> LayoutTests/http/tests/inspector/network/resources/resource.php:55
> +            echo("function foo() {}");

Could be rewritten as
if (!$jscontent)
    $jscontent = "function foo() {}";
and use same logic for both branches.

[Pavel Feldman]

Created attachment 84037 [details]
Patch

[Pavel Feldman]

Committed r79849: <http://trac.webkit.org/changeset/79849>

[James Robinson]

This watchdog is evil, evil, evil, evil!  It prevents us from using slow/timeout/etc expectations as we want.

You should _never_ need to do your own watchdog timer for layout tests, the layout test harness has a timeout per test that we can modify as needed (for example SLOW modifiers, debug/release tweaks, TIMEOUT expectations).  Can you please remove this?  Otherwise we'll have to =TEXT a lot of slow-running inspector tests - which is a whole lot of them in debug!

[Pavel Feldman]

(In reply to comment #5)
> This watchdog is evil, evil, evil, evil!  It prevents us from using slow/timeout/etc expectations as we want.

Ok, ok. We need this for debugging mostly because we are special. Inspector tests are happening in the front-end mostly, and when there is a problem, we end up timed out. In that case, not all the outputs are flushed into the page, so I added a watchdog doing that. I did not think of SLOW.

Let me follow up and make sure watchdog is flushing, but not notifying done. As a result we all will be happy.