Commit a623c259 authored by Kunal Sachdev's avatar Kunal Sachdev Committed by Ted Bowman
Browse files

Issue #3259810 by tedbow, kunal.sachdev: Validation errors are not logged on cron update attempts

parent 01845303

package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php

0 → 100644
+33 −0
Original line number Diff line number Diff line 
<?php



namespace Drupal\Tests\package_manager\Traits;



/**

 * Common functions for testing using the package_manager_bypass module.

 */

trait PackageManagerBypassTestTrait {



  /**

   * Asserts the number of times an update was staged.

   *

   * @param int $attempted_times

   *   The expected number of times an update was staged.

   */

  private function assertUpdateStagedTimes(int $attempted_times): void {

    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $beginner */

    $beginner = $this->container->get('package_manager.beginner');

    $this->assertCount($attempted_times, $beginner->getInvocationArguments());



    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $stager */

    $stager = $this->container->get('package_manager.stager');

    // If an update was attempted, then there will be two calls to the stager:

    // one to change the constraints in composer.json, and another to actually

    // update the installed dependencies.

    $this->assertCount($attempted_times * 2, $stager->getInvocationArguments());



    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $committer */

    $committer = $this->container->get('package_manager.committer');

    $this->assertEmpty($committer->getInvocationArguments());

  }



}

src/CronUpdater.php

+32 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ namespace Drupal\automatic_updates; 


use Drupal\Core\Config\ConfigFactoryInterface;

use Drupal\Core\Logger\LoggerChannelFactoryInterface;

use Drupal\package_manager\Exception\StageValidationException;



/**

 * Defines a service that updates via cron.
@@ -114,6 +115,10 @@ class CronUpdater extends Updater { 
      $this->apply();

      $this->destroy();

    }

    catch (StageValidationException $e) {

      $this->logger->error($this->getLogMessageForValidationException($e));

      return;

    }

    catch (\Throwable $e) {

      $this->logger->error($e->getMessage());

      return;
@@ -128,4 +133,31 @@ class CronUpdater extends Updater { 
    );

  }



  /**

   * Generates a log message from a stage validation exception.

   *

   * @param \Drupal\package_manager\Exception\StageValidationException $exception

   *   The validation exception.

   *

   * @return string

   *   The formatted log message, including all the validation results.

   */

  protected function getLogMessageForValidationException(StageValidationException $exception): string {

    $log_message = '';

    foreach ($exception->getResults() as $result) {

      $summary = $result->getSummary();

      if ($summary) {

        $log_message .= "<h3>$summary</h3><ul>";

        foreach ($result->getMessages() as $message) {

          $log_message .= "<li>$message</li>";

        }

        $log_message .= "</ul>";

      }

      else {

        $log_message .= ($log_message ? ' ' : '') . $result->getMessages()[0];

      }

    }

    return "<h2>{$exception->getMessage()}</h2>$log_message";

  }



}

tests/src/Functional/UpdaterFormTest.php

+2 −23
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ use Drupal\automatic_updates\Event\ReadinessCheckEvent; 
use Drupal\package_manager\Event\PreCreateEvent;

use Drupal\package_manager\ValidationResult;

use Drupal\automatic_updates_test\ReadinessChecker\TestChecker1;

use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;

use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait;



/**
@@ -16,6 +17,7 @@ use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait; 
class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {



  use ValidationTestTrait;

  use PackageManagerBypassTestTrait;



  /**

   * {@inheritdoc}
@@ -268,27 +270,4 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { 
    $assert_session->buttonExists('Continue');

  }



  /**

   * Asserts the number of times an update was staged.

   *

   * @param int $attempted_times

   *   The expected number of times an update was staged.

   */

  private function assertUpdateStagedTimes(int $attempted_times): void {

    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $beginner */

    $beginner = $this->container->get('package_manager.beginner');

    $this->assertCount($attempted_times, $beginner->getInvocationArguments());



    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $stager */

    $stager = $this->container->get('package_manager.stager');

    // If an update was attempted, then there will be two calls to the stager:

    // one to change the constraints in composer.json, and another to actually

    // update the installed dependencies.

    $this->assertCount($attempted_times * 2, $stager->getInvocationArguments());



    /** @var \Drupal\package_manager_bypass\InvocationRecorderBase $committer */

    $committer = $this->container->get('package_manager.committer');

    $this->assertEmpty($committer->getInvocationArguments());

  }



}

tests/src/Kernel/CronUpdaterTest.php

+72 −0
Original line number Diff line number Diff line
@@ -3,8 +3,14 @@ 
namespace Drupal\Tests\automatic_updates\Kernel;



use Drupal\automatic_updates\CronUpdater;

use Drupal\automatic_updates_test\ReadinessChecker\TestChecker1;

use Drupal\Core\Form\FormState;

use Drupal\Core\Logger\RfcLogLevel;

use Drupal\package_manager\Event\PreCreateEvent;

use Drupal\package_manager\ValidationResult;

use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;

use Drupal\update\UpdateSettingsForm;

use Psr\Log\Test\TestLogger;

use Symfony\Component\EventDispatcher\EventDispatcherInterface;



/**
@@ -15,6 +21,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; 
 */

class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {



  use PackageManagerBypassTestTrait;



  /**

   * {@inheritdoc}

   */
@@ -22,6 +30,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { 
    'automatic_updates',

    'package_manager',

    'package_manager_bypass',

    'automatic_updates_test',

  ];



  /**
@@ -117,4 +126,67 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { 
    $this->assertCount($will_update, $this->container->get('package_manager.committer')->getInvocationArguments());

  }



  /**

   * Data provider for testErrors().

   *

   * @return array[]

   *   The test cases for testErrors().

   */

  public function providerErrors(): array {

    $messages = [

      'Precreate Event Error',

      'Precreate Event Error 2',

    ];

    $summary = 'There were errors in updates';

    $result_no_summary = ValidationResult::createError([$messages[0]]);

    $result_with_summary = ValidationResult::createError($messages, t($summary));

    $result_with_summary_message = "<h3>{$summary}</h3><ul><li>{$messages[0]}</li><li>{$messages[1]}</li></ul>";



    return [

      '1 result with summary' => [

        [$result_with_summary],

        $result_with_summary_message,

      ],

      '2 results with summary' => [

        [$result_with_summary, $result_with_summary],

        "$result_with_summary_message$result_with_summary_message",

      ],

      '1 result without summary' => [

        [$result_no_summary],

        $messages[0],

      ],

      '2 results without summary' => [

        [$result_no_summary, $result_no_summary],

        $messages[0] . ' ' . $messages[0],

      ],

      '1 result with summary, 1 result without summary' => [

        [$result_with_summary, $result_no_summary],

        $result_with_summary_message . ' ' . $messages[0],

      ],

    ];

  }



  /**

   * Tests errors during a cron update attempt.

   *

   * @param \Drupal\package_manager\ValidationResult[] $validation_results

   *   The expected validation results which should be logged.

   * @param string $expected_log_message

   *   The error message should be logged.

   *

   * @dataProvider providerErrors

   */

  public function testErrors(array $validation_results, string $expected_log_message): void {

    TestChecker1::setTestResult($validation_results, PreCreateEvent::class);



    $logger = new TestLogger();

    $this->container->get('logger.factory')

      ->get('automatic_updates')

      ->addLogger($logger);



    $this->container->get('cron')->run();

    $this->assertUpdateStagedTimes(0);

    $this->assertTrue($logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>$expected_log_message", RfcLogLevel::ERROR));

  }



}