Endless loop in patch stalls testing - SimpleTest needs a time limit

Maximum testing time seems like a good idea.

I'll disable testing for the moment, I'll try and get to this a soon as possible.

"SimpleTest needs a time limit"

Lets pick one, maybe make it a setting.

I'm not very familiar with the guts of simpletest, but were you thinking a timeout per-test or per-group of tests? You (boombatower) suggested 30 minutes at http://testing.drupal.org/node/18, but on my test bot running all tests currently takes pretty close to 30 minutes, so when additional tests are added I could see it hitting this timeout if the timeout applies to all tests as a whole. If instead the timeout would apply to each test individually, then 30 minutes seems like a long time for a single test to run.

I argue per-test, because then it's much easier to do in batch: we can just use set_time_limit, and register a shutdown function that logs an error to the assertions table. Jimmy argues global, because it varies. What do you guys think?

Additionally, if we do it per-test, we could have a $time_limit variable which has a reasonable default, but can be set to a long time for onger tests, and a shorter time for this - http://drupalbin.com/4670

[Edit: don't know how that bogus stuff got in here, sorry]

Sorry about status change. I'll pick this patch up but we need a consensus about per-test or per-suite

Per-test++. The per-suite time is going to vary all over the place depending on a variety of factors and will constantly be going up as we add more tests so will need to be revised over and over.

Thanks for picking this up, dmitri!

Per test function.

Let's run set_time_limit(180) after calling ->setUp().

Oh, for those wondering, calling set_time_limit() resets the timeout timer:

When called, set_time_limit() restarts the timeout counter from zero. In other words, if the timeout is the default 30 seconds, and 25 seconds into script execution a call such as set_time_limit(20) is made, the script will run for a total of 45 seconds before timing out.

Ah, and +1 on the register_shutdown_function() that's a clever idea. Could we properly continue the batch too?

@DamZ I don't know. Who's our batch export

I've deleted the offending comment/patch (*cough*) so it might be possible to switch things back on in the interim.

This is very easy. The tests can just override the time_limit property as necessary. Batch continues as the batch data gets written by a shutdown function and those run even when the script times out.

test with http://drupalbin.com/4670

What happens when a test times out? It should result in a failure, IMO.

@Dries - it's kind of hard to catch timeouts, so while i think it should result in a fail i am not exactly sure how.

Actually i take that back. Is this overly complex?
- set a global variable such as $timed_out and default to TRUE
- set a shutdown function
- at the end of tests, set it to FALSE
- the shutdown function writes a fail to the database if global $timed_out == TRUE.

That seems reasonable to me. We should also think about what sort of metadata we could add (if applicable) to the failure to help the test runner determine where the timeout was incurred.

The critical issue at hand is to restart the test bot. chx patch in #12 solves this. We need to get it committed as soon as possible.

We studied the "shutdown function" idea with chx, and it is not really easy to grok right: we need to properly interact with the batch API in both javascript and non javascript mode. I opened a separate issue #354518: Simpletest should properly report timeout for this.

In other words, all the wishes lead to a slaughter of kittens:

The solution is not optimal but I committed it to CVS HEAD. Thanks for investigating our options, and for creating a new ticket. Thanks.