Problem/Motivation

\Drupal\system\Tests\System\CronQueueTest is currently implemented as a web test, even everything happens without HTTP requests but rather in the scope of the test process.

This is slower than possible for example.

Proposed resolution

Move the test to extend KernelTestBase to be a little bit faster and more important, more convenient to work with.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

juampynr’s picture

Status: Active » Needs review
FileSize
3.75 KB
7.14 KB

Here it is. I have added a diff between the WebTest and the KernelTest to make easier to see what I changed.

dawehner’s picture

  1. +++ b/core/modules/system/tests/src/Kernel/System/CronQueueTest.php
    @@ -30,20 +58,23 @@ public function testExceptions() {
    +    // Expire the queue item manually. system_cron() relies in REQUEST_TIME to
    +    // find queue items whose expire field needs to be reset to 0. This is a
    +    // Kernel test, so REQUEST_TIME won't change when cron runs.
    +    // @see system_cron()
         // @see \Drupal\Core\Cron::processQueues()
    

    We could clean that up in https://www.drupal.org/node/2717207 a bit, I think, but for now this seems okay/the only solution to test that?

  2. +++ b/core/modules/system/tests/src/Kernel/System/CronQueueTest.php
    @@ -57,7 +88,8 @@ public function testExceptions() {
         // item.
    -    $this->cronRun();
    +    system_cron();
    +    $this->cron->run();
    
    @@ -72,7 +104,9 @@ public function testExceptions() {
         $queue->createItem([]);
    -    $this->cronRun();
    +    system_cron();
    +    $this->cron->run();
    

    Why does $this->cron->run() not execute system_cron()?
    Do we care that this bit is actually implemented in system module itself?

juampynr’s picture

1. Yes, for the moment this is how we need to do it. I have subscribed to that issue, so I will see if I can help.

2. Good catch! I did further debugging, and saw that cron->run() calls Cron Handlers, which invokes hook_cron, which calls system_cron(). Therefore, there is no need to call system_cron() directly. I have removed it from the patch.

Status: Needs review » Needs work

The last submitted patch, 4: convert-2765207-4.patch, failed testing.

juampynr’s picture

Status: Needs work » Needs review

Drupal CI has been drinking. Not me.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 2c4aa41e63ff42c668c605f49d671f55e3d3a116 to 8.2.x and 71300b5 to 8.1.x. Thanks!

  • alexpott committed 2c4aa41 on 8.2.x
    Issue #2765207 by juampynr, dawehner: Convert \Drupal\system\Tests\...

  • alexpott committed 71300b5 on 8.1.x
    Issue #2765207 by juampynr, dawehner: Convert \Drupal\system\Tests\...

  • alexpott committed 2c4aa41 on 8.3.x
    Issue #2765207 by juampynr, dawehner: Convert \Drupal\system\Tests\...

  • alexpott committed 2c4aa41 on 8.3.x
    Issue #2765207 by juampynr, dawehner: Convert \Drupal\system\Tests\...

Status: Fixed » Closed (fixed)

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