[webkit-reviews] review granted: [Bug 205473] REGRESSION: [ Mac wk2 ] http/tests/inspector/target/provisional-load-cancels-previous-load.html is a flaky failure : [Attachment 386634] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 205473: REGRESSION: [ Mac wk2 ]
http/tests/inspector/target/provisional-load-cancels-previous-load.html is a
flaky failure
https://bugs.webkit.org/show_bug.cgi?id=205473

Attachment 386634: Patch

https://bugs.webkit.org/attachment.cgi?id=386634&action=review




--- Comment #5 from Brian Burg <bburg at apple.com> ---
Comment on attachment 386634
  --> https://bugs.webkit.org/attachment.cgi?id=386634
Patch

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

r=me with some comments

> Source/WebInspectorUI/ChangeLog:8
> +

May want to clarify that the flaky failure is due to a problem with the test
itself, not the code under test.

> Source/WebInspectorUI/ChangeLog:12
> +	   can be overriden in the tests.

Nit: overridden

> Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js:130
> +    deferOutputUntilTestPageIsReloaded()

Are there other cases where we think this will be needed?

>
LayoutTests/http/tests/inspector/target/provisional-load-cancels-previous-load.
html:23
> +	       WI.Target.prototype._resumeIfPaused = () => {}

It is kind of weird to monkey patch like this, but I suppose it's just for a
test.

Nit: need trailing semicolon.