From 49fb3de6bba2c21d33be8bf7241f24c6c8eaf36d Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Thu, 14 Jul 2022 20:55:19 +0000 Subject: [PATCH] Issue #3294600 by phenaproxima, tedbow: Functional tests should use a fake site fixture by default --- .../tests/src/Functional/UpdaterFormTest.php | 2 + .../PackageManagerBypassServiceProvider.php | 4 ++ .../src/PathLocator.php | 71 +++++++++++++++++++ .../Traits/PackageManagerBypassTestTrait.php | 10 +-- .../AutomaticUpdatesFunctionalTestBase.php | 23 +++--- .../Functional/AvailableUpdatesReportTest.php | 5 -- .../Functional/ReadinessValidationTest.php | 8 --- tests/src/Functional/UpdaterFormTest.php | 7 +- 8 files changed, 95 insertions(+), 35 deletions(-) create mode 100644 package_manager/tests/modules/package_manager_bypass/src/PathLocator.php diff --git a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php index 18c5173098..0457f014a4 100644 --- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php +++ b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php @@ -4,6 +4,7 @@ namespace Drupal\Tests\automatic_updates_extensions\Functional; use Drupal\automatic_updates\Event\ReadinessCheckEvent; use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1; +use Drupal\automatic_updates_test\StagedDatabaseUpdateValidator; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\ValidationResult; use Drupal\package_manager_bypass\Beginner; @@ -151,6 +152,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $state->set('system.maintenance_mode', $maintenance_mode_on); Beginner::setFixturePath(__DIR__ . '/../../fixtures/fake-site'); + StagedDatabaseUpdateValidator::setExtensionsWithUpdates(['system', 'automatic_updates_theme_with_updates']); $page = $this->getSession()->getPage(); // Navigate to the automatic updates form. diff --git a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php index 51a0d5d66e..2d7866983f 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php +++ b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php @@ -30,6 +30,10 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase { foreach ($services as $id => $class) { $container->getDefinition($id)->setClass($class)->setArguments($arguments); } + + $container->getDefinition('package_manager.path_locator') + ->setClass(PathLocator::class) + ->addArgument($arguments[0]); } } diff --git a/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php b/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php new file mode 100644 index 0000000000..8d985308b6 --- /dev/null +++ b/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php @@ -0,0 +1,71 @@ +<?php + +namespace Drupal\package_manager_bypass; + +use Drupal\Core\State\StateInterface; +use Drupal\package_manager\PathLocator as BasePathLocator; + +/** + * Overrides the path locator to return pre-set values for testing purposes. + */ +class PathLocator extends BasePathLocator { + + /** + * The state service. + * + * @var \Drupal\Core\State\StateInterface + */ + private $state; + + /** + * Constructs a PathLocator object. + * + * @param string $app_root + * The Drupal application root. + * @param \Drupal\Core\State\StateInterface $state + * The state service. + */ + public function __construct(string $app_root, StateInterface $state) { + parent::__construct($app_root); + $this->state = $state; + } + + /** + * {@inheritdoc} + */ + public function getProjectRoot(): string { + return $this->state->get(static::class . ' root', parent::getProjectRoot()); + } + + /** + * {@inheritdoc} + */ + public function getVendorDirectory(): string { + return $this->state->get(static::class . ' vendor', parent::getVendorDirectory()); + } + + /** + * {@inheritdoc} + */ + public function getWebRoot(): string { + return $this->state->get(static::class . ' web', parent::getWebRoot()); + } + + /** + * Sets the paths to return. + * + * @param string|null $project_root + * The project root, or NULL to defer to the parent class. + * @param string|null $vendor_dir + * The vendor directory, or NULL to defer to the parent class. + * @param string|null $web_root + * The web root, relative to the project root, or NULL to defer to the + * parent class. + */ + public function setPaths(?string $project_root, ?string $vendor_dir, ?string $web_root): void { + $this->state->set(static::class . ' root', $project_root); + $this->state->set(static::class . ' vendor', $vendor_dir); + $this->state->set(static::class . ' web', $web_root); + } + +} diff --git a/package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php b/package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php index 0c1f93fde7..7e52efa8a0 100644 --- a/package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php +++ b/package_manager/tests/src/Traits/PackageManagerBypassTestTrait.php @@ -20,10 +20,12 @@ trait PackageManagerBypassTestTrait { /** @var \Drupal\package_manager_bypass\BypassedStagerServiceBase $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()); + // If an update was attempted, then there will be at least two calls to the + // stager: one to change the runtime constraints in composer.json, and + // another to actually update the installed dependencies. If any dev + // packages (like `drupal/core-dev`) are installed, there may also be an + // additional call to change the dev constraints. + $this->assertGreaterThanOrEqual($attempted_times * 2, count($stager->getInvocationArguments())); /** @var \Drupal\package_manager_bypass\BypassedStagerServiceBase $committer */ $committer = $this->container->get('package_manager.committer'); diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index 79e1682d6e..7109612ba2 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -27,13 +27,6 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { * @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', // Disable the Composer executable validator, since it may cause the tests // to fail if a supported version of Composer is unavailable to the web // server. This should be okay in most situations because, apart from the @@ -43,10 +36,6 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { 'package_manager.validator.composer_executable', // Always allow tests to run with Xdebug on. 'automatic_updates.validator.xdebug', - // Disable the symlink validator, since the running code base may contain - // symlinks that don't affect functional testing. - 'automatic_updates.validator.symlink', - 'package_manager.validator.symlink', ]; /** @@ -55,7 +44,17 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { protected function setUp() { parent::setUp(); $this->disableValidators($this->disableValidators); - Beginner::setFixturePath(__DIR__ . '/../../fixtures/fake-site'); + + $fixture_dir = __DIR__ . '/../../fixtures/fake-site'; + // We cannot guarantee that the fixture directory will be writable in all + // testing situations, so to keep things consistent, fail the test if it + // isn't. Our build tests examine file system permissions more extensively. + // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError() + $this->assertDirectoryIsWritable($fixture_dir); + + Beginner::setFixturePath($fixture_dir); + $this->container->get('package_manager.path_locator') + ->setPaths($fixture_dir, $fixture_dir . '/vendor', ''); } /** diff --git a/tests/src/Functional/AvailableUpdatesReportTest.php b/tests/src/Functional/AvailableUpdatesReportTest.php index 327a3fe351..c48fa281c0 100644 --- a/tests/src/Functional/AvailableUpdatesReportTest.php +++ b/tests/src/Functional/AvailableUpdatesReportTest.php @@ -29,11 +29,6 @@ class AvailableUpdatesReportTest 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(); $user = $this->createUser([ 'administer site configuration', diff --git a/tests/src/Functional/ReadinessValidationTest.php b/tests/src/Functional/ReadinessValidationTest.php index 86e2ce4f44..46e48cccde 100644 --- a/tests/src/Functional/ReadinessValidationTest.php +++ b/tests/src/Functional/ReadinessValidationTest.php @@ -400,14 +400,6 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase { 'automatic_updates', 'automatic_updates_test', ]); - // Because all actual staging operations are bypassed by - // package_manager_bypass (enabled by the parent class), disable these - // validators because they will complain if there's no actual Composer data - // to inspect. - $this->disableValidators([ - 'automatic_updates.staged_projects_validator', - 'automatic_updates.validator.scaffold_file_permissions', - ]); // The error should be persistently visible, even after the checker stops // flagging it. diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index 746958a148..b09ddf9534 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -42,11 +42,6 @@ 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 these validators will complain - // because there is no actual Composer data for them to inspect. - $this->disableValidators[] = 'automatic_updates.staged_projects_validator'; - parent::setUp(); $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1-security.xml'); @@ -62,7 +57,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { // BEGIN: DELETE FROM CORE MERGE REQUEST // Check for permission that was added in Drupal core 9.4.x. $available_permissions = array_keys($this->container->get('user.permissions')->getPermissions()); - if (!in_array('view update notifications', $available_permissions)) { + if (!in_array('view update notifications', $available_permissions, TRUE)) { array_pop($permissions); } // END: DELETE FROM CORE MERGE REQUEST -- GitLab