search

web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
5k stars 3.1k forks source link

Fix slow tests #11571

Open zcorpan opened 6 years ago

zcorpan commented 6 years ago

From https://github.com/web-platform-tests/wpt/issues/9972#issuecomment-398058137

zcorpan commented 6 years ago

20.2750 websockets/keeping-connection-open/001.html 20.0833 websockets/keeping-connection-open/001.html?wss

This takes 20s by design; the only fix would be to remove the test.

It doesn't show up in https://bocoup.github.io/wpt-disabled-tests-report/#filter=websockets%2Fkeeping-connection-open%2F001.html so I'm assuming test results are stable.

zcorpan commented 6 years ago

12.3427 webrtc/RTCDTMFSender-ontonechange-long.https.html

I think this needs to take this long for what it's testing (a bit unclear why it takes 12 seconds when the timeout is 6 seconds, though).

zcorpan commented 6 years ago

11.1061 fetch/http-cache/cc-request.html 07.1871 fetch/http-cache/freshness.html

@annevk can you split these up similarly to #11593 ?

10.7306 html/semantics/embedded-content/media-elements/video_loop_base.html 10.3728 html/semantics/embedded-content/media-elements/audio_loop_base.html

@jugglinmike these need user interaction to allow play(), and also they could use a shorter media file. Can you look into these?

10.1424 service-workers/service-worker/fetch-frame-resource.https.html

@wanderview can you take a stab at splitting this test?

08.2459 html/semantics/scripting-1/the-script-element/async_007.htm 08.1276 html/semantics/scripting-1/the-script-element/async_010.htm 05.5754 html/semantics/scripting-1/the-script-element/execution-timing/078.html 05.1825 html/semantics/scripting-1/the-script-element/async_005.htm 03.0509 html/semantics/scripting-1/the-script-element/execution-timing/015a.html

@foolip can you take these? I think the setTimeout can be eliminated by having the last script call the "timeout" function (though it could be renamed).

07.8624 offscreen-canvas/convert-to-blob/offscreencanvas.convert.to.blob.html

I can't reproduce this taking a long time. cc @junov

07.4439 html/semantics/forms/textfieldselection/select-event.html

@domenic can you split this up using splitTestByKey similarly to #11593 (1 key per element)?

beacon/beacon-basic-formdataMax.html

I can't reproduce this as being slow. cc @Brandr0id

foolip commented 6 years ago

I'll be OOO until July 5, but I'll check back in here (some time) after I'm back.

wanderview commented 6 years ago

10.1424 service-workers/service-worker/fetch-frame-resource.https.html

I don't think splitting would help here. It only has a handful of cases and they all run in parallel.

The reason it can be slow is that it uses a 10 second setTimeout() to detect failed cross-origin window loads. AFAIK there is no cross-browser way to handle this case and the timeout was the best attempt to deal with it. If we want this test to be faster then we need another solution for detecting this case that works in all browsers.

wanderview commented 6 years ago

Also, it feels wrong to have some kind of arbitrary point where we require splitting tests. What is really driving this? It may be slower in some browsers than others, but these are not performance tests, etc.

Also, there seems to be a trend towards more new tests being written as promise_test() which serializes execution. In contrast, we could encourage folks to write async_test() instead that runs test cases in parallel. Or we should make a promise test method that runs in parallel.

Anyway, this splitting thing feels like its pointing towards some kind of infrastructure deficiency or something. It doesn't seem like splitting should be necessary or will be scalable in the long term.

zcorpan commented 6 years ago

If the test needs to take a long time, tick the checkbox and document rationale in a comment (as you did).

In #9972 @ricea said:

Chromium developers are spending a lot of time triaging test flakiness caused by tests that are simply too slow.

I'm trying to address this by investigating the slowest tests (this issue) and tests that are timing out or are flaky, as well as implementing a check to reject tests that are near their timeout. Sometimes splitting tests can lead to more stable results, sometimes not.

zcorpan commented 6 years ago

04.2130 encoding/legacy-mb-tchinese/big5/big5-encode-form-errors-extBb.html?21001-last

encoding tests have already been split up.

wanderview commented 6 years ago

Sure, but I also saw tests being split in chromium issues where the <meta name="timeout" content="long"> could have been added. I also feel like they've added a lot of serialized promise tests instead of parallel async tests. Both of those are strategies that could be pursued before test splitting.

Also, I would note test splitting creates a lot of churn in history, test expectations, and greater chance of conflict during automatic sync'ing.

domenic commented 6 years ago

@domenic can you split this up using splitTestByKey similarly to #11593 (1 key per element)?

I don't have the time to devote to this at the moment, but will be happy to review any such changes someone else is able to make.