REGRESSION (r174642): Watchdog timer expiration reported as DumpRenderTree.app timeout instead of test failure

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

A test that calls testRunner.waitUntilDone() and doesn't call testRunner.notifyDone() before the DumpRenderTree watchdog timer expires may be incorrectly reported as a DumpRenderTree.app timeout (i.e. the process was consider unresponsive by run-webkit-tests) instead of a test failure.

[Daniel Bates]

Created attachment 243090 [details]
Patch

[Daniel Bates]

Committed r177129: <http://trac.webkit.org/changeset/177129>

[Alexey Proskuryakov]

Comment on attachment 243090 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243090&action=review

> Tools/Scripts/webkitpy/port/ios.py:89
> +            # DumpRenderTree.app waits for the WebThread to run before dumping its output. In practice
> +            # it seems sufficient to wait up to 80ms to ensure that the WebThread ran and hence output
> +            # for the test is dumped.
> +            return 80 * 1000

I don't think that this does what is says it does. This added code changes default timeout from 35 seconds to 80 seconds, and it changes slow test timeout from 175 seconds to 400 seconds.

It is not clear to me what the actual problem was. DumpRenderTree's timeout is 30 seconds, are you saying that 5 seconds is not enough for the result to propagate? That would be surprising.

[Daniel Bates]

Comment on attachment 243090 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243090&action=review

>> Tools/Scripts/webkitpy/port/ios.py:89
>> +            return 80 * 1000
> 
> I don't think that this does what is says it does. This added code changes default timeout from 35 seconds to 80 seconds, and it changes slow test timeout from 175 seconds to 400 seconds.
> 
> It is not clear to me what the actual problem was. DumpRenderTree's timeout is 30 seconds, are you saying that 5 seconds is not enough for the result to propagate? That would be surprising.

You're right! I meant to write seconds instead of milliseconds. Yes, 5 seconds wasn't enough time for the results to propagate when running 16 to 24 instances of DumpRenderTree (why?).

[Alexey Proskuryakov]

I think that it's probably not about propagation, but about returning to event loop - DumpRenderTree detects the timeout with a watchdog timer, so the timer needs to fire in order for this to work.

I don't think that enabling a huge run-webkit-tests timeout is the right answer, as that makes tests that freeze permanently take an inordinate amount of time. Instead, I'm fixing this by having both types of timeout be reported uniformly.