RESOLVED FIXED 24908
run-webkit-tests unable to set timeout manually
https://bugs.webkit.org/show_bug.cgi?id=24908
Summary run-webkit-tests unable to set timeout manually
Brian Weinstein
Reported 2009-03-27 22:52:20 PDT
run-webkit-tests is currently unable to set the timeout time through a command line argument, which can mean tests are either running for too long (multiple timeouts where the system is in a broken state), or too short (want tests to timeout more quickly, but there are some longer tests).
Attachments
Timeout able to be set as an argument (1.99 KB, patch)
2009-03-27 22:58 PDT, Brian Weinstein
darin: review+
Fix argument order, --timeout instead of --timeout-seconds (2.20 KB, patch)
2009-03-30 19:34 PDT, Brian Weinstein
aroben: review-
Allows run-webkit-tests to specify timeout (2.23 KB, patch)
2009-03-31 10:53 PDT, Brian Weinstein
darin: review+
Mark Rowe (bdash)
Comment 1 2009-03-27 22:57:58 PDT
If tests are taking so long that they're timing out then either the test needs simplified or split, the code being tested improved so that the test doesn't take so long, or the timeout raised. I'm not sure why a command-line argument would be necessary here.
Brian Weinstein
Comment 2 2009-03-27 22:58:33 PDT
Created attachment 29035 [details] Timeout able to be set as an argument
Darin Adler
Comment 3 2009-03-28 17:41:48 PDT
Comment on attachment 29035 [details] Timeout able to be set as an argument The other arguments are mostly in alphabetical order except for merge-leak-depth in the usage message. It would be good to keep them sorted. I'm not sure --timeout-seconds is better than just --timeout r=me though
Brian Weinstein
Comment 4 2009-03-30 19:34:30 PDT
Created attachment 29107 [details] Fix argument order, --timeout instead of --timeout-seconds Made the quick fixes requested by Darin
Adam Roben (:aroben)
Comment 5 2009-03-31 09:51:14 PDT
Comment on attachment 29107 [details] Fix argument order, --timeout instead of --timeout-seconds > + --timeout Sets the length of time before a test times out The Usage output should indicate that --timeout takes a parameter. It would be good for it to also say what the default value is. > + 'timeout-seconds=i' => \$timeoutSeconds, This wasn't changed to match the new name of the option. Your ChangeLog should also mention that this patch changes the default value back to 60 seconds from 15. Or your patch keep the default at 15 seconds (which might be better). r- since the code doesn't match the Usage statement.
Brian Weinstein
Comment 6 2009-03-31 10:53:26 PDT
Created attachment 29125 [details] Allows run-webkit-tests to specify timeout Fixed silly mistakes in last patch.
Adam Roben (:aroben)
Comment 7 2009-04-07 15:24:03 PDT
Landed in r42292
Note You need to log in before you can comment on or make changes to this bug.