Bug 26917 – Increase delay in worker-terminate layout test to make it more reliable.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dmitry Titov]

The worker-terminate tests starts workers that replay with a message to a message (like echo), then posts a message to them, immediately calls 'worker.terminate()' and then waits for a while to check that after calling 'terminate()' there is nothing coming back from the worker (it was terminated). To make sure the worker started at all, before 'terminate()' it may wait for the first message to come back (and be captured in results).

Current timeout for create a worker -> postMessage('ping1') -> wait for 'pong' from the worker -> call terminate() -> postMessage('ping2') - wait to see that there is no 'pong2' is set to 500ms.

On debug builds (of multi-process Chromium) and virtual test machines the time until first pong returns is roughly 500ms these days. So the test is flakey in some test environments, giving a fals negative when the first 'pong' is absent - and it doesn't even start to test that the 'pong2' does not come during some time after since it exits.

It is better to avoid such delay-based tests in general. In this case, I can't think of a way to detect the termination of the worker other then to verify it doesn't return messages anymore...

Proposed patch moves start of the timeout after 'pong1' and increases the timeout 2x to make test more reliable.

[Dmitry Titov]

Created attachment 32169 [details]
Proposed patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 32169 [details]
Proposed patch

R=me, but it sure would be nice if there were a way to construct a
test that wasn't timing sensitive.  Adding delays to the layout
tests just slows down running all of the layout tests, and if 500ms
wasn't enough, then sometimes (maybe only rarely) 1000ms will also
not be enough.

[Dmitry Titov]

(In reply to comment #2)
> (From update of attachment 32169 [details] [review])
> R=me, but it sure would be nice if there were a way to construct a
> test that wasn't timing sensitive.

Absolutely. There is a separate discussion on how to test termination of workers (dedicated and shared) - seems we'll need to implement additional API on layoutTestController since the GC and actual destruction of workers is intentionally designed to be undetectable from a 'normal' page.

[Eric Seidel (no email)]

Do we have a follow-up bug about termination testing?

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/45533