Bug 205473 – REGRESSION: [ Mac wk2 ] http/tests/inspector/target/provisional-load-cancels-previous-load.html is a flaky failure

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Truitt Savell]

http/tests/inspector/target/provisional-load-cancels-previous-load.html

This test is a flaky failure on Mac wk2 sense around r253723

History:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Finspector%2Ftarget%2Fprovisional-load-cancels-previous-load.html

Diff:
--- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/inspector/target/provisional-load-cancels-previous-load-expected.txt
+++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/inspector/target/provisional-load-cancels-previous-load-actual.txt
@@ -1,17 +1,2 @@
+ALERT: PASS: Should receive DidCommitProvisionalTarget event 0 => 2.
 Test that two consequtive cross domain navigation requests will result in two provisional targets being created, first of which is later destroyed and the second is committed.
-
-
-== Running test suite: Target.PSON
--- Running test case: ProvisionalPagePaused
-Current target is 0.
-PASS: Should receive TargetAdded event for target 1.
-PASS: Target 1 should be provisional.
-PASS: Target 1 should be paused on start.
-PASS: Should receive TargetRemoved event for target 1
-PASS: Should receive TargetAdded event for target 2.
-PASS: Target 2 should be provisional.
-PASS: Target 2 should be paused on start.
-PASS: Should receive TargetRemoved event for target 0
-PASS: Should receive DidCommitProvisionalTarget event 0 => 2.
-PASS: Should have seen 3 different targets.
-

[Truitt Savell]

I can reproduce this failure by running the test in iterations

[Radar WebKit Bug Importer]

<rdar://problem/58093690>

[Alexey Proskuryakov]

This test was added on 2019-12-04, was green for a week, then saw various infrequent failures, and became super flaky on 2019-12-18. Perhaps https://trac.webkit.org/r253718 ?

But there may be multiple causes for the flakiness.

[Yury Semikhatsky]

Created attachment 386634 [details]
Patch

[Blaze Burg]

Comment on attachment 386634 [details]
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.

[Yury Semikhatsky]

Comment on attachment 386634 [details]
Patch

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

>> 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.

Done.

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

Done.

>> Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js:130
>> +    deferOutputUntilTestPageIsReloaded()
> 
> Are there other cases where we think this will be needed?

Yes, other navigation tests that need to log during navigation will need to suspend the output as well.

>> 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.

Yes, this is just for the testing purposes, we always want to resume in normal flow. Added semicolon.

[Yury Semikhatsky]

Created attachment 386926 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 386926 [details]
Patch for landing

Clearing flags on attachment: 386926

Committed r254111: <https://trac.webkit.org/changeset/254111>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.