Problem/Motivation

If updates are being run during coron then we should give warning if cron is not set to run often enough.

Proposed resolution

For general instructions see the parent issue: #3230045: [Meta] Create validation listeners

If #3239103: Add setting to enable/disable updates on cron is committed first then we should only run the validation if the updates will happen during cron.

Copy logic for new validator from the 1.x version of this module: https://git.drupalcode.org/project/automatic_updates/-/blob/8.x-1.x/src/...

The subscriber should subscribe to events `\Drupal\automatic_updates\AutomaticUpdatesEvents::READINESS_CHECK` and
it should add a Warning validation result

Issue fork automatic_updates-3242724

Command icon 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

kunal.sachdev made their first commit to this issue’s fork.

kunal.sachdev opened merge request !70

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Title: Ensure core package is validated in readiness check and pre-start » Ensure cron runs frequently enough to run updates
Status: Active » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Needs works for Merge Request comments and custom command fails.

Also needs tests

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented

for the tests we can use the test from 8.x-1.x as an example https://git.drupalcode.org/project/automatic_updates/-/blob/8.x-1.x/test...

We could make this kernel and since our checkers are different we would have to replace

`$messages = $this->container->get('automatic_updates.cron_frequency')->run();`

see `\Drupal\Tests\automatic_updates\Kernel\ReadinessValidation\ComposerExecutableValidatorTest::testErrorIfComposerNotFound` for code for running example replacement

$results = $this->container->get('automatic_updates.readiness_validation_manager')
      ->run()
      ->getResults();
    $this->assertValidationResultsEqual([$error], $results);
kunal.sachdev’s picture

kunal.sachdev CreditAttribution: kunal.sachdev commented
Status: Needs work » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Assigned: tedbow » Unassigned
Status: Needs review » Needs work
kunal.sachdev’s picture

kunal.sachdev CreditAttribution: kunal.sachdev commented
Status: Needs work » Needs review
kunal.sachdev’s picture

kunal.sachdev CreditAttribution: kunal.sachdev at Acquia commented
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Issue tags: +8.x-2.0-alpha1 blocker
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs review » Needs work
Issue tags: -Needs tests

Needs for some comments on the MR also needs to merge in changes in 8.x-2.x

phenaproxima made their first commit to this issue’s fork.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs review » Reviewed & tested by the community

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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