Bug 147231

Summary: Web Inspector: rewrite inspector-protocol/console tests to use new testing patterns
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 147093    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Brian Burg 2015-07-23 12:25:26 PDT
These are currently poorly structured. This is also a simple test case for improving the way that tests listen for events (both with and without promises).
Comment 1 Radar WebKit Bug Importer 2015-07-23 12:25:47 PDT
<rdar://problem/21966884>
Comment 2 Brian Burg 2015-07-23 12:34:44 PDT
Created attachment 257362 [details]
Patch
Comment 3 Joseph Pecoraro 2015-07-23 14:15:26 PDT
Comment on attachment 257362 [details]
Patch

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

r=me

> LayoutTests/ChangeLog:29
> +        * inspector-protocol/console/console-message-expected.txt:
> +        * inspector-protocol/console/console-message.html:

I'd rather we just favor the LayoutTest/inspector/console tests, and remove these. But maybe I'll too idealistic.

> LayoutTests/http/tests/inspector-protocol/resources/InspectorTest.js:42
> +    if (typeof methodOrObject === "object") {
> +        var {method, params, handler} = methodOrObject;
> +    }

Style: braces

> LayoutTests/http/tests/inspector-protocol/resources/InspectorTest.js:91
> +    else if (typeof listeners == "function")

Style: ===

> LayoutTests/inspector-protocol/console/console-message-expected.txt:8
> +Running test case: ConsoleLogString

Would be nice to output newlines before running test cases to break the output up a bit and make it more readable.
Comment 4 Brian Burg 2015-07-23 14:31:07 PDT
Comment on attachment 257362 [details]
Patch

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

>> LayoutTests/ChangeLog:29
>> +        * inspector-protocol/console/console-message.html:
> 
> I'd rather we just favor the LayoutTest/inspector/console tests, and remove these. But maybe I'll too idealistic.

I'm coming around to your point of view. However, there are some hardcoded path comparisons in DRT/WKTR that decide whether to use the stub or full frontend based on where the test lives. So this would be a separate patch.

>> LayoutTests/inspector-protocol/console/console-message-expected.txt:8
>> +Running test case: ConsoleLogString
> 
> Would be nice to output newlines before running test cases to break the output up a bit and make it more readable.

Agree, might as well make the change now before it spreads.
Comment 5 Brian Burg 2015-07-23 15:00:13 PDT
Created attachment 257389 [details]
Patch for landing
Comment 6 Joseph Pecoraro 2015-07-23 15:32:55 PDT
Comment on attachment 257389 [details]
Patch for landing

r=me
Comment 7 WebKit Commit Bot 2015-07-23 16:21:53 PDT
Comment on attachment 257389 [details]
Patch for landing

Clearing flags on attachment: 257389

Committed r187269: <http://trac.webkit.org/changeset/187269>
Comment 8 WebKit Commit Bot 2015-07-23 16:21:57 PDT
All reviewed patches have been landed.  Closing bug.