Bug 106231 - Web Inspector: Refactor InspectorTest to create output interface
Summary: Web Inspector: Refactor InspectorTest to create output interface
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: 2013-01-07 10:22 PST by johnjbarton
Modified: 2013-01-10 10:27 PST (History)
10 users (show)

See Also:


Attachments
Patch (28.82 KB, patch)
2013-01-07 10:34 PST, johnjbarton
no flags Details | Formatted Diff | Diff
Patch (3.36 KB, patch)
2013-01-09 11:38 PST, johnjbarton
no flags Details | Formatted Diff | Diff
Patch (3.37 KB, patch)
2013-01-10 09:21 PST, johnjbarton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description johnjbarton 2013-01-07 10:22:59 PST
InspectorTest is complex and hard to understand. The StandaloneTestrunnerStub (related to bug 92429) helped clarify the components. Along the same lines, separating the output functions, ie the functions used by InspectorTest to communicate test results and completion would help. (Also the code involved is incorrectly formatted).

Patch ready.
Comment 1 johnjbarton 2013-01-07 10:34:56 PST
Created attachment 181523 [details]
Patch
Comment 2 johnjbarton 2013-01-07 10:35:46 PST
Hmm, the patch is hard to read, let me know if you want the re-indent separate.
Comment 3 WebKit Review Bot 2013-01-07 11:17:58 PST
Comment on attachment 181523 [details]
Patch

Attachment 181523 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15731857

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 4 johnjbarton 2013-01-07 11:38:11 PST
(In reply to comment #3)
> New failing tests:
> inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html


Passes for me on linux. output says DumpRenderTree crashed?
Comment 5 Pavel Feldman 2013-01-09 05:49:15 PST
Comment on attachment 181523 [details]
Patch

We don't really indent within the namespaces to save some horizontal space. Could you split (or drop) the formatting and upload the refactoring changes only? Otherwise it is really hard to follow them.
Comment 6 johnjbarton 2013-01-09 11:38:43 PST
Created attachment 181959 [details]
Patch
Comment 7 johnjbarton 2013-01-09 11:39:39 PST
Re-indent removed to bug 106475
Comment 8 Pavel Feldman 2013-01-10 05:46:49 PST
Comment on attachment 181959 [details]
Patch

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

> LayoutTests/http/tests/inspector/inspector-test.js:20
> +    addResult: function(text) 

Please add blank line between methods.
Comment 9 johnjbarton 2013-01-10 09:21:50 PST
Created attachment 182154 [details]
Patch
Comment 10 WebKit Review Bot 2013-01-10 10:27:04 PST
Comment on attachment 182154 [details]
Patch

Clearing flags on attachment: 182154

Committed r139332: <http://trac.webkit.org/changeset/139332>
Comment 11 WebKit Review Bot 2013-01-10 10:27:08 PST
All reviewed patches have been landed.  Closing bug.