From 5ef2211afcbf49b6e9d6659c703afe7e4519cb11 Mon Sep 17 00:00:00 2001
From: Dave Long <dave@longwaveconsulting.com>
Date: Sat, 25 Nov 2023 11:33:10 +0000
Subject: [PATCH] Issue #3391776 by alexpott, smustgrave, catch: InfoParser
returns an empty array if passed a non-existing file
---
.../Core/Extension/InfoParserDynamic.php | 108 +++++++++---------
.../Core/Test/FunctionalTestSetupTrait.php | 2 -
.../Core/Theme/BaseThemeMissingTest.php | 78 ++-----------
.../Drupal/KernelTests/KernelTestBase.php | 3 -
.../Commands/TestSiteReleaseLocksCommand.php | 2 +
.../Commands/TestSiteTearDownCommand.php | 2 +
.../Tests/Core/Asset/CssOptimizerUnitTest.php | 5 -
.../Tests/Core/Command/QuickStartTest.php | 1 -
.../Tests/Core/Database/UrlConversionTest.php | 10 --
.../Core/Extension/InfoParserUnitTest.php | 5 +-
.../Tests/Scripts/TestSiteApplicationTest.php | 1 -
core/tests/bootstrap.php | 4 +
12 files changed, 74 insertions(+), 147 deletions(-)
diff --git a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php
index 8298223a374c..fc6e8a56265e 100644
--- a/core/lib/Drupal/Core/Extension/InfoParserDynamic.php
+++ b/core/lib/Drupal/Core/Extension/InfoParserDynamic.php
@@ -37,68 +37,68 @@ public function __construct(string $app_root = NULL) {
*/
public function parse($filename) {
if (!file_exists($filename)) {
- $parsed_info = [];
+ throw new InfoParserException("Unable to parse $filename as it does not exist");
}
- else {
- try {
- $parsed_info = Yaml::decode(file_get_contents($filename));
- }
- catch (InvalidDataTypeException $e) {
- throw new InfoParserException("Unable to parse $filename " . $e->getMessage());
- }
- $missing_keys = array_diff($this->getRequiredKeys(), array_keys($parsed_info));
- if (!empty($missing_keys)) {
- throw new InfoParserException('Missing required keys (' . implode(', ', $missing_keys) . ') in ' . $filename);
- }
- if (!isset($parsed_info['core_version_requirement'])) {
- if (str_starts_with($filename, 'core/') || str_starts_with($filename, $this->root . '/core/')) {
- // Core extensions do not need to specify core compatibility: they are
- // by definition compatible so a sensible default is used. Core
- // modules are allowed to provide these for testing purposes.
- $parsed_info['core_version_requirement'] = \Drupal::VERSION;
- }
- elseif (isset($parsed_info['package']) && $parsed_info['package'] === 'Testing') {
- // Modules in the testing package are exempt as well. This makes it
- // easier for contrib to use test modules.
- $parsed_info['core_version_requirement'] = \Drupal::VERSION;
- }
- else {
- // Non-core extensions must specify core compatibility.
- throw new InfoParserException("The 'core_version_requirement' key must be present in " . $filename);
- }
- }
- // Determine if the extension is compatible with the current version of
- // Drupal core.
- try {
- $parsed_info['core_incompatible'] = !Semver::satisfies(\Drupal::VERSION, $parsed_info['core_version_requirement']);
+ try {
+ $parsed_info = Yaml::decode(file_get_contents($filename));
+ }
+ catch (InvalidDataTypeException $e) {
+ throw new InfoParserException("Unable to parse $filename " . $e->getMessage());
+ }
+ $missing_keys = array_diff($this->getRequiredKeys(), array_keys($parsed_info));
+ if (!empty($missing_keys)) {
+ throw new InfoParserException('Missing required keys (' . implode(', ', $missing_keys) . ') in ' . $filename);
+ }
+ if (!isset($parsed_info['core_version_requirement'])) {
+ if (str_starts_with($filename, 'core/') || str_starts_with($filename, $this->root . '/core/')) {
+ // Core extensions do not need to specify core compatibility: they are
+ // by definition compatible so a sensible default is used. Core
+ // modules are allowed to provide these for testing purposes.
+ $parsed_info['core_version_requirement'] = \Drupal::VERSION;
}
- catch (\UnexpectedValueException $exception) {
- throw new InfoParserException("The 'core_version_requirement' constraint ({$parsed_info['core_version_requirement']}) is not a valid value in $filename");
+ elseif (isset($parsed_info['package']) && $parsed_info['package'] === 'Testing') {
+ // Modules in the testing package are exempt as well. This makes it
+ // easier for contrib to use test modules.
+ $parsed_info['core_version_requirement'] = \Drupal::VERSION;
}
- if (isset($parsed_info['version']) && $parsed_info['version'] === 'VERSION') {
- $parsed_info['version'] = \Drupal::VERSION;
+ else {
+ // Non-core extensions must specify core compatibility.
+ throw new InfoParserException("The 'core_version_requirement' key must be present in " . $filename);
}
- $parsed_info += [ExtensionLifecycle::LIFECYCLE_IDENTIFIER => ExtensionLifecycle::STABLE];
- $lifecycle = $parsed_info[ExtensionLifecycle::LIFECYCLE_IDENTIFIER];
- if (!ExtensionLifecycle::isValid($lifecycle)) {
- $valid_values = [
- ExtensionLifecycle::EXPERIMENTAL,
- ExtensionLifecycle::STABLE,
- ExtensionLifecycle::DEPRECATED,
- ExtensionLifecycle::OBSOLETE,
- ];
- throw new InfoParserException("'lifecycle: {$lifecycle}' is not valid in $filename. Valid values are: '" . implode("', '", $valid_values) . "'.");
+ }
+
+ // Determine if the extension is compatible with the current version of
+ // Drupal core.
+ try {
+ $parsed_info['core_incompatible'] = !Semver::satisfies(\Drupal::VERSION, $parsed_info['core_version_requirement']);
+ }
+ catch (\UnexpectedValueException $exception) {
+ throw new InfoParserException("The 'core_version_requirement' constraint ({$parsed_info['core_version_requirement']}) is not a valid value in $filename");
+ }
+ if (isset($parsed_info['version']) && $parsed_info['version'] === 'VERSION') {
+ $parsed_info['version'] = \Drupal::VERSION;
+ }
+ $parsed_info += [ExtensionLifecycle::LIFECYCLE_IDENTIFIER => ExtensionLifecycle::STABLE];
+ $lifecycle = $parsed_info[ExtensionLifecycle::LIFECYCLE_IDENTIFIER];
+ if (!ExtensionLifecycle::isValid($lifecycle)) {
+ $valid_values = [
+ ExtensionLifecycle::EXPERIMENTAL,
+ ExtensionLifecycle::STABLE,
+ ExtensionLifecycle::DEPRECATED,
+ ExtensionLifecycle::OBSOLETE,
+ ];
+ throw new InfoParserException("'lifecycle: {$lifecycle}' is not valid in $filename. Valid values are: '" . implode("', '", $valid_values) . "'.");
+ }
+ if (in_array($lifecycle, [ExtensionLifecycle::DEPRECATED, ExtensionLifecycle::OBSOLETE], TRUE)) {
+ if (empty($parsed_info[ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER])) {
+ throw new InfoParserException(sprintf("Extension %s (%s) has 'lifecycle: %s' but is missing a '%s' entry.", $parsed_info['name'], $filename, $lifecycle, ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER));
}
- if (in_array($lifecycle, [ExtensionLifecycle::DEPRECATED, ExtensionLifecycle::OBSOLETE], TRUE)) {
- if (empty($parsed_info[ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER])) {
- throw new InfoParserException(sprintf("Extension %s (%s) has 'lifecycle: %s' but is missing a '%s' entry.", $parsed_info['name'], $filename, $lifecycle, ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER));
- }
- if (!filter_var($parsed_info[ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER], FILTER_VALIDATE_URL)) {
- throw new InfoParserException(sprintf("Extension %s (%s) has a '%s' entry that is not a valid URL.", $parsed_info['name'], $filename, ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER));
- }
+ if (!filter_var($parsed_info[ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER], FILTER_VALIDATE_URL)) {
+ throw new InfoParserException(sprintf("Extension %s (%s) has a '%s' entry that is not a valid URL.", $parsed_info['name'], $filename, ExtensionLifecycle::LIFECYCLE_LINK_IDENTIFIER));
}
}
+
return $parsed_info;
}
diff --git a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
index 48c28243b024..2e7083ff86f0 100644
--- a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
+++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
@@ -633,8 +633,6 @@ protected function prepareEnvironment() {
$this->classLoader = require __DIR__ . '/../../../../../autoload.php';
$request = Request::createFromGlobals();
$kernel = TestRunnerKernel::createFromRequest($request, $this->classLoader);
- // TestRunnerKernel expects the working directory to be DRUPAL_ROOT.
- chdir(DRUPAL_ROOT);
$kernel->boot();
$kernel->preHandle($request);
$this->prepareDatabasePrefix();
diff --git a/core/tests/Drupal/KernelTests/Core/Theme/BaseThemeMissingTest.php b/core/tests/Drupal/KernelTests/Core/Theme/BaseThemeMissingTest.php
index 334bde6d1bac..44f7c5638cb5 100644
--- a/core/tests/Drupal/KernelTests/Core/Theme/BaseThemeMissingTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Theme/BaseThemeMissingTest.php
@@ -2,12 +2,9 @@
namespace Drupal\KernelTests\Core\Theme;
-use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\Extension\ExtensionDiscovery;
use Drupal\Core\Extension\InfoParserException;
-use Drupal\Core\Extension\ThemeExtensionList;
+use Drupal\Core\Site\Settings;
use Drupal\KernelTests\KernelTestBase;
-use org\bovigo\vfs\vfsStream;
/**
* Tests the behavior of a theme when base_theme info key is missing.
@@ -34,80 +31,23 @@ class BaseThemeMissingTest extends KernelTestBase {
protected function setUp(): void {
parent::setUp();
- $this->themeInstaller = $this->container->get('theme_installer');
- }
+ // Add a directory to extension discovery to find the theme with a missing
+ // base class.
+ // @see \Drupal\Core\Extension\ExtensionDiscovery::scan()
+ $settings = Settings::getAll();
+ $settings['test_parent_site'] = 'core/tests/fixtures/test_missing_base_theme';
+ new Settings($settings);
- /**
- * {@inheritdoc}
- */
- public function register(ContainerBuilder $container) {
- parent::register($container);
-
- $container->getDefinition('extension.list.theme')
- ->setClass(VfsThemeExtensionList::class);
- }
-
- /**
- * {@inheritdoc}
- */
- protected function setUpFilesystem() {
- parent::setUpFilesystem();
-
- $vfs_root = vfsStream::setup('core');
- vfsStream::create([
- 'themes' => [
- 'test_missing_base_theme' => [
- 'test_missing_base_theme.info.yml' => file_get_contents(DRUPAL_ROOT . '/core/tests/fixtures/test_missing_base_theme/test_missing_base_theme.info.yml'),
- 'test_missing_base_theme.theme' => file_get_contents(DRUPAL_ROOT . '/core/tests/fixtures/test_missing_base_theme/test_missing_base_theme.theme'),
- ],
- ],
- ], $vfs_root);
+ $this->themeInstaller = $this->container->get('theme_installer');
}
/**
* Tests exception is thrown.
*/
public function testMissingBaseThemeException() {
- $this->container->get('extension.list.theme')
- ->setExtensionDiscovery(new ExtensionDiscovery('vfs://core'));
-
$this->expectException(InfoParserException::class);
- $this->expectExceptionMessage('Missing required key ("base theme") in themes/test_missing_base_theme/test_missing_base_theme.info.yml, see https://www.drupal.org/node/3066038');
+ $this->expectExceptionMessage('Missing required key ("base theme") in core/tests/fixtures/test_missing_base_theme/test_missing_base_theme.info.yml, see https://www.drupal.org/node/3066038');
$this->themeInstaller->install(['test_missing_base_theme']);
}
}
-
-/**
- * Test theme extension list class.
- */
-class VfsThemeExtensionList extends ThemeExtensionList {
-
- /**
- * The extension discovery for this extension list.
- *
- * @var \Drupal\Core\Extension\ExtensionDiscovery
- */
- protected $extensionDiscovery;
-
- /**
- * Sets the extension discovery.
- *
- * @param \Drupal\Core\Extension\ExtensionDiscovery $discovery
- * The extension discovery.
- *
- * @return self
- */
- public function setExtensionDiscovery(ExtensionDiscovery $discovery) {
- $this->extensionDiscovery = $discovery;
- return $this;
- }
-
- /**
- * {@inheritdoc}
- */
- public function getExtensionDiscovery() {
- return $this->extensionDiscovery;
- }
-
-}
diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php
index 3acf47a5eccb..46b4d8801899 100644
--- a/core/tests/Drupal/KernelTests/KernelTestBase.php
+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -241,9 +241,6 @@ abstract class KernelTestBase extends TestCase implements ServiceProviderInterfa
public static function setUpBeforeClass(): void {
parent::setUpBeforeClass();
VarDumper::setHandler(TestVarDumper::class . '::cliHandler');
-
- // Change the current dir to DRUPAL_ROOT.
- chdir(static::getDrupalRoot());
}
/**
diff --git a/core/tests/Drupal/TestSite/Commands/TestSiteReleaseLocksCommand.php b/core/tests/Drupal/TestSite/Commands/TestSiteReleaseLocksCommand.php
index c88abb84ab18..f2b606aec75d 100644
--- a/core/tests/Drupal/TestSite/Commands/TestSiteReleaseLocksCommand.php
+++ b/core/tests/Drupal/TestSite/Commands/TestSiteReleaseLocksCommand.php
@@ -30,6 +30,8 @@ protected function configure() {
* {@inheritdoc}
*/
protected function execute(InputInterface $input, OutputInterface $output): int {
+ $root = dirname(__DIR__, 5);
+ chdir($root);
TestDatabase::releaseAllTestLocks();
$output->writeln('<info>Successfully released all the test database locks</info>');
return 0;
diff --git a/core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php b/core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php
index a2b5f54e3203..efde0539d947 100644
--- a/core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php
+++ b/core/tests/Drupal/TestSite/Commands/TestSiteTearDownCommand.php
@@ -38,6 +38,8 @@ protected function configure() {
* {@inheritdoc}
*/
protected function execute(InputInterface $input, OutputInterface $output): int {
+ $root = dirname(__DIR__, 5);
+ chdir($root);
$db_prefix = $input->getArgument('db-prefix');
// Validate the db_prefix argument.
try {
diff --git a/core/tests/Drupal/Tests/Core/Asset/CssOptimizerUnitTest.php b/core/tests/Drupal/Tests/Core/Asset/CssOptimizerUnitTest.php
index 347759527f1a..e4833ec41a2b 100644
--- a/core/tests/Drupal/Tests/Core/Asset/CssOptimizerUnitTest.php
+++ b/core/tests/Drupal/Tests/Core/Asset/CssOptimizerUnitTest.php
@@ -247,11 +247,6 @@ public function testOptimize($css_asset, $expected) {
$original_base_path = $base_path;
$base_path = '/';
- // \Drupal\Core\Asset\CssOptimizer::loadFile() relies on the current working
- // directory being the one that is used when index.php is the entry point.
- // Note: PHPUnit automatically restores the original working directory.
- chdir(realpath(__DIR__ . '/../../../../../../'));
-
$this->assertEquals($expected, $this->optimizer->optimize($css_asset), 'Group of file CSS assets optimized correctly.');
$base_path = $original_base_path;
diff --git a/core/tests/Drupal/Tests/Core/Command/QuickStartTest.php b/core/tests/Drupal/Tests/Core/Command/QuickStartTest.php
index 536a67398f1d..ff6c3c5462ac 100644
--- a/core/tests/Drupal/Tests/Core/Command/QuickStartTest.php
+++ b/core/tests/Drupal/Tests/Core/Command/QuickStartTest.php
@@ -54,7 +54,6 @@ protected function setUp(): void {
$php_executable_finder = new PhpExecutableFinder();
$this->php = $php_executable_finder->find();
$this->root = dirname(substr(__DIR__, 0, -strlen(__NAMESPACE__)), 2);
- chdir($this->root);
if (!is_writable("{$this->root}/sites/simpletest")) {
$this->markTestSkipped('This test requires a writable sites/simpletest directory');
}
diff --git a/core/tests/Drupal/Tests/Core/Database/UrlConversionTest.php b/core/tests/Drupal/Tests/Core/Database/UrlConversionTest.php
index c3f75582bace..9a93304e7841 100644
--- a/core/tests/Drupal/Tests/Core/Database/UrlConversionTest.php
+++ b/core/tests/Drupal/Tests/Core/Database/UrlConversionTest.php
@@ -21,16 +21,6 @@
*/
class UrlConversionTest extends UnitTestCase {
- /**
- * {@inheritdoc}
- */
- protected function setUp(): void {
- parent::setUp();
- $this->root = dirname(__FILE__, 7);
- // This unit test relies on reading files relative to Drupal root.
- chdir($this->root);
- }
-
/**
* @covers ::convertDbUrlToConnectionInfo
*
diff --git a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
index 8b51ed22c32d..c1cf06d64057 100644
--- a/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
+++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php
@@ -45,8 +45,9 @@ protected function setUp(): void {
*/
public function testInfoParserNonExisting() {
vfsStream::setup('modules');
- $info = $this->infoParser->parse(vfsStream::url('modules') . '/does_not_exist.info.txt');
- $this->assertEmpty($info, 'Non existing info.yml returns empty array.');
+ $this->expectException('\Drupal\Core\Extension\InfoParserException');
+ $this->expectExceptionMessage('Unable to parse vfs://modules/does_not_exist.info.txt as it does not exist');
+ $this->infoParser->parse(vfsStream::url('modules') . '/does_not_exist.info.txt');
}
/**
diff --git a/core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php b/core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php
index 490e49242ad7..d15ef7946e83 100644
--- a/core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php
+++ b/core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php
@@ -42,7 +42,6 @@ protected function setUp(): void {
parent::setUp();
$php_executable_finder = new PhpExecutableFinder();
$this->php = $php_executable_finder->find();
- $this->root = dirname(substr(__DIR__, 0, -strlen(__NAMESPACE__)), 2);
}
/**
diff --git a/core/tests/bootstrap.php b/core/tests/bootstrap.php
index 0ea13b0f3937..a5718d049a46 100644
--- a/core/tests/bootstrap.php
+++ b/core/tests/bootstrap.php
@@ -179,3 +179,7 @@ class_alias('\Drupal\Tests\DocumentElement', '\Behat\Mink\Element\DocumentElemen
$deprecation_ignore_filename = realpath(__DIR__ . "/../.deprecation-ignore.txt");
putenv("SYMFONY_DEPRECATIONS_HELPER=ignoreFile=$deprecation_ignore_filename");
}
+
+// Drupal expects to be run from its root directory. This ensures all test types
+// are consistent.
+chdir(dirname(__DIR__, 2));
--
GitLab