Problem/Motivation
If a functional test installs the Automated Cron module and then performs some requests against those test sites, any of those requests may lead to cron being run. Since this happens outside of the actual test flow, cron may still not be completed when the test has completed and the test site is being torn down. This will then lead to database exceptions of the cron jobs because database tables they expect to exist no longer do.
Steps to reproduce
Create a test that installs Umami (as that includes Automated Cron and Search) and only performs a single request and then finishes. Run that test on SQLite (not sure why that is required to reproduce this, but it seems to be) and be surprised that you will get, for example, the following exception:
Failed to log error: Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY000]: General error: 1 no such table: test66738342.watchdog: INSERT INTO "test66738342"."watchdog" ("uid", "type", "message", "variables", "severity", "link", "location", "referer", "hostname", "timestamp") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?); Array
(
[0] => 0
[1] => cron
[2] => %type: @message in %function (line %line of %file).
[3] => a:6:{s:5:"%type";s:44:"Drupal\search\Exception\SearchIndexException";s:8:"@message";s:40:"Failed to update totals for index words.";s:9:"%function";s:46:"Drupal\search\SearchIndex->updateWordWeights()";s:5:"%file";s:74:".../core/modules/search/src/SearchIndex.php";s:5:"%line";i:292;s:17:"@backtrace_string";s:2705:"#0 .../core/modules/help/src/Plugin/Search/HelpSearch.php(372): Drupal\search\SearchIndex->updateWordWeights(Array)
Note that I was not able to reproduce this locally, thus far, but this Config Overlay CI job shows the problem.
Proposed resolution
Explicitly wait for cron to finish before tearing down the test site. (If you have any more elegant solutions for this, please share, I couldn't come up with anything...)
Remaining tasks
User interface changes
-
Introduced terminology
-
API changes
-
Data model changes
-
Release notes snippet
-
Issue fork drupal-3540786
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3540786-browsertest-teardown-cron
changes, plain diff MR !12961
Comments
Comment #3
tstoecklerComment #4
smustgrave commentedCan I asked where 900 came from?
Comment #5
tstoecklerI just used that because it's the same as what is used in
\Drupal\Core\Cron::run()so it seemed appropriate.Comment #6
smustgrave commentedThat’s fair. No objections to the rest of the changes.
Comment #7
catchI think we might want to make this even more special cased and do a ::moduleExists('automated_cron') first - nothing else should run cron in an end of request task.
There are other things that can happen end of request too, but:
- they should be a lot quicker than a cron run
- I can't think of any way to generically handle those, except for breaking end of request processing for everyone including outside tests.
Comment #8
tstoecklerThat's fair enough, updated the patch accordingly. Updated the patch for Config Overlay, as well, and the passing pipeline proves that this still works: https://git.drupalcode.org/project/config_overlay/-/merge_requests/44
Comment #9
smustgrave commentedFeedback appears to be addressed on this one.
Comment #10
catchCommitted/pushed to 11.x, thanks!