Issue #3321256 by omkar.podey, tedbow, Wim Leers: Fix race condition in...

Issue #3321256 by omkar.podey, tedbow, Wim Leers: Fix race condition in \Drupal\automatic_updates_test_cron\Enabler

automatic_updates.install

@@ -7,7 +7,9 @@
 
 declare(strict_types = 1);
 
+use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\Validation\StatusCheckRequirements;
+use Drupal\system\SystemManager;
 
 /**
  * Implements hook_uninstall().
@@ -24,7 +26,19 @@ function automatic_updates_requirements($phase) {
     // Check that site is ready to perform automatic updates.
     /** @var \Drupal\automatic_updates\Validation\StatusCheckRequirements $status_check_requirement */
     $status_check_requirement = \Drupal::classResolver(StatusCheckRequirements::class);
-    return $status_check_requirement->getRequirements();
+    $requirements = $status_check_requirement->getRequirements();
+
+    // Check that site has cron updates enabled or not.
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    if (\Drupal::configFactory()->get('automatic_updates.settings')->get('cron') !== CronUpdater::DISABLED) {
+      $requirements['automatic_updates_cron'] = [
+        'title' => t('Cron installs updates automatically'),
+        'severity' => SystemManager::REQUIREMENT_WARNING,
+        'value' => t('Enabled. This is NOT an officially supported feature of the Automatic Updates module at this time. Use at your own risk.'),
+      ];
+    }
+
+    return $requirements;
   }
 }
 
@@ -38,3 +52,19 @@ function automatic_updates_update_9001(): void {
   $key_value->rename('readiness_validation_last_run', 'status_check_last_run');
   $key_value->rename('readiness_check_timestamp', 'status_check_timestamp');
 }
+
+/**
+ * Sets cron setting to \Drupal\automatic_updates\CronUpdater::DISABLED.
+ *
+ * This is necessary until TUF support is available on drupal.org. This used to
+ * be hardcoded, but that caused complex race conditions in tests. Hence, it is
+ * now just configured.
+ *
+ * @see automatic_updates_requirements()
+ * @see https://www.drupal.org/project/automatic_updates/issues/3284443
+ */
+function automatic_updates_update_9002(): void {
+  /** @var \Drupal\Core\Config\Config $config */
+  $config = \Drupal::service('config.factory')->getEditable('automatic_updates.settings');
+  $config->set('cron', CronUpdater::DISABLED)->save();
+}

automatic_updates_extensions/tests/src/Kernel/Validator/UpdateReleaseValidatorTest.php

@@ -16,6 +16,11 @@ use Drupal\Tests\automatic_updates_extensions\Kernel\AutomaticUpdatesExtensionsK
  */
 class UpdateReleaseValidatorTest extends AutomaticUpdatesExtensionsKernelTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['automatic_updates'];
+
   /**
    * Data provider for testPreCreateException().
    *

config/install/automatic_updates.settings.yml

-cron: security
+cron: disable
 allow_core_minor_updates: false
 status_check_mail: errors_only

src/CronUpdater.php

@@ -26,15 +26,6 @@ use Symfony\Component\HttpFoundation\Response;
  */
 class CronUpdater extends Updater {
 
-  /**
-   * Whether or not cron updates are hard-disabled.
-   *
-   * @var bool
-   *
-   * @todo Remove this when TUF integration is stable.
-   */
-  private static $disabled = TRUE;
-
   /**
    * All automatic updates are disabled.
    *
@@ -337,9 +328,6 @@ class CronUpdater extends Updater {
    *     during cron.
    */
   final public function getMode(): string {
-    if (self::$disabled) {
-      return static::DISABLED;
-    }
     $mode = $this->configFactory->get('automatic_updates.settings')->get('cron');
     return $mode ?: CronUpdater::SECURITY;
   }

tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.module

@@ -7,6 +7,8 @@
  * @todo Move into automatic_updates when TUF integration is stable.
  */
 
+declare(strict_types=1);
+
 use Drupal\package_manager\ProjectInfo;
 use Drupal\automatic_updates\CronUpdater;
 use Drupal\Core\Extension\ExtensionVersion;
@@ -15,6 +17,8 @@ use Drupal\update\ProjectSecurityData;
 
 /**
  * Implements hook_form_FORM_ID_alter() for 'update_settings' form.
+ *
+ * @todo Move to automatic_updates in https://www.drupal.org/i/3322361
  */
 function automatic_updates_test_cron_form_update_settings_alter(array &$form, FormStateInterface $form_state, string $form_id) {
   $project_info = new ProjectInfo('drupal');

tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.services.yml

-services:
-  automatic_updates_test_cron.enabler:
-    class: Drupal\automatic_updates_test_cron\Enabler
-    tags:
-      - { name: event_subscriber }

tests/modules/automatic_updates_test_cron/src/Enabler.php

-<?php
-
-declare(strict_types = 1);
-
-namespace Drupal\automatic_updates_test_cron;
-
-use Drupal\automatic_updates\CronUpdater;
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpKernel\KernelEvents;
-
-/**
- * Enables automatic updates during cron.
- *
- * @todo Remove this when TUF integration is stable.
- */
-class Enabler implements EventSubscriberInterface {
-
-  /**
-   * {@inheritdoc}
-   */
-  public static function getSubscribedEvents(): array {
-    return [
-      KernelEvents::REQUEST => 'enableCron',
-    ];
-  }
-
-  /**
-   * Enables automatic updates during cron.
-   */
-  public function enableCron(): void {
-    if (class_exists(CronUpdater::class)) {
-      $reflector = new \ReflectionClass(CronUpdater::class);
-      $reflector = $reflector->getProperty('disabled');
-      $reflector->setAccessible(TRUE);
-      $reflector->setValue(NULL, FALSE);
-    }
-  }
-
-}

tests/src/Build/UpdateTestBase.php

@@ -19,12 +19,15 @@ abstract class UpdateTestBase extends TemplateProjectTestBase {
    */
   protected function createTestProject(string $template): void {
     parent::createTestProject($template);
-
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $code = <<<END
+\$config['automatic_updates.settings']['cron'] = 'security';
+END;
+    $this->writeSettings($code);
     // Install Automatic Updates, and other modules needed for testing.
     $this->installModules([
       'automatic_updates',
       'automatic_updates_test_api',
-      'automatic_updates_test_cron',
     ]);
   }
 

tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\Tests\automatic_updates\Functional;
 
+use Drupal\automatic_updates\CronUpdater;
 use Drupal\Core\Site\Settings;
 use Drupal\package_manager_bypass\Beginner;
 use Drupal\package_manager_bypass\Stager;
@@ -23,7 +24,7 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
    * {@inheritdoc}
    */
   protected static $modules = [
-    'automatic_updates_test_cron',
+    'automatic_updates',
     'automatic_updates_test_disable_validators',
     'package_manager_bypass',
   ];
@@ -54,6 +55,8 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
     parent::setUp();
     $this->disableValidators($this->disableValidators);
     $this->useFixtureDirectoryAsActive(__DIR__ . '/../../../package_manager/tests/fixtures/fake_site');
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
   }
 
   /**

tests/src/Functional/StatusCheckTest.php

@@ -5,6 +5,7 @@ declare(strict_types = 1);
 namespace Drupal\Tests\automatic_updates\Functional;
 
 use Behat\Mink\Element\NodeElement;
+use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\StatusCheckMailer;
 use Drupal\automatic_updates_test\Datetime\TestTime;
 use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
@@ -330,10 +331,9 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
 
     // Confirm status check messages are not displayed when cron updates are
     // disabled.
-    $this->drupalGet(Url::fromRoute('update.settings'));
-    $edit['automatic_updates_cron'] = 'disable';
-    $this->submitForm($edit, 'Save configuration');
-    $this->drupalGet(Url::fromRoute($admin_route));
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::DISABLED)->save();
+    $this->drupalGet('admin/structure');
+    $this->checkForMetaRefresh();
     $assert->pageTextNotContains(static::$warningsExplanation);
     $assert->pageTextNotContains($expected_results[0]->getMessages()[0]);
   }
@@ -344,6 +344,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
   public function testStatusCheckAfterInstall(): void {
     $assert = $this->assertSession();
     $this->drupalLogin($this->checkerRunnerUser);
+    $this->container->get('module_installer')->uninstall(['automatic_updates']);
 
     $this->drupalGet('admin/reports/status');
     $assert->pageTextNotContains('Update readiness checks');
@@ -351,6 +352,8 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
     // We have to install the automatic_updates_test module because it provides
     // the functionality to retrieve our fake release history metadata.
     $this->container->get('module_installer')->install(['automatic_updates', 'automatic_updates_test']);
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
     $this->drupalGet('admin/reports/status');
     $this->assertNoErrors(TRUE);
 

tests/src/Functional/UpdatePathTest.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\Tests\automatic_updates\Functional;
 
+use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\StatusCheckMailer;
 use Drupal\FunctionalTests\Update\UpdatePathTestBase;
 
@@ -55,8 +56,12 @@ class UpdatePathTest extends UpdatePathTestBase {
     }
     $this->assertEmpty($this->config('automatic_updates.settings')->get('status_check_mail'));
 
+    $this->assertSame(CronUpdater::SECURITY, $this->config('automatic_updates.settings')->get('cron'));
+
     $this->runUpdates();
 
+    $this->assertSame(CronUpdater::DISABLED, $this->config('automatic_updates.settings')->get('cron'));
+
     // TRICKY: we do expect `readiness_validation_last_run` to have been renamed
     // to `status_check_last_run`, but then
     // automatic_updates_post_update_create_status_check_mail_config() should

tests/src/Kernel/AutomaticUpdatesKernelTestBase.php

@@ -23,10 +23,18 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
 
   /**
    * {@inheritdoc}
+   *
+   * TRICKY: due to the way that automatic_updates forcibly disables cron-based
+   * updating for the end user, we need to override the current default
+   * configuration BEFORE the module is installed. This triggers config schema
+   * exceptions. Since none of these tests are interacting with configuration
+   * anyway, this is a reasonable temporary workaround.
+   *
+   * @see ::setUp()
+   * @see https://www.drupal.org/project/automatic_updates/issues/3284443
+   * @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
    */
-  protected static $modules = [
-    'automatic_updates_test_cron',
-  ];
+  protected $strictConfigSchema = FALSE;
 
   /**
    * {@inheritdoc}
@@ -46,6 +54,9 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
     $this->disableValidators[] = 'automatic_updates.validator.xdebug';
     $this->disableValidators[] = 'package_manager.validator.xdebug';
     parent::setUp();
+    // Enable cron updates, which will eventually be the default.
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
 
     // By default, pretend we're running Drupal core 9.8.0 and a non-security
     // update to 9.8.1 is available.
@@ -56,9 +67,6 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
     // from a sane state.
     // @see \Drupal\automatic_updates\Validator\CronFrequencyValidator
     $this->container->get('state')->set('system.cron_last', time());
-
-    // @todo Remove this when TUF integration is stable.
-    $this->container->get('automatic_updates_test_cron.enabler')->enableCron();
   }
 
   /**

tests/src/Kernel/CronUpdaterTest.php

@@ -7,7 +7,6 @@ namespace Drupal\Tests\automatic_updates\Kernel;
 use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\Form\FormState;
 use Drupal\Core\Logger\RfcLogLevel;
 use Drupal\Core\Url;
 use Drupal\package_manager\Event\PostApplyEvent;
@@ -24,9 +23,8 @@ use Drupal\package_manager_bypass\Committer;
 use Drupal\Tests\automatic_updates\Traits\EmailNotificationsTestTrait;
 use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\Tests\user\Traits\UserCreationTrait;
-use Drupal\update\UpdateSettingsForm;
-use ColinODell\PsrTestLogger\TestLogger;
 use Prophecy\Argument;
+use ColinODell\PsrTestLogger\TestLogger;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -53,7 +51,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
   /**
    * The test logger.
    *
-   * @var ColinODell\PsrTestLogger\TestLogger
+   * @var \ColinODell\PsrTestLogger\TestLogger
    */
   private $logger;
 
@@ -151,16 +149,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     $this->setCoreVersion('9.8.0');
     update_get_available(TRUE);
 
-    // Submit the configuration form programmatically, to prove our alterations
-    // work as expected.
-    $form_builder = $this->container->get('form_builder');
-    $form_state = new FormState();
-    $form = $form_builder->buildForm(UpdateSettingsForm::class, $form_state);
-    // Ensure that the version ranges in the setting's description, which are
-    // computed dynamically, look correct.
-    $this->assertStringContainsString('Automatic updates are only supported for 9.8.x versions of Drupal core. Drupal 9.8 will receive security updates until 9.10.0 is released.', (string) $form['automatic_updates_cron']['#description']);
-    $form_state->setValue('automatic_updates_cron', $setting);
-    $form_builder->submitForm(UpdateSettingsForm::class, $form_state);
+    $this->config('automatic_updates.settings')->set('cron', $setting)->save();
 
     // Since we're just trying to ensure that all of Package Manager's services
     // are called as expected, disable validation by replacing the event
@@ -258,6 +247,8 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
    */
   public function testStageDestroyedOnError(string $event_class, string $exception_class): void {
     $this->installConfig('automatic_updates');
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
     $this->setCoreVersion('9.8.0');
     // Ensure that there is a security release to which we should update.
     $this->setReleaseMetadata([

tests/src/Kernel/StatusCheck/StatusCheckFailureEmailTest.php

@@ -53,6 +53,8 @@ class StatusCheckFailureEmailTest extends AutomaticUpdatesKernelTestBase {
     $this->installSchema('user', ['users_data']);
 
     $this->installConfig('automatic_updates');
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
     $this->setUpEmailRecipients();
 
     // Allow stored available update data to live for a very, very long time.

tests/src/Kernel/StatusCheck/StatusCheckerTest.php

@@ -36,6 +36,12 @@ class StatusCheckerTest extends AutomaticUpdatesKernelTestBase {
     $this->setCoreVersion('9.8.2');
     $this->installEntitySchema('user');
     $this->installSchema('user', ['users_data']);
+
+    // Undoes the override in parent::setUp(), to allow the module to be
+    // installed, which every other test methods in this class does. Without
+    // this \Drupal\Core\Config\PreExistingConfigException is thrown.
+    // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+    $this->config('automatic_updates.settings')->delete();
   }
 
   /**

tests/src/Kernel/ValidationResultDisplayTraitTest.php

@@ -20,6 +20,11 @@ class ValidationResultDisplayTraitTest extends AutomaticUpdatesKernelTestBase {
   use ValidationResultDisplayTrait;
   use StringTranslationTrait;
 
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['automatic_updates'];
+
   /**
    * @covers ::displayResults
    */