From d4fb6edeb097a7bef202fec4c4a0b1fbcb58e250 Mon Sep 17 00:00:00 2001
From: tedbow <tedbow@240860.no-reply.drupal.org>
Date: Mon, 31 Jan 2022 17:04:11 +0000
Subject: [PATCH] Issue #3260698 by phenaproxima, tedbow: Make it simpler to
 disable validators in tests

---
 .../Kernel/PackageManagerKernelTestBase.php   | 33 ++++++----
 .../WritableFileSystemValidatorTest.php       | 16 ++---
 ...c_updates_test_disable_validators.info.yml |  4 --
 ...esTestDisableValidatorsServiceProvider.php | 26 --------
 .../AutomaticUpdatesFunctionalTestBase.php    | 62 ++++++++++++-------
 .../Functional/ReadinessValidationTest.php    | 20 ++----
 tests/src/Functional/UpdaterFormTest.php      | 23 +++----
 .../Kernel/AutomaticUpdatesKernelTestBase.php | 34 +++++-----
 .../PackageManagerReadinessChecksTest.php     |  8 +--
 .../StagedProjectsValidatorTest.php           | 19 +++---
 10 files changed, 111 insertions(+), 134 deletions(-)
 delete mode 100644 tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml
 delete mode 100644 tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php

diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
index 1cb6afab36..c75251baf1 100644
--- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
+++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
@@ -28,6 +28,21 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
     'package_manager_bypass',
   ];
 
+  /**
+   * The service IDs of any validators to disable.
+   *
+   * @var string[]
+   */
+  protected $disableValidators = [
+    // Disable the filesystem permissions validator, since we cannot guarantee
+    // that the current code base will be writable in all testing situations.
+    // We test this validator functionally in Automatic Updates' build tests,
+    // since those do give us control over the filesystem permissions.
+    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
+    // @see \Drupal\Tests\package_manager\Kernel\WritableFileSystemValidatorTest
+    'package_manager.validator.file_system',
+  ];
+
   /**
    * {@inheritdoc}
    */
@@ -41,20 +56,12 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
    */
   public function register(ContainerBuilder $container) {
     parent::register($container);
-    $this->disableValidators($container);
-  }
 
-  /**
-   * Disables any validators that will interfere with this test.
-   */
-  protected function disableValidators(ContainerBuilder $container): void {
-    // Disable the filesystem permissions validator, since we cannot guarantee
-    // that the current code base will be writable in all testing situations.
-    // We test this validator functionally in Automatic Updates' build tests,
-    // since those do give us control over the filesystem permissions.
-    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
-    // @see \Drupal\Tests\package_manager\Kernel\WritableFileSystemValidatorTest
-    $container->removeDefinition('package_manager.validator.file_system');
+    foreach ($this->disableValidators as $service_id) {
+      if ($container->hasDefinition($service_id)) {
+        $container->getDefinition($service_id)->clearTag('event_subscriber');
+      }
+    }
   }
 
   /**
diff --git a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
index 40e3f683c8..addf0a2172 100644
--- a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
+++ b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
@@ -21,6 +21,14 @@ use Drupal\Core\DependencyInjection\ContainerBuilder;
  */
 class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $disableValidators = [
+    // The parent class disables the validator we're testing, so prevent that
+    // here with an empty array.
+  ];
+
   /**
    * {@inheritdoc}
    */
@@ -33,14 +41,6 @@ class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase {
       ->setClass(TestWritableFileSystemValidator::class);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function disableValidators(ContainerBuilder $container): void {
-    // The parent method disables the validator we're testing, so we don't want
-    // to do anything here.
-  }
-
   /**
    * Data provider for ::testWritable().
    *
diff --git a/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml b/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml
deleted file mode 100644
index 31c0d195f8..0000000000
--- a/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml
+++ /dev/null
@@ -1,4 +0,0 @@
-name: 'Automatic Updates Test: Disable validators'
-description: Allows certain update validators to be disabled during testing.
-type: module
-package: Testing
diff --git a/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php b/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php
deleted file mode 100644
index 307b1a7124..0000000000
--- a/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php
+++ /dev/null
@@ -1,26 +0,0 @@
-<?php
-
-namespace Drupal\automatic_updates_test_disable_validators;
-
-use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\DependencyInjection\ServiceProviderBase;
-use Drupal\Core\Site\Settings;
-
-/**
- * Allows specific validators to be disabled by site settings.
- *
- * This should only really be used by functional tests. Kernel tests should
- * override their ::register() method to remove service definitions; build tests
- * should stay out of the API/services layer unless absolutely necessary.
- */
-class AutomaticUpdatesTestDisableValidatorsServiceProvider extends ServiceProviderBase {
-
-  /**
-   * {@inheritdoc}
-   */
-  public function alter(ContainerBuilder $container) {
-    $validators = Settings::get('automatic_updates_disable_validators', []);
-    array_walk($validators, [$container, 'removeDefinition']);
-  }
-
-}
diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
index 4abd7e8c32..6d27adfa7a 100644
--- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
+++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\automatic_updates\Functional;
 
+use Drupal\Component\Serialization\Yaml;
 use Drupal\Tests\BrowserTestBase;
 
 /**
@@ -12,41 +13,54 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
   /**
    * {@inheritdoc}
    */
-  protected static $modules = [
-    'automatic_updates_test_disable_validators',
-    'update',
-    'update_test',
+  protected static $modules = ['update', 'update_test'];
+
+  /**
+   * The service IDs of any validators to disable.
+   *
+   * @var string[]
+   */
+  protected $disableValidators = [
+    // Disable the filesystem permissions validators, since we cannot guarantee
+    // that the current code base will be writable in all testing situations. We
+    // test these validators in our build tests, since those do give us control
+    // over the filesystem permissions.
+    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
+    'automatic_updates.validator.file_system_permissions',
+    'package_manager.validator.file_system',
   ];
 
   /**
    * {@inheritdoc}
    */
-  protected function prepareSettings() {
-    parent::prepareSettings();
-
-    $settings['settings']['automatic_updates_disable_validators'] = (object) [
-      'value' => $this->disableValidators(),
-      'required' => TRUE,
-    ];
-    $this->writeSettings($settings);
+  protected function setUp() {
+    parent::setUp();
+    $this->disableValidators($this->disableValidators);
   }
 
   /**
-   * Returns the service IDs of any validators to disable.
+   * Disables validators in the test site's services.yml.
+   *
+   * This modifies the service container such that the disabled validators are
+   * instances of stdClass, and not subscribed to any events.
    *
-   * @return string[]
+   * @param string[] $validators
    *   The service IDs of the validators to disable.
    */
-  protected function disableValidators(): array {
-    // Disable the filesystem permissions validators, since we cannot guarantee
-    // that the current code base will be writable in all testing situations. We
-    // test these validators in our build tests, since those do give us control
-    // over the filesystem permissions.
-    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
-    return [
-      'automatic_updates.validator.file_system_permissions',
-      'package_manager.validator.file_system',
-    ];
+  protected function disableValidators(array $validators): void {
+    $services_file = $this->getDrupalRoot() . '/' . $this->siteDirectory . '/services.yml';
+    $this->assertFileIsWritable($services_file);
+    $services = file_get_contents($services_file);
+    $services = Yaml::decode($services);
+
+    foreach ($validators as $service_id) {
+      $services['services'][$service_id] = [
+        'class' => 'stdClass',
+      ];
+    }
+    file_put_contents($services_file, Yaml::encode($services));
+    // Ensure the container is rebuilt ASAP.
+    $this->kernel->invalidateContainer();
   }
 
   /**
diff --git a/tests/src/Functional/ReadinessValidationTest.php b/tests/src/Functional/ReadinessValidationTest.php
index 7f334f7d19..93334493b4 100644
--- a/tests/src/Functional/ReadinessValidationTest.php
+++ b/tests/src/Functional/ReadinessValidationTest.php
@@ -70,21 +70,6 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
     $this->drupalLogin($this->reportViewerUser);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function disableValidators(): array {
-    $disable_validators = parent::disableValidators();
-    // Because all actual staging operations are bypassed by
-    // package_manager_bypass, disable this validator because it will complain
-    // if there's no actual Composer data to inspect.
-    // @todo Do this in ::testStoredResultsClearedAfterUpdate() only once
-    //   https://www.drupal.org/project/automatic_updates/issues/3260698 is
-    //   fixed.
-    $disable_validators[] = 'automatic_updates.staged_projects_validator';
-    return $disable_validators;
-  }
-
   /**
    * Tests readiness checkers on status report page.
    */
@@ -388,6 +373,11 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
    * Tests that stored validation results are deleted after an update.
    */
   public function testStoredResultsClearedAfterUpdate(): void {
+    // Because all actual staging operations are bypassed by
+    // package_manager_bypass, disable this validator because it will complain
+    // if there's no actual Composer data to inspect.
+    $this->disableValidators(['automatic_updates.staged_projects_validator']);
+
     $assert_session = $this->assertSession();
     $page = $this->getSession()->getPage();
     $this->drupalLogin($this->checkerRunnerUser);
diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php
index 82eef1933d..71140ad703 100644
--- a/tests/src/Functional/UpdaterFormTest.php
+++ b/tests/src/Functional/UpdaterFormTest.php
@@ -6,8 +6,8 @@ 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;
+use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 
 /**
  * @covers \Drupal\automatic_updates\Form\UpdaterForm
@@ -16,8 +16,8 @@ use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait;
  */
 class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
 
-  use ValidationTestTrait;
   use PackageManagerBypassTestTrait;
+  use ValidationTestTrait;
 
   /**
    * {@inheritdoc}
@@ -38,6 +38,11 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
    * {@inheritdoc}
    */
   protected function setUp(): void {
+    // In this test class, all actual staging operations are bypassed by
+    // package_manager_bypass, which means this validator will complain because
+    // there is no actual Composer data for it to inspect.
+    $this->disableValidators[] = 'automatic_updates.staged_projects_validator';
+
     parent::setUp();
 
     $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1-security.xml');
@@ -45,20 +50,6 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $this->checkForUpdates();
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function disableValidators(): array {
-    $disabled_validators = parent::disableValidators();
-
-    // In this test class, all actual staging operations are bypassed by
-    // package_manager_bypass, which means this validator will complain because
-    // there is no actual Composer data for it to inspect.
-    $disabled_validators[] = 'automatic_updates.staged_projects_validator';
-
-    return $disabled_validators;
-  }
-
   /**
    * Data provider for URLs to the update form.
    *
diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
index 3625f62a32..1ba9df56bf 100644
--- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
+++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
@@ -25,6 +25,21 @@ abstract class AutomaticUpdatesKernelTestBase extends KernelTestBase {
    */
   protected static $modules = ['system', 'update', 'update_test'];
 
+  /**
+   * The service IDs of any validators to disable.
+   *
+   * @var string[]
+   */
+  protected $disableValidators = [
+    // Disable the filesystem permissions validator, since we cannot guarantee
+    // that the current code base will be writable in all testing situations.
+    // We test this validator functionally in our build tests, since those do
+    // give us control over the filesystem permissions.
+    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
+    'automatic_updates.validator.file_system_permissions',
+    'package_manager.validator.file_system',
+  ];
+
   /**
    * The mocked HTTP client that returns metadata about available updates.
    *
@@ -93,20 +108,11 @@ abstract class AutomaticUpdatesKernelTestBase extends KernelTestBase {
       $container->set('http_client', $this->client);
     }
 
-    $this->disableValidators($container);
-  }
-
-  /**
-   * Disables any validators that will interfere with this test.
-   */
-  protected function disableValidators(ContainerBuilder $container): void {
-    // Disable the filesystem permissions validator, since we cannot guarantee
-    // that the current code base will be writable in all testing situations.
-    // We test this validator functionally in our build tests, since those do
-    // give us control over the filesystem permissions.
-    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
-    $container->removeDefinition('automatic_updates.validator.file_system_permissions');
-    $container->removeDefinition('package_manager.validator.file_system');
+    foreach ($this->disableValidators as $service_id) {
+      if ($container->hasDefinition($service_id)) {
+        $container->getDefinition($service_id)->clearTag('event_subscriber');
+      }
+    }
   }
 
   /**
diff --git a/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php b/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php
index 7b2fd421b6..3d8e0c7210 100644
--- a/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php
+++ b/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php
@@ -3,7 +3,6 @@
 namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
 
 use Drupal\automatic_updates\Event\ReadinessCheckEvent;
-use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\package_manager\Validator\PreOperationStageValidatorInterface;
 use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase;
 use Prophecy\Argument;
@@ -33,9 +32,10 @@ class PackageManagerReadinessChecksTest extends AutomaticUpdatesKernelTestBase {
   /**
    * {@inheritdoc}
    */
-  protected function disableValidators(ContainerBuilder $container): void {
-    // No need to disable any validators in this test.
-  }
+  protected $disableValidators = [
+    // The parent class disables one of the validators that we're testing, so
+    // prevent that with an empty array.
+  ];
 
   /**
    * Data provider for ::testValidatorInvoked().
diff --git a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
index 13db2035d0..3799a239b9 100644
--- a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
@@ -30,22 +30,21 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
   /**
    * {@inheritdoc}
    */
-  public function register(ContainerBuilder $container) {
-    parent::register($container);
-
-    $container->getDefinition('automatic_updates.updater')
-      ->setClass(TestUpdater::class);
+  protected function setUp(): void {
+    // This test deals with fake sites that don't necessarily have lock files,
+    // so disable lock file validation.
+    $this->disableValidators[] = 'package_manager.validator.lock_file';
+    parent::setUp();
   }
 
   /**
    * {@inheritdoc}
    */
-  protected function disableValidators(ContainerBuilder $container): void {
-    parent::disableValidators($container);
+  public function register(ContainerBuilder $container) {
+    parent::register($container);
 
-    // This test deals with fake sites that don't necessarily have lock files,
-    // so disable lock file validation.
-    $container->removeDefinition('package_manager.validator.lock_file');
+    $container->getDefinition('automatic_updates.updater')
+      ->setClass(TestUpdater::class);
   }
 
   /**
-- 
GitLab