From d72467538117eb62682b70cf928147731dcb3f17 Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Tue, 12 Apr 2022 17:55:14 +0000 Subject: [PATCH] Issue #3274858 by phenaproxima: CoreUpdateTest should test that default.settings.php and default.services.yml can be updated --- .../SiteConfigurationExcluder.php | 32 ++++++------------- .../SiteConfigurationExcluderTest.php | 9 +++--- tests/src/Build/CoreUpdateTest.php | 32 ++++++++++++++++--- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/package_manager/src/PathExcluder/SiteConfigurationExcluder.php b/package_manager/src/PathExcluder/SiteConfigurationExcluder.php index 4ed7a8729b..68aa30831e 100644 --- a/package_manager/src/PathExcluder/SiteConfigurationExcluder.php +++ b/package_manager/src/PathExcluder/SiteConfigurationExcluder.php @@ -36,43 +36,29 @@ class SiteConfigurationExcluder implements EventSubscriberInterface { } /** - * Excludes common paths from staging operations. + * Excludes site configuration files from staging operations. * * @param \Drupal\package_manager\Event\PreApplyEvent|\Drupal\package_manager\Event\PreCreateEvent $event * The event object. * * @see \Drupal\package_manager\Event\ExcludedPathsTrait::excludePath() */ - public function ignoreCommonPaths(StageEvent $event): void { + public function excludeSiteConfiguration(StageEvent $event): void { // Site configuration files are always excluded relative to the web root. - $web = []; + $paths = []; // Ignore site-specific settings files, which are always in the web root. + // By default, Drupal core will always try to write-protect these files. $settings_files = [ 'settings.php', 'settings.local.php', 'services.yml', ]; foreach ($settings_files as $settings_file) { - $web[] = $this->sitePath . '/' . $settings_file; - $web[] = 'sites/default/' . $settings_file; + $paths[] = $this->sitePath . '/' . $settings_file; + $paths[] = 'sites/default/' . $settings_file; } - - $this->excludeInWebRoot($event, $web); - } - - /** - * Reacts before staged changes are committed the active directory. - * - * @param \Drupal\package_manager\Event\PreApplyEvent $event - * The event object. - */ - public function preApply(PreApplyEvent $event): void { - // Don't copy anything from the staging area's sites/default. - // @todo Make this a lot smarter in https://www.drupal.org/i/3228955. - $this->excludeInWebRoot($event, ['sites/default']); - - $this->ignoreCommonPaths($event); + $this->excludeInWebRoot($event, $paths); } /** @@ -80,8 +66,8 @@ class SiteConfigurationExcluder implements EventSubscriberInterface { */ public static function getSubscribedEvents() { return [ - PreCreateEvent::class => 'ignoreCommonPaths', - PreApplyEvent::class => 'preApply', + PreCreateEvent::class => 'excludeSiteConfiguration', + PreApplyEvent::class => 'excludeSiteConfiguration', ]; } diff --git a/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php index 095461dcd1..95c4352e4c 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/SiteConfigurationExcluderTest.php @@ -75,13 +75,12 @@ class SiteConfigurationExcluderTest extends PackageManagerKernelTestBase { // Regular module files should be staged. $this->assertFileExists("$stage_dir/modules/example/example.info.yml"); - // A new file added to the staging area in an excluded directory, should not - // be copied to the active directory. - $file = "$stage_dir/sites/default/no-copy.txt"; + // A new file added to the site directory in the staging area should be + // copied to the active directory. + $file = "$stage_dir/sites/default/new.txt"; touch($file); - $this->assertFileExists($file); $stage->apply(); - $this->assertFileDoesNotExist("$active_dir/sites/default/no-copy.txt"); + $this->assertFileExists("$active_dir/sites/default/new.txt"); // The ignored files should still be in the active directory. foreach ($ignore as $path) { diff --git a/tests/src/Build/CoreUpdateTest.php b/tests/src/Build/CoreUpdateTest.php index 2edd328b27..7e32c651fb 100644 --- a/tests/src/Build/CoreUpdateTest.php +++ b/tests/src/Build/CoreUpdateTest.php @@ -51,6 +51,9 @@ class CoreUpdateTest extends UpdateTestBase { $this->checkForUpdates(); $this->visit('/admin/modules/automatic-update'); $this->getMink()->assertSession()->pageTextContains('9.8.1'); + + // Ensure that Drupal has write-protected the site directory. + $this->assertDirectoryIsNotWritable($this->getWebRoot() . '/sites/default'); } /** @@ -184,10 +187,20 @@ class CoreUpdateTest extends UpdateTestBase { } // Change the \Drupal::VERSION constant and put placeholder text in the - // README so we can ensure that we really updated to the correct version. + // README so we can ensure that we really updated to the correct version. We + // also change the default site configuration files so we can ensure that + // these are updated as well, despite `sites/default` being write-protected. // @see ::assertUpdateSuccessful() + // @see ::createTestProject() Composer::setDrupalVersion($workspace_dir, $version); file_put_contents("$workspace_dir/core/README.txt", "Placeholder for Drupal core $version."); + + foreach (['default.settings.php', 'default.services.yml'] as $file) { + $file = fopen("$workspace_dir/core/assets/scaffold/files/$file", 'a'); + $this->assertIsResource($file); + fwrite($file, "# This is part of Drupal $version.\n"); + fclose($file); + } } /** @@ -225,12 +238,23 @@ class CoreUpdateTest extends UpdateTestBase { $this->getMink()->assertSession()->pageTextContains('No update available'); // The status page should report that we're running the expected version and - // the README should contain the placeholder text written by - // ::setUpstreamCoreVersion(). + // the README and default site configuration files should contain the + // placeholder text written by ::setUpstreamCoreVersion(), even though + // `sites/default` is write-protected. + // @see ::createTestProject() + // @see ::setUpstreamCoreVersion() $this->assertCoreVersion($expected_version); - $placeholder = file_get_contents($this->getWebRoot() . '/core/README.txt'); + $web_root = $this->getWebRoot(); + $placeholder = file_get_contents("$web_root/core/README.txt"); $this->assertSame("Placeholder for Drupal core $expected_version.", $placeholder); + foreach (['default.settings.php', 'default.services.yml'] as $file) { + $file = $web_root . '/sites/default/' . $file; + $this->assertFileIsReadable($file); + $this->assertStringContainsString("# This is part of Drupal $expected_version.", file_get_contents($file)); + } + $this->assertDirectoryIsNotWritable("$web_root/sites/default"); + $info = $this->runComposer('composer info --self --format json', 'project', TRUE); // The production dependencies should have been updated. -- GitLab