Bug 139734 – REGRESSION (r177363): Gtk and Efl testing is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Alexey Proskuryakov]

1) On the GTK port, the timeout is 6000ms (release), 12000ms (debug) or 60000ms (if checking memory leaks with valgrind)
 http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/gtk.py?rev=177363#L83

So, subtracting 5 seconds is not ok for this port.

 2) r177363 breakes all ports that don't enable supports_per_test_timeout() on Tools/Scripts/webkitpy/port/$port.py
 This is because m_timeout is initialized to 0 <http://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/TestInvocation.cpp?rev=177363#L96>, and never updated with the port default value because the python scripts don't pass the --timeout argument when the previous is not enabled.

[Alexey Proskuryakov]

Created attachment 243442 [details]
proposed fix

[Carlos Alberto Lopez Perez]

Comment on attachment 243442 [details]
proposed fix

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

> Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:73
> +        elif self._timeout > 5:

Shouldn't this be 5000 instead of 5? Otherwise we could end with negative values.

> Tools/Scripts/webkitpy/port/gtk.py:95
>  

I would suggest adding 6000 and 2000 (that is, 1 second more than what later is going to be substracted).
Otherwise the timeouts for WTRK and run-webkit-test are going to be the same (and therefore the race condition we talked before would happen). Or I'm wrong?

[Alexey Proskuryakov]

> Shouldn't this be 5000 instead of 5? Otherwise we could end with negative values.

Oops, thanks for catching that.

> I would suggest adding 6000 and 2000 (that is, 1 second more than what later is going to be substracted).
> Otherwise the timeouts for WTRK and run-webkit-test are going to be the same (and therefore the race condition we talked before would happen). Or I'm wrong?

Hmm, let me double check the flow of the timeout value.

[Alexey Proskuryakov]

Created attachment 243448 [details]
proposed fix

You are right, and I didn't realize that the script timeout was passed through the driver. The correct solution is to pass both timeouts as part of DriverInput. That would likely require updating regression tests, and I'm out of time for further refactoring today, so let's just do the hacky fix.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r177456>.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 139749

[Alexey Proskuryakov]

Created attachment 243462 [details]
proposed fix

I missed an important part in the original patch; not sure how tests almost worked.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r177471>.