Declare timeout in test and extract it into manifest

[foolip]

https://bugs.chromium.org/p/chromium/issues/detail?id=843213 is about how the information in async_test(undefined, { timeout: 10000 }) isn't machine readable, and so the test runner still can't increase the external timeout based on it.

To make that work, I think the information would have to be in <meta something=somethin> and we should extract it into the manifest for ./wpt run to use.

[foolip]

@jgraham, how do you deal with slow tests in Gecko?

[jgraham]

Setting timeouts on individual tests is deprecated and has been for years; we should remove that feature entirely.

When tests are slow I add <meta name=timeout content=long>. If that isn't enough the test needs to be split up.

[foolip]

OK, so we already have <meta name=timeout content=long>. Is that information extracted into the manifest and used by the runner already?

[jgraham]

Yes.

[foolip]

Should the per-test timeout be nuked? Nothing about var timeout = properties.timeout ? properties.timeout : settings.test_timeout looks deprecated.

[jgraham]

The indicaton that it's deprecated is that that it's not in the documentation. I admit it's not a great signal, as there are lots of other things that aren't deprecated and aren't in the documentation. Anyway I would welcome a PR to remove the feature and all the instances of per-test timeouts in existing tests.

[foolip]

Trying to throw an exception if it's used to see how many tests are affected in https://chromium-review.googlesource.com/c/chromium/src/+/1061753

But now I'm not sure I understand why testharness.js itself has any notion of timeouts at all. In Chromium's infra we in fact just ignore that and there's only an external timeout. Is there some running environment where we actually trust the browser under test to tell us about timeouts?

[jgraham]

wptrunner makes a distinction between an internal timeout ("browser still respodng enough to signal a timeout occurred; keep using the same instance for the next test") and an external timeout ("things are so broken we should kill the browser and restart"). It's possible to just have an external timeout everywhere, but the distinction is meaningful. Also it is helpful for people running tests directly in a browser to understand if they are taking too long.

[foolip]

OK, I agree that's a meaningful distinction and useful when running manually. I presume the external timeout is always a bit longer, to not make it entirely racy?

[jgraham]

Yes. We set the selenium/marionette timeout to the test timeout + 5s and the "give up waiting for the driver to return" timeout to the test timeout + 10s.

[foolip]

OK, nothing to be done here.

[Hexcles]

This matches my understanding ("timeouts in tests" are discouraged).

Shall we remove that section from https://web-platform-tests.org/writing-tests/testharness-api.html, or at least mark it as deprecated?