Issue #3374753 by tedbow, phenaproxima: HookCronTest is broken and doesn't...

Issue #3374753 by tedbow, phenaproxima: HookCronTest is broken and doesn't test how often status checks will be run

tests/src/Functional/StatusCheckTest.php

@@ -7,7 +7,6 @@ namespace Drupal\Tests\automatic_updates\Functional;
 use Behat\Mink\Element\NodeElement;
 use Drupal\automatic_updates\CronUpdateStage;
 use Drupal\automatic_updates\StatusCheckMailer;
-use Drupal\automatic_updates_test\Datetime\TestTime;
 use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
 use Drupal\automatic_updates_test_status_checker\EventSubscriber\TestSubscriber2;
 use Drupal\Core\Url;
@@ -142,8 +141,6 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
 
     // Confirm a user without the permission to run status checks does not
     // have a link to run the checks when the checks need to be run again.
-    // @todo Change this to fake the request time in
-    //   https://www.drupal.org/node/3113971.
     /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $key_value */
     $key_value = $this->container->get('keyvalue.expirable')->get('automatic_updates');
     $key_value->delete('status_check_last_run');
@@ -247,7 +244,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
   }
 
   /**
-   * Tests status check results on admin pages..
+   * Tests status check results on admin pages.
    *
    * @param string $admin_route
    *   The admin route to check.
@@ -271,9 +268,6 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
     // a link to run the checks when the checks need to be run again.
     $expected_results = [$this->createValidationResult(SystemManager::REQUIREMENT_ERROR)];
     TestSubscriber1::setTestResult($expected_results, StatusCheckEvent::class);
-    // @todo Change this to use ::delayRequestTime() to simulate running cron
-    //   after a 24 wait instead of directly deleting 'status_check_last_run'
-    //   https://www.drupal.org/node/3113971.
     /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $key_value */
     $key_value = $this->container->get('keyvalue.expirable')->get('automatic_updates');
     $key_value->delete('status_check_last_run');
@@ -297,9 +291,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
       '1 warning' => $this->createValidationResult(SystemManager::REQUIREMENT_WARNING),
     ];
     TestSubscriber1::setTestResult($expected_results, StatusCheckEvent::class);
-    // Confirm a new message is displayed if the cron is run after an hour.
-    $this->delayRequestTime();
-    $this->cronRun();
+    $this->runStatusChecks();
     $this->drupalGet(Url::fromRoute($admin_route));
     $assert->pageTextContainsOnce(static::$errorsExplanation);
     // Confirm on admin pages that the summary will be displayed.
@@ -311,25 +303,22 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
     $assert->pageTextNotContains($expected_results['1 warning']->messages[0]);
     $assert->pageTextNotContains($expected_results['1 warning']->summary);
 
-    // Confirm that if cron runs less than hour after it previously ran it will
-    // not run the checkers again.
+    // Confirm the status check event is not dispatched on every admin page
+    // load.
     $unexpected_results = [
       '2 errors' => $this->createValidationResult(SystemManager::REQUIREMENT_ERROR, 2),
       '2 warnings' => $this->createValidationResult(SystemManager::REQUIREMENT_WARNING, 2),
     ];
     TestSubscriber1::setTestResult($unexpected_results, StatusCheckEvent::class);
-    $this->delayRequestTime(30);
-    $this->cronRun();
     $this->drupalGet(Url::fromRoute($admin_route));
     $assert->pageTextNotContains($unexpected_results['2 errors']->summary);
     $assert->pageTextContainsOnce((string) $expected_results['1 error']->summary);
     $assert->pageTextNotContains($unexpected_results['2 warnings']->summary);
     $assert->pageTextNotContains($expected_results['1 warning']->messages[0]);
 
-    // Confirm that is if cron is run over an hour after the checkers were
-    // previously run the checkers will be run again.
-    $this->delayRequestTime(31);
-    $this->cronRun();
+    // Confirm the updated results will be shown when status checks are run
+    // again.
+    $this->runStatusChecks();
     $expected_results = $unexpected_results;
     $this->drupalGet(Url::fromRoute($admin_route));
     // Confirm on admin pages only the error summary will be displayed if there
@@ -347,8 +336,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
 
     $expected_results = [$this->createValidationResult(SystemManager::REQUIREMENT_WARNING, 2)];
     TestSubscriber1::setTestResult($expected_results, StatusCheckEvent::class);
-    $this->delayRequestTime();
-    $this->cronRun();
+    $this->runStatusChecks();
     $this->drupalGet(Url::fromRoute($admin_route));
     // Confirm that the warnings summary is displayed on admin pages if there
     // are no errors.
@@ -361,8 +349,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
 
     $expected_results = [$this->createValidationResult(SystemManager::REQUIREMENT_WARNING)];
     TestSubscriber1::setTestResult($expected_results, StatusCheckEvent::class);
-    $this->delayRequestTime();
-    $this->cronRun();
+    $this->runStatusChecks();
     $this->drupalGet(Url::fromRoute($admin_route));
     $assert->pageTextNotContains(static::$errorsExplanation);
     // Confirm that a single warning is displayed and not the summary on admin
@@ -761,15 +748,11 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
   }
 
   /**
-   * Delays the request for the test.
-   *
-   * @param int $minutes
-   *   The number of minutes to delay request time. Defaults to 61 minutes.
+   * Runs status checks.
    */
-  private function delayRequestTime(int $minutes = 61): void {
-    static $total_delay = 0;
-    $total_delay += $minutes;
-    TestTime::setFakeTimeByOffset("+$total_delay minutes");
+  private function runStatusChecks(): void {
+    $this->drupalGet('/admin/reports/status');
+    $this->clickLink('Rerun readiness checks');
   }
 
 }

tests/src/Kernel/HookCronTest.php

@@ -2,7 +2,9 @@
 
 namespace Drupal\Tests\automatic_updates\Kernel;
 
-use Drupal\automatic_updates\Validation\StatusChecker;
+use Drupal\automatic_updates\CronUpdateStage;
+use Drupal\automatic_updates_test\Datetime\TestTime;
+use Drupal\package_manager\Event\StatusCheckEvent;
 
 /**
  * @group automatic_updates
@@ -12,25 +14,47 @@ class HookCronTest extends AutomaticUpdatesKernelTestBase {
   /**
    * {@inheritdoc}
    */
-  protected static $modules = ['automatic_updates'];
+  protected static $modules = ['automatic_updates', 'automatic_updates_test'];
 
   /**
-   * Tests that our cron hook does not run if we're at the command line.
+   * Tests that our cron hook will run status checks.
    */
-  public function testCronHookBypassedAtCommandLine(): void {
-    if (PHP_SAPI !== 'cli') {
-      $this->markTestSkipped('This test requires that PHP be running at the command line.');
-    }
-
-    // The status check should not have run yet.
-    /** @var \Drupal\automatic_updates\Validation\StatusChecker $status_checker */
-    $status_checker = $this->container->get(StatusChecker::class);
-    $this->assertNull($status_checker->getLastRunTime());
+  public function testStatusChecksRunOnCron(): void {
+    // Set the core version to 9.8.1 so there will not be an update attempted.
+    // The hook_cron implementations will not be run if there is an update.
+    // @see \Drupal\automatic_updates\CronUpdateStage::run()
+    // @todo Remove this is https://drupal.org/i/3357969
+    $this->setCoreVersion('9.8.1');
+    // Undo override of the 'serverApi' property from the parent test class.
+    // @see \Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase::setUp
+    $property = new \ReflectionProperty(CronUpdateStage::class, 'serverApi');
+    $property->setValue(NULL, 'cli');
+    $this->assertTrue(CronUpdateStage::isCommandLine());
+    $status_check_count = 0;
+    $this->addEventTestListener(function () use (&$status_check_count) {
+      $status_check_count++;
+    }, StatusCheckEvent::class);
 
     // Since we're at the command line, status checks should still not run, even
     // if we do run cron.
     $this->container->get('cron')->run();
-    $this->assertNull($status_checker->getResults());
+    $this->assertSame(0, $status_check_count);
+
+    // If we are on the web the status checks should run.
+    $property->setValue(NULL, 'cgi-fcgi');
+    $this->assertFalse(CronUpdateStage::isCommandLine());
+    $this->container->get('cron')->run();
+    $this->assertSame(1, $status_check_count);
+
+    // Ensure that the status checks won't run if less than an hour has passed.
+    TestTime::setFakeTimeByOffset("+30 minutes");
+    $this->container->get('cron')->run();
+    $this->assertSame(1, $status_check_count);
+
+    // The status checks should run if more than an hour has passed.
+    TestTime::setFakeTimeByOffset("+61 minutes");
+    $this->container->get('cron')->run();
+    $this->assertSame(2, $status_check_count);
   }
 
 }