Issue #3260698 by phenaproxima, tedbow: Make it simpler to disable validators in tests

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');
+      }
+    }
   }
 
   /**

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().
    *

tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml

-name: 'Automatic Updates Test: Disable validators'
-description: Allows certain update validators to be disabled during testing.
-type: module
-package: Testing

tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php

-<?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']);
-  }
-
-}

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();
   }
 
   /**

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);

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.
    *

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');
+      }
+    }
   }
 
   /**

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().

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);
   }
 
   /**