From 5be50483699e7c7717a0e9db658ac1c4c606f5c0 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Mon, 7 Jun 2021 16:10:08 +0100 Subject: [PATCH] Issue #3174200 by mondrake, longwave: Use PHPUnit-bridge polyfills for forward compatibility layer --- .../src/Functional/ImageAdminStylesTest.php | 2 +- .../Functional/ImageStylesPathAndUrlTest.php | 2 +- .../src/Functional/UpdateSemverCoreTest.php | 2 +- ...ConfigDirectorySetNoDirectoryErrorTest.php | 2 +- .../KernelTests/Core/File/DirectoryTest.php | 2 +- .../Core/File/FileDeleteRecursiveTest.php | 8 +- .../Core/Test/EnvironmentCleanerTest.php | 2 +- .../PhpUnit8/ClassWriter.php | 84 ++++++++++++++++--- .../Component/PhpStorage/FileStorageTest.php | 2 +- .../Listeners/DeprecationListenerTrait.php | 1 - 10 files changed, 84 insertions(+), 23 deletions(-) diff --git a/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php b/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php index cb9133b5eeb..27f005ed02d 100644 --- a/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php +++ b/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php @@ -292,7 +292,7 @@ class ImageAdminStylesTest extends ImageFieldTestBase { // Confirm the style directory has been removed. $directory = 'public://styles/' . $style_name; - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); $this->assertNull(ImageStyle::load($style_name), new FormattableMarkup('Image style %style successfully deleted.', ['%style' => $style->label()])); diff --git a/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php b/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php index 7b0f3baf809..fd388153b63 100644 --- a/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php +++ b/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php @@ -323,7 +323,7 @@ class ImageStylesPathAndUrlTest extends BrowserTestBase { // directories in the file system. $directory = $scheme . '://styles/' . $this->style->id() . '/' . $scheme . '/' . $this->randomMachineName(); $this->drupalGet(file_create_url($directory . '/' . $this->randomString())); - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); } } diff --git a/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php b/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php index 19a2546dc0b..d0708ffca8c 100644 --- a/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php +++ b/core/modules/update/tests/src/Functional/UpdateSemverCoreTest.php @@ -311,7 +311,7 @@ class UpdateSemverCoreTest extends UpdateSemverTestBase { ]; // Check that update directories does not exists. foreach ($directories as $directory) { - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); } // Method must not fail if update directories do not exists. diff --git a/core/tests/Drupal/FunctionalTests/Installer/InstallerConfigDirectorySetNoDirectoryErrorTest.php b/core/tests/Drupal/FunctionalTests/Installer/InstallerConfigDirectorySetNoDirectoryErrorTest.php index ece3bed7f52..7562eb8ba7e 100644 --- a/core/tests/Drupal/FunctionalTests/Installer/InstallerConfigDirectorySetNoDirectoryErrorTest.php +++ b/core/tests/Drupal/FunctionalTests/Installer/InstallerConfigDirectorySetNoDirectoryErrorTest.php @@ -60,7 +60,7 @@ class InstallerConfigDirectorySetNoDirectoryErrorTest extends InstallerTestBase */ public function testError() { $this->assertSession()->pageTextContains("An automated attempt to create the directory {$this->configDirectory}/sync failed, possibly due to a permissions problem."); - $this->assertDirectoryNotExists($this->configDirectory . '/sync'); + $this->assertDirectoryDoesNotExist($this->configDirectory . '/sync'); } } diff --git a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php index a496f902d76..d6105f57878 100644 --- a/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php @@ -64,7 +64,7 @@ class DirectoryTest extends FileTestBase { // A directory to operate on. $default_scheme = 'public'; $directory = $default_scheme . '://' . $this->randomMachineName() . '/' . $this->randomMachineName(); - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); // Non-existent directory. /** @var \Drupal\Core\File\FileSystemInterface $file_system */ diff --git a/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php b/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php index 99d9c76f9e7..224fcab96b4 100644 --- a/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php +++ b/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php @@ -31,7 +31,7 @@ class FileDeleteRecursiveTest extends FileTestBase { // Delete the directory. $this->assertTrue(\Drupal::service('file_system')->deleteRecursive($directory), 'Function reported success.'); - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); } /** @@ -49,7 +49,7 @@ class FileDeleteRecursiveTest extends FileTestBase { $this->assertTrue(\Drupal::service('file_system')->deleteRecursive($directory), 'Function reported success.'); $this->assertFileNotExists($filepathA); $this->assertFileNotExists($filepathB); - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($directory); } /** @@ -68,8 +68,8 @@ class FileDeleteRecursiveTest extends FileTestBase { $this->assertTrue(\Drupal::service('file_system')->deleteRecursive($directory), 'Function reported success.'); $this->assertFileNotExists($filepathA); $this->assertFileNotExists($filepathB); - $this->assertDirectoryNotExists($subdirectory); - $this->assertDirectoryNotExists($directory); + $this->assertDirectoryDoesNotExist($subdirectory); + $this->assertDirectoryDoesNotExist($directory); } } diff --git a/core/tests/Drupal/KernelTests/Core/Test/EnvironmentCleanerTest.php b/core/tests/Drupal/KernelTests/Core/Test/EnvironmentCleanerTest.php index 0ca66926007..f0f35586bae 100644 --- a/core/tests/Drupal/KernelTests/Core/Test/EnvironmentCleanerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Test/EnvironmentCleanerTest.php @@ -47,7 +47,7 @@ class EnvironmentCleanerTest extends KernelTestBase { $this->assertEquals(2, $do_cleanup_ref->invoke($cleaner)); - $this->assertDirectoryNotExists(vfsStream::url('cleanup_test/sites/simpletest/delete_dir')); + $this->assertDirectoryDoesNotExist(vfsStream::url('cleanup_test/sites/simpletest/delete_dir')); $this->assertFileNotExists(vfsStream::url('cleanup_test/sites/simpletest/delete_dir/delete.me')); $this->assertFileNotExists(vfsStream::url('cleanup_test/sites/simpletest/delete_me.too')); } diff --git a/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php b/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php index 063b9ff05bf..da085473572 100644 --- a/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php +++ b/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php @@ -2,6 +2,8 @@ namespace Drupal\TestTools\PhpUnitCompatibility\PhpUnit8; +use Composer\Autoload\ClassLoader; + /** * Helper class to rewrite PHPUnit's TestCase class. * @@ -21,7 +23,7 @@ final class ClassWriter { } /** - * Mutates the TestCase class from PHPUnit to make it compatible with Drupal. + * Mutates PHPUnit classes to make it compatible with Drupal. * * @param \Composer\Autoload\ClassLoader $autoloader * The autoloader. @@ -29,6 +31,44 @@ final class ClassWriter { * @throws \ReflectionException */ public static function mutateTestBase($autoloader) { + static::alterAssert($autoloader); + static::alterTestCase($autoloader); + } + + /** + * Alters the Assert class. + * + * @param \Composer\Autoload\ClassLoader $autoloader + * The autoloader. + * + * @throws \ReflectionException + */ + private static function alterAssert(ClassLoader $autoloader): void { + // If the class exists already there is nothing we can do. Hopefully this + // is happening because this has been called already. The call from + // \Drupal\Core\Test\TestDiscovery::registerTestNamespaces() necessitates + // this protection. + if (class_exists('PHPUnit\Framework\Assert', FALSE)) { + return; + } + // Mutate Assert code to make it forward compatible with different PhpUnit + // versions, by adding Symfony's PHPUnit-bridge PolyfillAssertTrait. + $alteredFile = $autoloader->findFile('PHPUnit\Framework\Assert'); + $phpunit_dir = dirname($alteredFile, 3); + $alteredCode = file_get_contents($alteredFile); + $alteredCode = preg_replace('/abstract class Assert[^\{]+\{/', '$0 ' . \PHP_EOL . " use \Symfony\Bridge\PhpUnit\Legacy\PolyfillAssertTrait;" . \PHP_EOL, $alteredCode, 1); + include static::flushAlteredCodeToFile('Assert.php', $alteredCode); + } + + /** + * Alters the TestCase class. + * + * @param \Composer\Autoload\ClassLoader $autoloader + * The autoloader. + * + * @throws \ReflectionException + */ + private static function alterTestCase(ClassLoader $autoloader): void { // If the class exists already there is nothing we can do. Hopefully this // is happening because this has been called already. The call from // \Drupal\Core\Test\TestDiscovery::registerTestNamespaces() necessitates @@ -36,24 +76,46 @@ final class ClassWriter { if (class_exists('PHPUnit\Framework\TestCase', FALSE)) { return; } - // Inspired by Symfony's simple-phpunit remove typehints from TestCase. + // Mutate TestCase code to make it forward compatible with different PhpUnit + // versions, by adding Symfony's PHPUnit-bridge PolyfillTestCaseTrait. $alteredFile = $autoloader->findFile('PHPUnit\Framework\TestCase'); $phpunit_dir = dirname($alteredFile, 3); - // Mutate TestCase code to make it compatible with Drupal 8 and 9 tests. $alteredCode = file_get_contents($alteredFile); - $alteredCode = preg_replace('/^ ((?:protected|public)(?: static)? function \w+\(\)): void/m', ' $1', $alteredCode); + $alteredCode = preg_replace('/abstract class TestCase[^\{]+\{/', '$0 ' . \PHP_EOL . " use \Symfony\Bridge\PhpUnit\Legacy\PolyfillTestCaseTrait;" . \PHP_EOL, $alteredCode, 1); $alteredCode = str_replace("__DIR__ . '/../Util/", "'$phpunit_dir/src/Util/", $alteredCode); - $simpletest_directory = __DIR__ . '/../../../../../../sites/simpletest'; + // While Drupal still allows methods in test base classes that inherit from + // TestCase with no void return typehints specified, we also alter TestCase + // to remove the typehints. + // @see https://www.drupal.org/project/drupal/issues/3182103 + $alteredCode = preg_replace('/^ ((?:protected|public)(?: static)? function \w+\(\)): void/m', ' $1', $alteredCode); + include static::flushAlteredCodeToFile('TestCase.php', $alteredCode); + } + + /** + * Flushes altered class code to file when necessary. + * + * @param string $file_name + * The file name. + * @param string $altered_code + * The altered code. + * + * @return string + * The full path of the file to be included. + */ + private static function flushAlteredCodeToFile(string $file_name, string $altered_code): string { + $directory = __DIR__ . '/../../../../../../sites/simpletest'; + $full_path = $directory . '/' . $file_name; + // Only write when necessary. - $filename = $simpletest_directory . '/TestCase.php'; - if (!file_exists($filename) || md5_file($filename) !== md5($alteredCode)) { + if (!file_exists($full_path) || md5_file($full_path) !== md5($altered_code)) { // Create directory when necessary. - if (!file_exists($simpletest_directory)) { - mkdir($simpletest_directory, 0777, TRUE); + if (!file_exists($directory)) { + mkdir($directory, 0777, TRUE); } - file_put_contents($filename, $alteredCode); + file_put_contents($full_path, $altered_code); } - include $filename; + + return $full_path; } } diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php index c17b1bb516d..2ad99b4b226 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php +++ b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php @@ -84,7 +84,7 @@ class FileStorageTest extends PhpStorageTestBase { $this->assertTrue($php->deleteAll(), 'Delete all reported success'); $this->assertFalse($php->load($name)); - $this->assertDirectoryNotExists($this->directory . '/test'); + $this->assertDirectoryDoesNotExist($this->directory . '/test'); // Should still return TRUE if directory has already been deleted. $this->assertTrue($php->deleteAll(), 'Delete all succeeds with nothing to delete'); diff --git a/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php index 69172797f2c..905560b34bf 100644 --- a/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php +++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php @@ -128,7 +128,6 @@ trait DeprecationListenerTrait { 'assertFileNotExists() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertFileDoesNotExist() instead.', 'assertRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertMatchesRegularExpression() instead.', 'assertNotRegExp() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertDoesNotMatchRegularExpression() instead.', - 'assertDirectoryNotExists() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertDirectoryDoesNotExist() instead.', 'Support for using expectException() with PHPUnit\\Framework\\Error\\Warning is deprecated and will be removed in PHPUnit 10. Use expectWarning() instead.', 'Support for using expectException() with PHPUnit\\Framework\\Error\\Error is deprecated and will be removed in PHPUnit 10. Use expectError() instead.', 'assertDirectoryNotIsWritable() is deprecated and will be removed in PHPUnit 10. Refactor your code to use assertDirectoryIsNotWritable() instead.',