Bug 148077 - Web Inspector: split TestStub.js into multiple files and modernize it
Summary: Web Inspector: split TestStub.js into multiple files and modernize it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on: 147955 148122
Blocks: 147934
  Show dependency treegraph
 
Reported: 2015-08-17 08:55 PDT by BJ Burg
Modified: 2015-08-19 09:14 PDT (History)
10 users (show)

See Also:


Attachments
Proposed Fix (44.94 KB, patch)
2015-08-17 09:05 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Proposed Fix (73.00 KB, patch)
2015-08-17 10:13 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Make sure non-Mac works (84.11 KB, patch)
2015-08-17 14:15 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Proposed Fix (with arrows) (91.11 KB, patch)
2015-08-18 16:03 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (544.38 KB, application/zip)
2015-08-18 16:36 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (597.36 KB, application/zip)
2015-08-18 16:40 PDT, Build Bot
no flags Details
Proposed Fix (with arrows) (91.10 KB, patch)
2015-08-18 17:44 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Proposed Fix (with arrows) (91.01 KB, patch)
2015-08-18 21:58 PDT, BJ Burg
timothy: review+
bburg: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2015-08-17 08:55:05 PDT
,
Comment 1 Radar WebKit Bug Importer 2015-08-17 08:55:27 PDT
<rdar://problem/22307794>
Comment 2 BJ Burg 2015-08-17 09:05:21 PDT
Created attachment 259153 [details]
Proposed Fix

This patch renames some things that were moved around in the dependent patch, so it won't apply.
Comment 3 BJ Burg 2015-08-17 10:13:33 PDT
Created attachment 259158 [details]
Proposed Fix
Comment 4 BJ Burg 2015-08-17 14:15:37 PDT
Created attachment 259182 [details]
Make sure non-Mac works
Comment 5 BJ Burg 2015-08-17 14:52:00 PDT
Committed r188544: <http://trac.webkit.org/changeset/188544>
Comment 6 WebKit Commit Bot 2015-08-17 23:29:45 PDT
Re-opened since this is blocked by bug 148122
Comment 7 Alexey Proskuryakov 2015-08-17 23:35:44 PDT
Rolled out because this depended on r188539.
Comment 8 BJ Burg 2015-08-18 16:03:03 PDT
Created attachment 259307 [details]
Proposed Fix (with arrows)

Since I have to re-land this patch, I tried adding arrows to the affected code. Please re-review for style, and I'll summarize some things I ran into for the style guide.
Comment 9 Build Bot 2015-08-18 16:36:15 PDT
Comment on attachment 259307 [details]
Proposed Fix (with arrows)

Attachment 259307 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/73301

New failing tests:
inspector/dom/dom-search.html
inspector/dom/dom-search-with-context.html
Comment 10 Build Bot 2015-08-18 16:36:18 PDT
Created attachment 259316 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 11 Build Bot 2015-08-18 16:40:13 PDT
Comment on attachment 259307 [details]
Proposed Fix (with arrows)

Attachment 259307 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/73306

New failing tests:
inspector/dom/dom-search.html
inspector/dom/dom-search-with-context.html
Comment 12 Build Bot 2015-08-18 16:40:16 PDT
Created attachment 259318 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 13 BJ Burg 2015-08-18 17:44:01 PDT
Created attachment 259333 [details]
Proposed Fix (with arrows)
Comment 14 BJ Burg 2015-08-18 21:58:36 PDT
Created attachment 259352 [details]
Proposed Fix (with arrows)
Comment 15 Timothy Hatcher 2015-08-19 01:54:13 PDT
Comment on attachment 259352 [details]
Proposed Fix (with arrows)

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

> Source/WebInspectorUI/UserInterface/Test/InspectorProtocol.js:139
> +    // Otherwise, it is an event.
> +    } else {

I'd flip the order of these lines and indent the comment one more level.

> Source/WebInspectorUI/UserInterface/Test/InspectorProtocol.js:150
> +            handler.map(function(listener) {
> +                listener.call(null, messageObject);
> +            });

Arrow.

> Source/WebInspectorUI/UserInterface/Test/Test.js:234
> +        .then(function() {

Arrow?
Comment 16 BJ Burg 2015-08-19 08:41:47 PDT
Comment on attachment 259352 [details]
Proposed Fix (with arrows)

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

>> Source/WebInspectorUI/UserInterface/Test/Test.js:234
>> +        .then(function() {
> 
> Arrow?

This file gets rewritten in the next patch, so not fixing here.
Comment 17 BJ Burg 2015-08-19 09:14:32 PDT
Committed r188639: <http://trac.webkit.org/changeset/188639>