Download & Extend

Running cron does not end well

Project:Drupal core
Version:7.x-dev
Component:simpletest.module
Category:bug report
Priority:normal
Assigned:chx
Status:closed (fixed)
Issue tags:Needs Documentation

Issue Summary

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

Why?

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

Simplest fix: provide a cron runner method.

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.33 KBIdlePassed: 13645 passes, 0 fails, 0 exceptionsView details

Comments

#1

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.

#2

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.33 KBIdleFailed: 13787 passes, 2 fails, 0 exceptionsView details

#3

+++ 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.

#4

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

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.27 KBIdleFailed: 13810 passes, 1 fail, 0 exceptionsView details

#5

Fixed htat comment.

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.28 KBIdleFailed: 13787 passes, 1 fail, 0 exceptionsView details

#6

+++ 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.

#7

And , removed that assert. No need to fake, there are many asserts always to check the effects of cron.

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.24 KBIdlePassed: 13800 passes, 0 fails, 0 exceptionsView details

#8

#9

Status:needs review» reviewed & tested by the community

Alrighty.

#10

Status:reviewed & tested by the community» needs work

We should document this limitation in the PHPDoc of drupal_cron_run(), so that this doesn't bite the next poor slob.

#11

Status:needs work» reviewed & tested by the community

We can do that.

AttachmentSizeStatusTest resultOperations
drupal_cron_run_ouch.patch3.78 KBIgnored: Check issue status.NoneNone

#12

Status:reviewed & tested by the community» needs work

Works for me! Committed to HEAD.

Needs documenting somewhere in the testing documentation at http://drupal.org/simpletest.

#13

Status:needs work» fixed

#14

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.