Bug 24908

Summary:

run-webkit-tests unable to set timeout manually

Product:

WebKit

Reporter:

Brian Weinstein <bweinstein>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aroben, slewis

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Timeout able to be set as an argument	darin: review+
Fix argument order, --timeout instead of --timeout-seconds	aroben: review-
Allows run-webkit-tests to specify timeout	darin: review+

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+	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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

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.