Bug 26917 - Increase delay in worker-terminate layout test to make it more reliable.
Summary: Increase delay in worker-terminate layout test to make it more reliable.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-01 22:24 PDT by Dmitry Titov
Modified: 2009-07-03 04:01 PDT (History)
2 users (show)

See Also:


Attachments
Proposed patch (1.49 KB, patch)
2009-07-01 22:29 PDT, Dmitry Titov
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Titov 2009-07-01 22:24:39 PDT
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.
Comment 1 Dmitry Titov 2009-07-01 22:29:39 PDT
Created attachment 32169 [details]
Proposed patch
Comment 2 Darin Fisher (:fishd, Google) 2009-07-02 09:11:01 PDT
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.
Comment 3 Dmitry Titov 2009-07-02 09:39:26 PDT
(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.

Comment 4 Eric Seidel (no email) 2009-07-02 12:17:24 PDT
Do we have a follow-up bug about termination testing?
Comment 5 Dmitry Titov 2009-07-03 04:01:19 PDT
Landed: http://trac.webkit.org/changeset/45533