Bug 162072 – Web Inspector: adopt Object.awaitEvent in LayoutTests/inspector/debugger

• Status: NEW
• Component: Web Inspector

[Devin Rousso]

.

[Radar WebKit Bug Importer]

<rdar://problem/28340067>

[Devin Rousso]

Created attachment 289131 [details]
Patch

[Build Bot]

Comment on attachment 289131 [details]
Patch

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

New failing tests:
inspector/debugger/break-on-exception-throw-in-promise.html

[Build Bot]

Created attachment 289150 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 289131 [details]
Patch

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

New failing tests:
inspector/debugger/break-on-exception-throw-in-promise.html

[Build Bot]

Created attachment 289151 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Blaze Burg]

Comment on attachment 289131 [details]
Patch

Please update test expectations as [Slow] for break-on-exception-throw-in-promise.html and resubmit to EWS. It might just be taking too long on these ports.

[Blaze Burg]

Comment on attachment 289131 [details]
Patch

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

> LayoutTests/inspector/debugger/break-on-exception-throw-in-promise.html:23
>                      InspectorTest.expectThat(WebInspector.debuggerManager.pauseReason === "exception", "Should pause for exception.");

You may want to sprinkle the magic to log protocol traffic or add sync logging so we can figure out why this dies.

[Joseph Pecoraro]

Comment on attachment 289131 [details]
Patch

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

> LayoutTests/inspector/debugger/break-on-uncaught-exception-throw-in-promise.html:40
> +                InspectorTest.evaluateInPage(expression);
> +                InspectorTest.evaluateInPage(`setTimeout(() => { TestPage.dispatchEventToFrontend("AfterTestFunction"); }, 50)`);

I find this reordering, in all tests, harder to read the tests. Is it necessary?

[Devin Rousso]

Comment on attachment 289131 [details]
Patch

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

>> LayoutTests/inspector/debugger/break-on-exception-throw-in-promise.html:23
>>                      InspectorTest.expectThat(WebInspector.debuggerManager.pauseReason === "exception", "Should pause for exception.");
> 
> You may want to sprinkle the magic to log protocol traffic or add sync logging so we can figure out why this dies.

While I do love your verbiage, I have no idea what you mean.  Can you explain?  Also, how do I mark a test as "slow"?

>> LayoutTests/inspector/debugger/break-on-uncaught-exception-throw-in-promise.html:40
>> +                InspectorTest.evaluateInPage(`setTimeout(() => { TestPage.dispatchEventToFrontend("AfterTestFunction"); }, 50)`);
> 
> I find this reordering, in all tests, harder to read the tests. Is it necessary?

Really?  I find that having the setup be before the call makes more sense.  In WebInspector code we typically create functions before they are used (not for prototype functions though).

[Joseph Pecoraro]

(In reply to comment #10)
> Comment on attachment 289131 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289131&action=review
> 
> >> LayoutTests/inspector/debugger/break-on-exception-throw-in-promise.html:23
> >>                      InspectorTest.expectThat(WebInspector.debuggerManager.pauseReason === "exception", "Should pause for exception.");
> > 
> > You may want to sprinkle the magic to log protocol traffic or add sync logging so we can figure out why this dies.
> 
> While I do love your verbiage, I have no idea what you mean.

We recently added:

    InspectorTest.debug()

Or:

    ProtocolTest.debug()

Which is now all you need to do to get the test to dump all the protocol traffic to stderr so it can be debugged when a test fails.


> Also, how do I mark a test as "slow"?

In the LayoutTests/TestExpectations you can add a line that has a grammar like:

    <bugzilla-link>? <release-type>? <test-path> <other-flags>?

In this case some examples of possible lines would be:

    inspector/debugger/break-on-exception-throw-in-promise.html [ Slow ]

    [ Debug ] inspector/debugger/break-on-exception-throw-in-promise.html [ Slow ]

    webkit.org/b/### [ Debug ] inspector/debugger/break-on-exception-throw-in-promise.html [ Slow ]

The later lines are all "way cooler" than the earlier ones.