Summary: | Web Inspector: add first network test, improve harness. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | apavlov, bweinstein, jamesr, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Pavel Feldman
2011-02-26 14:19:39 PST
Created attachment 84021 [details]
Patch
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. Created attachment 84037 [details]
Patch
Committed r79849: <http://trac.webkit.org/changeset/79849> 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! (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. |