Running cron does not end well [#602958]

Install an expert profile HEAD.
Run a test that runs cron. Trigger and System has one.
Get an obscure error message and yet the tests have finished (that's, I presume, why the bot does not pick the error).

Why?

The test runner does not have search on.
The installed child has search on.
The test points db_prefix points to the installed child tables.
drupal_cron_run calls search_cron.
search_cron registers search_update_totals as a shutdown function.
The test resets db_prefix to the original environemnt.
The shutdown functions run.
There are no search tables.

Simplest fix: provide a cron runner method.

Comment	File	Size	Author
#11	drupal_cron_run_ouch.patch	3.78 KB	chx
#7	drupal_cron_run_ouch.patch	3.24 KB	chx
#5	drupal_cron_run_ouch.patch	3.28 KB	chx
#4	drupal_cron_run_ouch.patch	3.27 KB	chx
#2	drupal_cron_run_ouch.patch	3.33 KB	chx
	drupal_cron_run_ouch.patch	3.33 KB	chx

Comments

Comment #1

damien tournoud commented 13 October 2009 at 04:48

This was reported before (we will have to find the other issue and mark it as a duplicate). The fix does make a lot of sense. By the way, the bot doesn't pick up this error because it is running with the default installation profile, not the expert one.

Comment #2

chx commented 13 October 2009 at 04:54

Status	File	Size
new	drupal_cron_run_ouch.patch	3.33 KB

Comment #3

boombatower commented 13 October 2009 at 05:03

+++ modules/simpletest/drupal_web_test_case.php	2009-10-13 04:53:03 +0000
@@ -1500,6 +1500,13 @@ class DrupalWebTestCase extends DrupalTe
+   * Runs cron in the installed Drupal.

I think 'installed Drupal' is ambiguous. Perhaps we should use the terms: prefixed install and base install.

+++ modules/system/system.test	2009-10-13 04:27:57 +0000
@@ -387,8 +387,8 @@ class CronRunTestCase extends DrupalWebT
+    $this->assertTrue($this->cronRun(), t('Cron ran successfully.'));

cronRun() does not return anything. Looks like it should pass on result from drupalGet().

This review is powered by Dreditor.

Comment #4

chx commented 13 October 2009 at 05:04

Status	File	Size
new	drupal_cron_run_ouch.patch	3.27 KB

system.test needs the direct test simply removed because it makes no sense any more.

Comment #5

chx commented 13 October 2009 at 05:05

Status	File	Size
new	drupal_cron_run_ouch.patch	3.28 KB

Fixed htat comment.

Comment #6

sun

German

Karlsruhe

commented 13 October 2009 at 05:09

+++ modules/simpletest/drupal_web_test_case.php	2009-10-13 05:05:33 +0000
@@ -1500,6 +1500,13 @@ class DrupalWebTestCase extends DrupalTe
+  protected function cronRun() {
+    $this->drupalGet($GLOBALS['base_url'] . '/cron.php', array('external' => TRUE, 'query' => array('cron_key' => variable_get('cron_key', 'drupal'))));
+  }
+++ modules/system/system.test	2009-10-13 05:03:33 +0000
@@ -475,7 +472,7 @@ class CronRunTestCase extends DrupalWebT
-    $this->assertTrue(drupal_cron_run(), t('Cron ran successfully.'));
+    $this->assertTrue($this->cronRun(), t('Cron ran successfully.'));

This did make sense before, because http://api.drupal.org/api/function/drupal_cron_run/7 did return a result.

cronRun(), however, doesn't return anything. Not sure, how tests can pass with this (apparently they did), and maybe we don't need the assertion at all. If we don't, then we might just add a fake assertion to cronRun() that just indicates that cron was supposed to run and "did run" (without asserting it).

This review is powered by Dreditor.