26917 2009-07-01 22:24:39 -0700 Increase delay in worker-terminate layout test to make it more reliable. 2009-07-03 04:01:19 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 dimich webkit-unassigned ap levin oldest_to_newest 129459 0 dimich 2009-07-01 22:24:39 -0700 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. 129460 1 32169 dimich 2009-07-01 22:29:39 -0700 Created attachment 32169 Proposed patch 129510 2 32169 fishd 2009-07-02 09:11:01 -0700 Comment on attachment 32169 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. 129514 3 dimich 2009-07-02 09:39:26 -0700 (In reply to comment #2) > (From update of attachment 32169 [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. 129544 4 eric 2009-07-02 12:17:24 -0700 Do we have a follow-up bug about termination testing? 129701 5 dimich 2009-07-03 04:01:19 -0700 Landed: http://trac.webkit.org/changeset/45533 32169 2009-07-01 22:29:39 -0700 2009-07-02 09:11:01 -0700 Proposed patch patch.txt text/plain 1527 dimich ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCAwMzU2ZjNiLi5kZDQxMjQ0IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMDktMDct MDEgIERtaXRyeSBUaXRvdiAgPGRpbWljaEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW5jcmVhc2UgZGVsYXkgaW4gd29ya2Vy LXRlcm1pbmF0ZSBsYXlvdXQgdGVzdCB0byBtYWtlIGl0IG1vcmUgcmVsaWFibGUuCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNjkxNworCisgICAgICAg IFRoaXMgdGVzdCB1c2VkIHRpbWUgZGVsYXkgdG8gc2VlIG9mIHRlcm1pbmF0ZWQgd29ya2VyIGRv ZXMgbm90IHJldHVybgorICAgICAgICBtZXNzYWdlcyAodGVybWluYXRlZCkuIEluIHNvbWUgdGVz dCBjb25kaXRpb25zLCB0aGUgY3VycmVudCBkZWxheSAoNTAwbXMpCisgICAgICAgIGlzIHRvbyBz aG9ydCBhbmQgdGVzdCBmYWlscy4gTW92ZSBzZXR0aW5nIHRoZSBkZWxheSB0byB0aGUgZW5kIG9m IGluaXRpYWxpemF0aW9uCisgICAgICAgIGFuZCBpbmNyZWFzZSBpdCAyeCB0byBtYWtlIHRlc3Qg bW9yZSByZWxpYWJsZS4KKworICAgICAgICAqIGZhc3Qvd29ya2Vycy93b3JrZXItdGVybWluYXRl Lmh0bWw6CisKIDIwMDktMDctMDEgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5j b20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFuIEJlcm5zdGVpbgpkaWZmIC0tZ2l0IGEvTGF5 b3V0VGVzdHMvZmFzdC93b3JrZXJzL3dvcmtlci10ZXJtaW5hdGUuaHRtbCBiL0xheW91dFRlc3Rz L2Zhc3Qvd29ya2Vycy93b3JrZXItdGVybWluYXRlLmh0bWwKaW5kZXggMGNjZjIyMi4uNjMxNjQw YSAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvZmFzdC93b3JrZXJzL3dvcmtlci10ZXJtaW5hdGUu aHRtbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L3dvcmtlcnMvd29ya2VyLXRlcm1pbmF0ZS5odG1s CkBAIC00MywxMSArNDMsMTAgQEAgd29ya2VyNC5vbm1lc3NhZ2UgPSBmdW5jdGlvbihldnQpIHsK ICAgICB3b3JrZXI0LnBvc3RNZXNzYWdlKCJldmFsIHRydWUiKTsKICAgICB3b3JrZXI0LnRlcm1p bmF0ZSgpOwogICAgIGxvZygiUmVjZWl2ZWQgbWVzc2FnZSBmcm9tIHdvcmtlcjQuIik7CisgICAg c2V0VGltZW91dCgibG9nKCdET05FJyk7IGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIp IGxheW91dFRlc3RDb250cm9sbGVyLm5vdGlmeURvbmUoKSIsIDEwMDApOwogfQogd29ya2VyNC5w b3N0TWVzc2FnZSgiZXZhbCB0cnVlIik7CiAKLXNldFRpbWVvdXQoImxvZygnRE9ORScpOyBpZiAo d2luZG93LmxheW91dFRlc3RDb250cm9sbGVyKSBsYXlvdXRUZXN0Q29udHJvbGxlci5ub3RpZnlE b25lKCkiLCA1MDApOwotCiA8L3NjcmlwdD4KIDwvYm9keT4KIDwvaHRtbD4K