Bug 24908 - run-webkit-tests unable to set timeout manually
Summary: run-webkit-tests unable to set timeout manually
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-27 22:52 PDT by Brian Weinstein
Modified: 2009-04-07 15:24 PDT (History)
2 users (show)

See Also:


Attachments
Timeout able to be set as an argument (1.99 KB, patch)
2009-03-27 22:58 PDT, Brian Weinstein
darin: review+
Details | Formatted Diff | Diff
Fix argument order, --timeout instead of --timeout-seconds (2.20 KB, patch)
2009-03-30 19:34 PDT, Brian Weinstein
aroben: review-
Details | Formatted Diff | Diff
Allows run-webkit-tests to specify timeout (2.23 KB, patch)
2009-03-31 10:53 PDT, Brian Weinstein
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Weinstein 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).
Comment 1 Mark Rowe (bdash) 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.
Comment 2 Brian Weinstein 2009-03-27 22:58:33 PDT
Created attachment 29035 [details]
Timeout able to be set as an argument
Comment 3 Darin Adler 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
Comment 4 Brian Weinstein 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
Comment 5 Adam Roben (:aroben) 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.
Comment 6 Brian Weinstein 2009-03-31 10:53:26 PDT
Created attachment 29125 [details]
Allows run-webkit-tests to specify timeout

Fixed silly mistakes in last patch.
Comment 7 Adam Roben (:aroben) 2009-04-07 15:24:03 PDT
Landed in r42292