Bug 207944 – (r256583) [ iOS ] http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html is a flaky timeout

• Component: New Bugs
• Status: RESOLVED FIXED

[Truitt Savell]

http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html

This test was introduced in https://trac.webkit.org/changeset/256583/webkit

and is a flaky timeout sense introduction. there is also at least one crash in history. 

history:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2FresourceLoadStatistics%2Fprevalent-domains-per-page-database.html

[Radar WebKit Bug Importer]

<rdar://problem/59592361>

[Kate Cheney]

Created attachment 391289 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 391289 [details]
Patch

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

r=ews

> LayoutTests/http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html:20
> +                if (arrayOfDomains.length == 0) {
> +                    askForPrevalentResources();
> +                    return;

This is a tight loop, which is unfriendly to other tests running in parallel. Maybe do it on a 100 ms timer?

> LayoutTests/http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html:25
>                      testFailed("Domain was not successfully marked prevalent.");

Does this become dead code now, as we'll keep retrying until timeout? Maybe it's worth to limit retries to say 20 seconds, because timeout is not a very clear failure mode - we often assume that those are not real failures.

[Kate Cheney]

Comment on attachment 391289 [details]
Patch

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

>> LayoutTests/http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html:20
>> +                    return;
> 
> This is a tight loop, which is unfriendly to other tests running in parallel. Maybe do it on a 100 ms timer?

Sounds good, I'll add this.

>> LayoutTests/http/tests/resourceLoadStatistics/prevalent-domains-per-page-database.html:25
>>                      testFailed("Domain was not successfully marked prevalent.");
> 
> Does this become dead code now, as we'll keep retrying until timeout? Maybe it's worth to limit retries to say 20 seconds, because timeout is not a very clear failure mode - we often assume that those are not real failures.

Do you have an example of another test which does this? I've only seen retrying for a certain number of iterations, not based on time.

[Alexey Proskuryakov]

Yes, we have some tests with a setTimeout() for 20 seconds that prints a failure message and calls notifyDone(). We also have some that limit the number of iterations, but I don't know if 200 iterations 100 ms each are guaranteed to complete in a time that's close enough to 20 seconds. Probably not.

[Kate Cheney]

Created attachment 391310 [details]
Patch

[Kate Cheney]

Letting ews run.

[Kate Cheney]

This test uses UIHelper.activateAt(), which doesn't produce a user gesture that ITP reliably captures on iOS. This is a problem we’ve faced before with ITP tests, and our solution for now is to skip these tests on iOS.

[Kate Cheney]

Committed: https://trac.webkit.org/r257100