Issue #3312619 by tedbow, omkar.podey, Wim Leers, kunal.sachdev: Ensure all...

Issue #3312619 by tedbow, omkar.podey, Wim Leers, kunal.sachdev: Ensure all validation results use translatable strings except when created from throwables

automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php

@@ -195,7 +195,10 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $this->checkForUpdates();
     $state = $this->container->get('state');
     $state->set('system.maintenance_mode', $maintenance_mode_on);
-    StagedDatabaseUpdateValidator::setExtensionsWithUpdates(['system', 'automatic_updates_theme_with_updates']);
+    StagedDatabaseUpdateValidator::setExtensionsWithUpdates([
+      'system',
+      'automatic_updates_theme_with_updates',
+    ]);
 
     $page = $this->getSession()->getPage();
     // Navigate to the automatic updates form.
@@ -289,7 +292,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     // Set an error before completing the update. This error should be visible
     // on admin pages after completing the update without having to explicitly
     // run the status checks.
-    TestSubscriber1::setTestResult([ValidationResult::createError(['Error before continue.'])], StatusCheckEvent::class);
+    TestSubscriber1::setTestResult([ValidationResult::createError([t('Error before continue.')])], StatusCheckEvent::class);
     if ($has_database_updates) {
       // Simulate a staged database update in the automatic_updates_test module.
       // We must do this after the update has started, because the pending
@@ -588,9 +591,9 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     // Add warnings from StatusCheckEvent.
     $summary_status_check_event = t('Some summary');
     $messages_status_check_event = [
-      "The only thing we're allowed to do is to",
-      "believe that we won't regret the choice",
-      "we made.",
+      t("The only thing we're allowed to do is to"),
+      t("believe that we won't regret the choice"),
+      t("we made."),
     ];
     $warning_status_check_event = ValidationResult::createWarning($messages_status_check_event, $summary_status_check_event);
     TestSubscriber::setTestResult([$warning_status_check_event], StatusCheckEvent::class);
@@ -619,9 +622,9 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $assert->checkboxChecked('edit-projects-semver-test');
     $assert->buttonExists('Update');
     $messages = [
-      "The only thing we're allowed to do is to",
-      "believe that we won't regret the choice",
-      "we made.",
+      t("The only thing we're allowed to do is to"),
+      t("believe that we won't regret the choice"),
+      t("we made."),
     ];
     $summary = t('Some summary');
     $error = ValidationResult::createError($messages, $summary);

automatic_updates_extensions/tests/src/Kernel/ExtensionUpdaterTest.php

@@ -163,7 +163,7 @@ class ExtensionUpdaterTest extends AutomaticUpdatesExtensionsKernelTestBase {
   public function testUpdateException(string $event_class): void {
     $extension_updater = $this->container->get('automatic_updates_extensions.updater');
     $results = [
-      ValidationResult::createError(['An error of some sorts.']),
+      ValidationResult::createError([t('An error of some sorts.')]),
     ];
     TestSubscriber1::setTestResult($results, $event_class);
     try {

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

@@ -67,7 +67,12 @@ class UpdateReleaseValidatorTest extends AutomaticUpdatesExtensionsKernelTestBas
     if ($error_expected) {
       $expected_results = [
         ValidationResult::createError(
-          ["Project $project to version " . LegacyVersionUtility::convertToSemanticVersion($target_version)],
+          [
+            t('Project @project to version @target_version', [
+              '@project' => $project,
+              '@target_version' => LegacyVersionUtility::convertToSemanticVersion($target_version),
+            ]),
+          ],
           t('Cannot update because the following project version is not in the list of installable releases.')
         ),
       ];

package_manager/src/Event/PreOperationStageEvent.php

@@ -51,4 +51,16 @@ abstract class PreOperationStageEvent extends StageEvent {
     $this->results[] = ValidationResult::createError($messages, $summary);
   }
 
+  /**
+   * Adds an error from a throwable.
+   *
+   * @param \Throwable $throwable
+   *   The throwable.
+   * @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $summary
+   *   (optional) The summary of error messages.
+   */
+  public function addErrorFromThrowable(\Throwable $throwable, ?TranslatableMarkup $summary = NULL): void {
+    $this->results[] = ValidationResult::createErrorFromThrowable($throwable, $summary);
+  }
+
 }

package_manager/src/ValidationResult.php

@@ -22,7 +22,7 @@ final class ValidationResult {
   /**
    * The error messages.
    *
-   * @var \Drupal\Core\StringTranslation\TranslatableMarkup[]
+   * @var \Drupal\Core\StringTranslation\TranslatableMarkup[]|string[]
    */
   protected $messages;
 
@@ -39,7 +39,7 @@ final class ValidationResult {
    * @param int $severity
    *   The severity of the result. Should be one of the
    *   SystemManager::REQUIREMENT_* constants.
-   * @param \Drupal\Core\StringTranslation\TranslatableMarkup[] $messages
+   * @param \Drupal\Core\StringTranslation\TranslatableMarkup[]|string[] $messages
    *   The error messages.
    * @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $summary
    *   The errors summary.
@@ -56,6 +56,20 @@ final class ValidationResult {
     $this->severity = $severity;
   }
 
+  /**
+   * Creates an error ValidationResult object from a throwable.
+   *
+   * @param \Throwable $throwable
+   *   The throwable.
+   * @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $summary
+   *   The errors summary.
+   *
+   * @return static
+   */
+  public static function createErrorFromThrowable(\Throwable $throwable, ?TranslatableMarkup $summary = NULL): self {
+    return new static(SystemManager::REQUIREMENT_ERROR, [$throwable->getMessage()], $summary);
+  }
+
   /**
    * Creates an error ValidationResult object.
    *
@@ -97,7 +111,7 @@ final class ValidationResult {
   /**
    * Gets the messages.
    *
-   * @return \Drupal\Core\StringTranslation\TranslatableMarkup[]
+   * @return \Drupal\Core\StringTranslation\TranslatableMarkup[]|string[]
    *   The error or warning messages.
    */
   public function getMessages(): array {

package_manager/src/Validator/OverwriteExistingPackagesValidator.php

@@ -73,7 +73,10 @@ final class OverwriteExistingPackagesValidator implements EventSubscriberInterfa
     $missing_new_packages = array_diff_key($new_packages, $installed_packages_data);
     if ($missing_new_packages) {
       $missing_new_packages = array_keys($missing_new_packages);
-      $event->addError($missing_new_packages, $this->t('Package Manager could not get the data for the following packages:'));
+      foreach ($missing_new_packages as &$missing_new_package) {
+        $missing_new_package = $this->t('@missing_new_package', ['@missing_new_package' => $missing_new_package]);
+      }
+      $event->addError(array_values($missing_new_packages), $this->t('Package Manager could not get the data for the following packages.'));
       return;
     }
 

package_manager/src/Validator/StagedDBUpdateValidator.php

@@ -166,7 +166,7 @@ class StagedDBUpdateValidator implements EventSubscriberInterface {
    * @param string $stage_dir
    *   The path of the stage directory.
    *
-   * @return string[]
+   * @return \Drupal\Core\StringTranslation\TranslatableMarkup[]
    *   The names of the extensions that have possible database updates.
    */
   public function getExtensionsWithDatabaseUpdates(string $stage_dir): array {

package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php

@@ -109,7 +109,7 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
   public function providerComposerVersionValidation(): array {
     // Invalid or undetectable Composer versions will always produce the same
     // error.
-    $invalid_version = ValidationResult::createError(['The Composer version could not be detected.']);
+    $invalid_version = ValidationResult::createError([t('The Composer version could not be detected.')]);
 
     // Unsupported Composer versions will report the detected version number
     // in the validation result, so we need a function to churn out those fake
@@ -118,7 +118,10 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
       $minimum_version = ComposerExecutableValidator::MINIMUM_COMPOSER_VERSION_CONSTRAINT;
 
       return ValidationResult::createError([
-        "A Composer version which satisfies <code>$minimum_version</code> is required, but version $version was detected.",
+        t('A Composer version which satisfies <code>@minimum_version</code> is required, but version @version was detected.', [
+          '@minimum_version' => $minimum_version,
+          '@version' => $version,
+        ]),
       ]);
     };
 

package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php

@@ -31,7 +31,9 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase {
     // factory as an array, Composer will assume that the configuration is
     // coming from a config.json file, even if one doesn't exist.
     $error = ValidationResult::createError([
-      "The <code>cweagans/composer-patches</code> plugin is installed, but the <code>composer-exit-on-patch-failure</code> key is not set to <code>true</code> in the <code>extra</code> section of $dir/config.json.",
+      t('The <code>cweagans/composer-patches</code> plugin is installed, but the <code>composer-exit-on-patch-failure</code> key is not set to <code>true</code> in the <code>extra</code> section of @dir/config.json.', [
+        '@dir' => $dir,
+      ]),
     ]);
     $this->assertStatusCheckResults([$error]);
     $this->assertResults([$error], PreCreateEvent::class);

package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php

@@ -24,7 +24,7 @@ class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase {
    */
   public function providerSecureHttpValidation(): array {
     $error = ValidationResult::createError([
-      'HTTPS must be enabled for Composer downloads. See <a href="https://getcomposer.org/doc/06-config.md#secure-http">the Composer documentation</a> for more information.',
+      t('HTTPS must be enabled for Composer downloads. See <a href="https://getcomposer.org/doc/06-config.md#secure-http">the Composer documentation</a> for more information.'),
     ]);
 
     return [

package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php

@@ -24,7 +24,7 @@ class EnvironmentSupportValidatorTest extends PackageManagerKernelTestBase {
     putenv(EnvironmentSupportValidator::VARIABLE_NAME . '=broken/url.org');
 
     $result = ValidationResult::createError([
-      'Package Manager is not supported by your environment.',
+      t('Package Manager is not supported by your environment.'),
     ]);
     foreach ([PreCreateEvent::class, StatusCheckEvent::class] as $event_class) {
       $this->assertEventPropagationStopped($event_class, [$this->container->get('package_manager.validator.environment_support'), 'validateStagePreOperation']);

package_manager/tests/src/Kernel/LockFileValidatorTest.php

@@ -42,7 +42,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
   public function testCreateWithNoLock(): void {
     unlink($this->activeDir . '/composer.lock');
 
-    $no_lock = ValidationResult::createError(['Could not hash the active lock file.']);
+    $no_lock = ValidationResult::createError([t('Could not hash the active lock file.')]);
     $stage = $this->assertResults([$no_lock], PreCreateEvent::class);
     // The stage was not created successfully, so the status check should be
     // clear.
@@ -78,7 +78,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
       file_put_contents($this->activeDir . '/composer.lock', 'changed');
     }, $event_class);
     $result = ValidationResult::createError([
-      'Unexpected changes were detected in composer.lock, which indicates that other Composer operations were performed since this Package Manager operation started. This can put the code base into an unreliable state and therefore is not allowed.',
+      t('Unexpected changes were detected in composer.lock, which indicates that other Composer operations were performed since this Package Manager operation started. This can put the code base into an unreliable state and therefore is not allowed.'),
     ]);
     $stage = $this->assertResults([$result], $event_class);
     // A status check should agree that there is an error here.
@@ -99,7 +99,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
       unlink($this->activeDir . '/composer.lock');
     }, $event_class);
     $result = ValidationResult::createError([
-      'Could not hash the active lock file.',
+      t('Could not hash the active lock file.'),
     ]);
     $stage = $this->assertResults([$result], $event_class);
     // A status check should agree that there is an error here.
@@ -123,7 +123,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
       $this->container->get('state')->delete($state_key);
     }, $event_class);
     $result = ValidationResult::createError([
-      'Could not retrieve stored hash of the active lock file.',
+      t('Could not retrieve stored hash of the active lock file.'),
     ]);
     $stage = $this->assertResults([$result], $event_class);
     // A status check should agree that there is an error here.
@@ -138,7 +138,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
     Stager::setLockFileShouldChange(FALSE);
 
     $result = ValidationResult::createError([
-      'There are no pending Composer operations.',
+      t('There are no pending Composer operations.'),
     ]);
     $stage = $this->assertResults([$result], PreApplyEvent::class);
     // A status check shouldn't produce raise any errors, because it's only

package_manager/tests/src/Kernel/MultisiteValidatorTest.php

@@ -27,7 +27,7 @@ class MultisiteValidatorTest extends PackageManagerKernelTestBase {
         TRUE,
         [
           ValidationResult::createError([
-            'Drupal multisite is not supported by Package Manager.',
+            t('Drupal multisite is not supported by Package Manager.'),
           ]),
         ],
       ],

package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php

@@ -113,13 +113,13 @@ class OverwriteExistingPackagesValidatorTest extends PackageManagerKernelTestBas
 
     $expected_results = [
       ValidationResult::createError([
-        'The new package drupal/module_1 will be installed in the directory /vendor/composer/../../modules/module_1, which already exists but is not managed by Composer.',
+        t('The new package drupal/module_1 will be installed in the directory /vendor/composer/../../modules/module_1, which already exists but is not managed by Composer.'),
       ]),
       ValidationResult::createError([
-        'The new package drupal/module_2 will be installed in the directory /vendor/composer/../../modules/module_2, which already exists but is not managed by Composer.',
+        t('The new package drupal/module_2 will be installed in the directory /vendor/composer/../../modules/module_2, which already exists but is not managed by Composer.'),
       ]),
       ValidationResult::createError([
-        'The new package drupal/module_4 will be installed in the directory /vendor/composer/../../modules/module_1, which already exists but is not managed by Composer.',
+        t('The new package drupal/module_4 will be installed in the directory /vendor/composer/../../modules/module_1, which already exists but is not managed by Composer.'),
       ]),
     ];
     $this->assertResults($expected_results, PreApplyEvent::class);

package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php

@@ -193,9 +193,9 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
    *
    * @param \Drupal\package_manager\ValidationResult[] $expected_results
    *   The expected validation results.
-   * @param \Drupal\package_manager\Stage|null $stage
-   *   (optional) The stage to use to create the status check event. If none is
-   *   provided a new stage will be created.
+   * @param \Drupal\Tests\package_manager\Kernel\TestStage|null $stage
+   *   (optional) The test stage to use to create the status check event. If
+   *   none is provided a new stage will be created.
    */
   protected function assertStatusCheckResults(array $expected_results, Stage $stage = NULL): void {
     $actual_results = $this->runStatusCheck($stage ?? $this->createStage(), $this->container->get('event_dispatcher'));
@@ -488,6 +488,19 @@ class TestStage extends Stage {
 
   use TestStageTrait;
 
+  /**
+   * {@inheritdoc}
+   *
+   * TRICKY: without this, any failed ::assertStatusCheckResults()
+   * will fail, because PHPUnit will want to serialize all arguments in the call
+   * stack.
+   *
+   * @see https://www.drupal.org/project/automatic_updates/issues/3312619#comment-14801308
+   */
+  public function __sleep(): array {
+    return [];
+  }
+
 }
 
 /**

package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php

@@ -44,7 +44,7 @@ class PendingUpdatesValidatorTest extends PackageManagerKernelTestBase {
     require_once __DIR__ . '/../../fixtures/db_update.php';
 
     $result = ValidationResult::createError([
-      'Some modules have database schema updates to install. You should run the <a href="/update.php">database update script</a> immediately.',
+      t('Some modules have database schema updates to install. You should run the <a href="/update.php">database update script</a> immediately.'),
     ]);
     $this->assertStatusCheckResults([$result]);
     $this->assertResults([$result], PreCreateEvent::class);
@@ -58,7 +58,7 @@ class PendingUpdatesValidatorTest extends PackageManagerKernelTestBase {
     // will think it's pending.
     require_once __DIR__ . '/../../fixtures/post_update.php';
     $result = ValidationResult::createError([
-      'Some modules have database schema updates to install. You should run the <a href="/update.php">database update script</a> immediately.',
+      t('Some modules have database schema updates to install. You should run the <a href="/update.php">database update script</a> immediately.'),
     ]);
     $this->assertStatusCheckResults([$result]);
     $this->assertResults([$result], PreCreateEvent::class);

package_manager/tests/src/Kernel/SettingsValidatorTest.php

@@ -22,7 +22,7 @@ class SettingsValidatorTest extends PackageManagerKernelTestBase {
    *   The test cases.
    */
   public function providerSettingsValidation(): array {
-    $result = ValidationResult::createError(['The <code>update_fetch_with_http_fallback</code> setting must be disabled.']);
+    $result = ValidationResult::createError([t('The <code>update_fetch_with_http_fallback</code> setting must be disabled.')]);
 
     return [
       'HTTP fallback enabled' => [TRUE, [$result]],

package_manager/tests/src/Kernel/StageEventsTest.php

@@ -141,7 +141,7 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc
    * @dataProvider providerValidationResults
    */
   public function testValidationResults(string $event_class): void {
-    $error_messages = ['Burn, baby, burn'];
+    $error_messages = [t('Burn, baby, burn')];
     // Set up an event listener which will only flag an error for the event
     // class under test.
     $handler = function (StageEvent $event) use ($event_class, $error_messages): void {

package_manager/tests/src/Kernel/StageValidationExceptionTest.php

@@ -31,10 +31,11 @@ class StageValidationExceptionTest extends PackageManagerKernelTestBase {
    */
   public function providerResultsAsText(): array {
     $messages = ['Bang!', 'Pow!'];
+    $translated_messages = [t('Bang!'), t('Pow!')];
     $summary = t('There was sadness.');
 
-    $result_no_summary = ValidationResult::createError([$messages[0]]);
-    $result_with_summary = ValidationResult::createError($messages, $summary);
+    $result_no_summary = ValidationResult::createError([$translated_messages[0]]);
+    $result_with_summary = ValidationResult::createError($translated_messages, $summary);
     $result_with_summary_message = "{$summary->getUntranslatedString()}\n{$messages[0]}\n{$messages[1]}\n";
 
     return [

package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php

@@ -96,7 +96,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
       unlink("$stage_dir/$path/$name.$suffix");
     }
 
-    $result = ValidationResult::createWarning(['System', 'Stark'], t('Possible database updates have been detected in the following extensions.'));
+    $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
     $this->assertStatusCheckResults([$result], $stage);
   }
 
@@ -117,7 +117,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
       file_put_contents("$stage_dir/$path/$name.$suffix", $this->randomString());
     }
 
-    $result = ValidationResult::createWarning(['System', 'Stark'], t('Possible database updates have been detected in the following extensions.'));
+    $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
     $this->assertStatusCheckResults([$result], $stage);
   }
 
@@ -139,7 +139,7 @@ class StagedDBUpdateValidatorTest extends PackageManagerKernelTestBase {
       unlink("$active_dir/$path/$name.$suffix");
     }
 
-    $result = ValidationResult::createWarning(['System', 'Stark'], t('Possible database updates have been detected in the following extensions.'));
+    $result = ValidationResult::createWarning([t('System'), t('Stark')], t('Possible database updates have been detected in the following extensions.'));
     $this->assertStatusCheckResults([$result], $stage);
   }