From faed578f1bf31afa803160eb3e552a1e51b44361 Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Thu, 21 Jul 2022 16:02:03 +0000 Subject: [PATCH] Issue #3296074 by phenaproxima: Kernel tests should use package_manager_bypass' path locator instead of mocking it --- .../tests/src/Kernel/ExtensionUpdaterTest.php | 6 +- .../PackageManagerBypassServiceProvider.php | 17 +++-- .../Kernel/PackageManagerKernelTestBase.php | 73 ++++++------------- .../Kernel/PathExcluder/GitExcluderTest.php | 4 +- .../SiteConfigurationExcluderTest.php | 4 +- .../PathExcluder/SiteFilesExcluderTest.php | 4 +- .../SqliteDatabaseExcluderTest.php | 4 +- .../PathExcluder/TestSiteExcluderTest.php | 4 +- .../VendorHardeningExcluderTest.php | 4 +- .../tests/src/Kernel/StageOwnershipTest.php | 8 -- .../StagedDatabaseUpdateValidatorTest.php | 4 +- tests/src/Kernel/UpdaterTest.php | 6 +- 12 files changed, 50 insertions(+), 88 deletions(-) diff --git a/automatic_updates_extensions/tests/src/Kernel/ExtensionUpdaterTest.php b/automatic_updates_extensions/tests/src/Kernel/ExtensionUpdaterTest.php index e664d5d5c3..d863afa1b1 100644 --- a/automatic_updates_extensions/tests/src/Kernel/ExtensionUpdaterTest.php +++ b/automatic_updates_extensions/tests/src/Kernel/ExtensionUpdaterTest.php @@ -46,8 +46,7 @@ class ExtensionUpdaterTest extends AutomaticUpdatesKernelTestBase { $user = $this->createUser([], NULL, TRUE, ['uid' => 2]); $this->setCurrentUser($user); - $fixture_dir = __DIR__ . '/../../fixtures/fake-site'; - $locator = $this->mockPathLocator($fixture_dir, $fixture_dir); + $this->createVirtualProject(__DIR__ . '/../../fixtures/fake-site'); $id = $this->container->get('automatic_updates_extensions.updater')->begin([ 'my_module' => '9.8.1', @@ -60,8 +59,7 @@ class ExtensionUpdaterTest extends AutomaticUpdatesKernelTestBase { $kernel = $this->container->get('kernel'); $kernel->rebuildContainer(); $this->container = $kernel->getContainer(); - // Keep using the mocked path locator and current user. - $this->container->set('package_manager.path_locator', $locator); + // Keep using the user account we created. $this->setCurrentUser($user); $extension_updater = $this->container->get('automatic_updates_extensions.updater'); 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 2d7866983f..b526d5242c 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php +++ b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php @@ -4,6 +4,7 @@ namespace Drupal\package_manager_bypass; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\DependencyInjection\ServiceProviderBase; +use Drupal\Core\Site\Settings; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\Filesystem\Filesystem; @@ -18,17 +19,19 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase { public function alter(ContainerBuilder $container) { parent::alter($container); - $services = [ - 'package_manager.beginner' => Beginner::class, - 'package_manager.stager' => Stager::class, - 'package_manager.committer' => Committer::class, - ]; $arguments = [ new Reference('state'), new Reference(Filesystem::class), ]; - foreach ($services as $id => $class) { - $container->getDefinition($id)->setClass($class)->setArguments($arguments); + if (Settings::get('package_manager_bypass_composer_stager', TRUE)) { + $services = [ + 'package_manager.beginner' => Beginner::class, + 'package_manager.stager' => Stager::class, + 'package_manager.committer' => Committer::class, + ]; + foreach ($services as $id => $class) { + $container->getDefinition($id)->setClass($class)->setArguments($arguments); + } } $container->getDefinition('package_manager.path_locator') diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index 281ce40da2..f4a0f22fd9 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -8,7 +8,6 @@ use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\Validator\DiskSpaceValidator; use Drupal\package_manager\Exception\StageException; use Drupal\package_manager\Exception\StageValidationException; -use Drupal\package_manager\PathLocator; use Drupal\package_manager\Stage; use Drupal\package_manager_bypass\Beginner; use Drupal\Tests\package_manager\Traits\ValidationTestTrait; @@ -77,19 +76,14 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $container->setDefinition($class, $definition->setPublic(FALSE)); $container->setAlias(PathFactoryInterface::class, $class); - // When a virtual project is used, the path locator and disk space validator - // are replaced with mocks. When staged changes are applied, the container - // is rebuilt, which destroys the mocked services and can cause unexpected - // side effects. The 'persist' tag prevents the mocks from being destroyed - // during a container rebuild. + // When a virtual project is used, the disk space validator is replaced with + // a mock. When staged changes are applied, the container is rebuilt, which + // destroys the mocked service and can cause unexpected side effects. The + // 'persist' tag prevents the mock from being destroyed during a container + // rebuild. // @see ::createVirtualProject() - $persist = [ - 'package_manager.path_locator', - 'package_manager.validator.disk_space', - ]; - foreach ($persist as $service_id) { - $container->getDefinition($service_id)->addTag('persist'); - } + $container->getDefinition('package_manager.validator.disk_space') + ->addTag('persist'); foreach ($this->disableValidators as $service_id) { if ($container->hasDefinition($service_id)) { @@ -173,12 +167,18 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { * 'active', which is the active directory containing a fake Drupal code base, * and 'stage', which is the root directory used to stage changes. The path * locator service will also be mocked so that it points to the test project. + * + * @param string|null $source_dir + * (optional) The path of a directory which should be copied into the + * virtual file system and used as the active directory. */ - protected function createVirtualProject(): void { + protected function createVirtualProject(?string $source_dir = NULL): void { + $source_dir = $source_dir ?? __DIR__ . '/../../fixtures/fake_site'; + // Create the active directory and copy its contents from a fixture. $active_dir = vfsStream::newDirectory('active'); $this->vfsRoot->addChild($active_dir); - vfsStream::copyFromFileSystem(__DIR__ . '/../../fixtures/fake_site', $active_dir); + vfsStream::copyFromFileSystem($source_dir, $active_dir); // Because we can't commit physical `.git` directories into the fixture, use // a visitor to traverse the virtual file system and rename all `_git` @@ -209,8 +209,13 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $this->vfsRoot->addChild($stage_dir); static::$testStagingRoot = $stage_dir->url(); + // Ensure the path locator points to the virtual active directory. We assume + // that is its own web root and that the vendor directory is at its top + // level. $active_dir = $active_dir->url(); - $path_locator = $this->mockPathLocator($active_dir); + /** @var \Drupal\package_manager_bypass\PathLocator $path_locator */ + $path_locator = $this->container->get('package_manager.path_locator'); + $path_locator->setPaths($active_dir, $active_dir . '/vendor', ''); // Ensure the active directory will be copied into the virtual staging area. Beginner::setFixturePath($active_dir); @@ -221,7 +226,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { // by vfsStream. This validator will persist through container rebuilds. // @see ::register() $validator = new TestDiskSpaceValidator( - $this->container->get('package_manager.path_locator'), + $path_locator, $this->container->get('string_translation') ); // By default, the validator should report that the root, vendor, and @@ -234,40 +239,6 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $this->container->set('package_manager.validator.disk_space', $validator); } - /** - * Mocks the path locator and injects it into the service container. - * - * The mocked path locator will persist through container rebuilds. - * - * @param string $project_root - * The project root. - * @param string|null $vendor_dir - * (optional) The vendor directory. Defaults to `$project_root/vendor`. - * @param string $web_root - * (optional) The web root, relative to the project root. Defaults to '' - * (i.e., same as the project root). - * - * @return \Drupal\package_manager\PathLocator - * The mocked path locator. - * - * @see ::register() - */ - protected function mockPathLocator(string $project_root, string $vendor_dir = NULL, string $web_root = ''): PathLocator { - if (empty($vendor_dir)) { - $vendor_dir = $project_root . '/vendor'; - } - $path_locator = $this->prophesize(PathLocator::class); - $path_locator->getProjectRoot()->willReturn($project_root); - $path_locator->getVendorDirectory()->willReturn($vendor_dir); - $path_locator->getWebRoot()->willReturn($web_root); - - // We don't need the prophet anymore. - $path_locator = $path_locator->reveal(); - $this->container->set('package_manager.path_locator', $path_locator); - - return $path_locator; - } - } /** diff --git a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php index 36007e400d..0966b1fa55 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php @@ -52,9 +52,9 @@ class GitExcluderTest extends PackageManagerKernelTestBase { public function testGitDirectoriesExcluded(): void { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php index 75f0b0cd3f..385e6132e6 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php @@ -40,9 +40,9 @@ class SiteConfigurationExcluderTest extends PackageManagerKernelTestBase { public function testExcludedPaths(): void { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/PathExcluder/SiteFilesExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/SiteFilesExcluderTest.php index 3dd86484ae..059c3115eb 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/SiteFilesExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/SiteFilesExcluderTest.php @@ -32,9 +32,9 @@ class SiteFilesExcluderTest extends PackageManagerKernelTestBase { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. This will also rebuild // the container, enabling the private stream wrapper. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/PathExcluder/SqliteDatabaseExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/SqliteDatabaseExcluderTest.php index bb1bc506cc..7b142714db 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/SqliteDatabaseExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/SqliteDatabaseExcluderTest.php @@ -57,9 +57,9 @@ class SqliteDatabaseExcluderTest extends PackageManagerKernelTestBase { public function testSqliteDatabaseFilesExcluded(): void { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/PathExcluder/TestSiteExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/TestSiteExcluderTest.php index 43a7ad9f06..4b53ba79ef 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/TestSiteExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/TestSiteExcluderTest.php @@ -28,9 +28,9 @@ class TestSiteExcluderTest extends PackageManagerKernelTestBase { public function testTestSitesExcluded(): void { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/PathExcluder/VendorHardeningExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/VendorHardeningExcluderTest.php index 0e28251ea7..9321842456 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/VendorHardeningExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/VendorHardeningExcluderTest.php @@ -28,9 +28,9 @@ class VendorHardeningExcluderTest extends PackageManagerKernelTestBase { public function testVendorHardeningFilesExcluded(): void { // In this test, we want to perform the actual staging operations so that we // can be sure that files are staged as expected. - $this->disableModules(['package_manager_bypass']); + $this->setSetting('package_manager_bypass_composer_stager', FALSE); // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); + $this->container = $this->container->get('kernel')->rebuildContainer(); $active_dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); diff --git a/package_manager/tests/src/Kernel/StageOwnershipTest.php b/package_manager/tests/src/Kernel/StageOwnershipTest.php index ccb4073793..39594018b3 100644 --- a/package_manager/tests/src/Kernel/StageOwnershipTest.php +++ b/package_manager/tests/src/Kernel/StageOwnershipTest.php @@ -239,14 +239,6 @@ class StageOwnershipTest extends PackageManagerKernelTestBase { * Tests that the stage is available if ::destroy() has a file system error. */ public function testStageDestroyedWithFileSystemError(): void { - // Enable the Composer Stager library, since we will actually want to create - // the stage directory. - $this->container->get('module_installer')->uninstall([ - 'package_manager_bypass', - ]); - // Ensure we have an up-to-date container. - $this->container = $this->container->get('kernel')->getContainer(); - $logger_channel = $this->container->get('logger.channel.file'); $arguments = [ $this->container->get('stream_wrapper_manager'), diff --git a/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php index bab2083783..2934a84bf0 100644 --- a/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php @@ -48,8 +48,8 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase { /** * {@inheritdoc} */ - protected function createVirtualProject(): void { - parent::createVirtualProject(); + protected function createVirtualProject(?string $source_dir = NULL): void { + parent::createVirtualProject($source_dir); $drupal_root = $this->getDrupalRoot(); $virtual_active_dir = $this->container->get('package_manager.path_locator') diff --git a/tests/src/Kernel/UpdaterTest.php b/tests/src/Kernel/UpdaterTest.php index 03a249e851..04344519a2 100644 --- a/tests/src/Kernel/UpdaterTest.php +++ b/tests/src/Kernel/UpdaterTest.php @@ -50,8 +50,7 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase { // Point to a fake site which requires Drupal core via a distribution. The // lock file should be scanned to determine the core packages, which should // result in drupal/core-recommended being updated. - $fixture_dir = __DIR__ . '/../../fixtures/fake-site'; - $locator = $this->mockPathLocator($fixture_dir, $fixture_dir); + $this->createVirtualProject(__DIR__ . '/../../fixtures/fake-site'); $id = $this->container->get('automatic_updates.updater')->begin([ 'drupal' => '9.8.1', @@ -61,8 +60,7 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase { $kernel = $this->container->get('kernel'); $kernel->rebuildContainer(); $this->container = $kernel->getContainer(); - // Keep using the mocked path locator and current user. - $this->container->set('package_manager.path_locator', $locator); + // Keep using the user account we created. $this->setCurrentUser($user); /** @var \Drupal\automatic_updates\Updater $updater */ -- GitLab