CronRunTrait::cronRun() should assert the response status code.

Created on 9 July 2024, 3 months ago
Updated 14 July 2024, 3 months ago

Problem/Motivation

The CronRunTrait::cronRun() is used in tests to run cron by visiting "/cron/..."
(In 📌 deprecate CronRunTrait Needs work we discuss whether this is a good idea - but I think at least in some cases we do want to keep it.)

The "/cron/.." path will respond with one of:
- Status code 204, if no php error occurs.
- Status code 200, if a php fatal error occurs, and error reporting is enabled.
- Status code 500, if a php fatal error occurs, and error reporting is disabled.

The typical "fatal error" can be a timeout from max_execution_time, because a hook_cron() or a queue operation took too long.

If this happens, the lock is not released, and subsequent cron jobs in the next 900 seconds will not do anything.

A test that runs cron should fail if the response code is not 204.

Steps to reproduce

Create a cron job that produces a fatal error.
Create a test which uses CronRunTrait and which calls cronRun().

Expected: Test should fail.
Actual: Test passes.

Proposed resolution

Now it gets tricky.
The straightforward solution is to add this line in CronRunTrait::cronRun():

    $this->assertSame(204, $this->getSession()->getStatusCode());

Unfortunately, we don't really know if the class that includes this trait actually has a getSession() method, and how it behaves.

We could do this to be safe:

    if ($this instanceof BrowserTestBase) {
      $this->assertSame(204, $this->getSession()->getStatusCode());
    }

I am open for ideas.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component
PHPUnit 

Last updated 1 day ago

Created by

🇩🇪Germany donquixote

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Production build 0.71.5 2024