Fatal error: Maximum execution time of 180 seconds exceeded

Since we have had issues with tests timeing out they set the timeout themseleves

The default is:

  /**
   * Time limit for the test.
   */
  protected $timeLimit = 180;

and in DrupalWebTestCase::setUp()

set_time_limit($this->timeLimit);

Seems like your machine is taking longer than usual, could be CPU speed or something related to configuration.

Seems reasonable, perhaps even a setting on setting page. Since it takes no where near that long to run on my machine I would rather it detect it quicker.

Right, this just needs a patch.

Re-rolled per: http://drupal.org/patch and cleaned up documentation.

Patch looks a bit wacky since there is double line spacing. I haven't look at it in detail.

+
+
+function simpletest_settings_form_validate($form, $form_state) {
+  $value = $form_state['values']['simpletest_time_limit'];
+  if ($value !== (string)(int)$value || $value < 0) {
+    form_error($form['general']['simpletest_time_limit'], t('Must be an integer >= 0.'));
+  }
+}
\ No newline at end of file

Should be end of line...needs documentation...and remove extra blank line from above.

Otherwise looks fine.

Simpletest calls set_time_limit() with this value to increase the PHP script exe

to

Simpletest calls set_time_limit() with this value to set the PHP script exe

I tested this with a value of 1 and it timedout....so looks good.

+++ modules/simpletest/simpletest.pages.inc	4 Aug 2009 02:26:50 -0000
@@ -449,3 +456,15 @@
+ * Validate settings form value 'simpletest_time_limit' is an integer >= 0
+ * to prevent user from entering invalid value.

Shorten so fits into one line, or add one-line summary.

+++ modules/simpletest/simpletest.pages.inc	4 Aug 2009 02:26:50 -0000
@@ -449,3 +456,15 @@
+  if ($value !== (string)(int)$value || $value < 0) {

What is that weird $value !== (string)(int)$value stuff? Isn't it enough to cast $value to an integer and ensure it's not < 0?

If not, let's comment this because it's very weird.

+++ modules/simpletest/simpletest.install	4 Aug 2009 02:26:50 -0000
@@ -132,6 +133,7 @@
+  $can_set_time_limit = !ini_get('safe_mode') && strpos(ini_get('disable_functions'), 'set_time_limit') === FALSE;

Is checking for function_exists('set_time_limit') not sufficient here?

+++ modules/simpletest/simpletest.install	4 Aug 2009 02:26:50 -0000
@@ -156,7 +158,16 @@
+    $requirements['php_set_time_limit']['description'] = $t('Simpletest cannot run in safe_mode or with function set_time_limit() disable. Correct this in php.ini');

disabled. php.ini should be followed by a period since it ends the sentence.

Beer-o-mania starts in 23 days! Don't drink and patch.

+++ modules/simpletest/simpletest.install	9 Aug 2009 07:26:03 -0000
@@ -156,7 +158,16 @@ function simpletest_requirements($phase)
+    'title' => $t('Can call %f', array('%f' => 'set_time_limit()')),

Single letter placeholders are not generally used, lets change to %function.

+++ modules/simpletest/simpletest.pages.inc	9 Aug 2009 07:26:03 -0000
@@ -453,3 +460,15 @@ function simpletest_settings_form(&$form
+  // ensure input value is an integer and not a float or non-integer

Comments begin with cap and end with period.

+++ modules/simpletest/simpletest.pages.inc	9 Aug 2009 07:26:03 -0000
@@ -453,3 +460,15 @@ function simpletest_settings_form(&$form
+  if ($value !== (string)(int)$value || $value < 0) {

I think "$value !== (string)(int)$value" could be changed to "$value != (int) $value". That would be a bit more standard as I don't think we need a strict compare here and it requires a double cast which, as webchick noted, is odd.

+++ modules/simpletest/simpletest.pages.inc	9 Aug 2009 07:26:03 -0000
@@ -453,3 +460,15 @@ function simpletest_settings_form(&$form
+    form_error($form['general']['simpletest_time_limit'], t('Must be an integer >= 0.'));

Form error messages should include the field name and should use plain english, '>=' should be 'greater then or equal to'.

Beer-o-mania starts in 18 days! Don't drink and patch.

I went ahead and made the changes and cleaned up the validate function.

The function never actually checked if the string was numeric, so 'a' was accepted.

(int) 'a'

Converts the character to its ord code. I added an is_numeric() check.

Confirmed it still works after re-roll. Since fatal errors pick it up...you get the following as the last assertion.

Maximum execution time of 1 second exceeded

I don't understand why we want to make this is a settings versus just bumping the value. That sounds like a really marginal use case to me, and would justify many more things to be settings.

In the description, shouldn't 'error' be 'errors'?

Agreed. Shouldn't we just throw the set_time_limit() call before each testXX function? The docs http://ca.php.net/set_time_limit indicate that it re-starts the timer each time it is called. If the underlying bug is that the scripts are running out of time, that just means we have to call it more often to reset the timer sooner, no?

So settings...and per test method good with everyone?

Why not hard-code the value to 500 or use a define?

And yes, set the value before each function.

Not sure it will solve for all machines, but sure. Patch in a moment.

The reason for leaving as class property is that tests can override if they need more time.

I think that does the job, unless proven otherwise.

ping

Seems appropriate.

Committed to HEAD!