From cbc4593ab464cf43f6c0edfb9cde75f68a5740ce Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Wed, 17 Nov 2021 12:37:16 +1300 Subject: [PATCH] ENH Don't index test from PHPUNit 5.7 module wdon using PHPUnit 9.5 --- src/Core/CoreKernel.php | 30 +++++++++++++++++-- src/Core/Manifest/ClassLoader.php | 5 ++-- src/Core/Manifest/ClassManifest.php | 9 ++++-- src/Core/Manifest/ManifestFileFinder.php | 6 ++-- src/Core/Manifest/Module.php | 8 ++--- src/Core/Manifest/ModuleLoader.php | 5 ++-- src/Core/Manifest/ModuleManifest.php | 9 ++++-- src/Dev/SapphireTest.php | 4 +++ src/Dev/TestKernel.php | 20 +++++++++++++ src/View/ThemeManifest.php | 9 ++++-- .../Core/Manifest/ManifestFileFinderTest.php | 4 +-- tests/php/Core/Manifest/ModuleTest.php | 10 +++---- 12 files changed, 89 insertions(+), 30 deletions(-) diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index 885fb50a8..27af9a7a1 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -509,6 +509,18 @@ class CoreKernel implements Kernel return false; } + /** + * When manifests are discovering files, tests files in modules using the following CI library type will be ignored. + * + * The purpose of this method is to avoid loading PHPUnit test files with incompatible definitions. + * + * @return string[] List of CI types to ignore as defined by `Module`. + */ + protected function getIgnoreCILibraries(): array + { + return []; + } + /** * Boot all manifests * @@ -517,10 +529,18 @@ class CoreKernel implements Kernel protected function bootManifests($flush) { // Setup autoloader - $this->getClassLoader()->init($this->getIncludeTests(), $flush); + $this->getClassLoader()->init( + $this->getIncludeTests(), + $flush, + $this->getIgnoreCILibraries() + ); // Find modules - $this->getModuleLoader()->init($this->getIncludeTests(), $flush); + $this->getModuleLoader()->init( + $this->getIncludeTests(), + $flush, + $this->getIgnoreCILibraries() + ); // Flush config if ($flush) { @@ -538,7 +558,11 @@ class CoreKernel implements Kernel $defaultSet->setProject( ModuleManifest::config()->get('project') ); - $defaultSet->init($this->getIncludeTests(), $flush); + $defaultSet->init( + $this->getIncludeTests(), + $flush, + $this->getIgnoreCILibraries() + ); } } diff --git a/src/Core/Manifest/ClassLoader.php b/src/Core/Manifest/ClassLoader.php index 809b471c0..f25800909 100644 --- a/src/Core/Manifest/ClassLoader.php +++ b/src/Core/Manifest/ClassLoader.php @@ -120,13 +120,14 @@ class ClassLoader * * @param bool $includeTests * @param bool $forceRegen + * @param string[] $ignoredCILibs */ - public function init($includeTests = false, $forceRegen = false) + public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) { foreach ($this->manifests as $manifest) { /** @var ClassManifest $instance */ $instance = $manifest['instance']; - $instance->init($includeTests, $forceRegen); + $instance->init($includeTests, $forceRegen, $ignoredCILibs); } $this->registerAutoloader(); diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 49b7e3e75..99c02a1ff 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -260,8 +260,9 @@ class ClassManifest * * @param bool $includeTests * @param bool $forceRegen + * @param string[] $ignoredCILibs */ - public function init($includeTests = false, $forceRegen = false) + public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) { $this->cache = $this->buildCache($includeTests); @@ -275,7 +276,7 @@ class ClassManifest } // Build - $this->regenerate($includeTests); + $this->regenerate($includeTests, $ignoredCILibs); } /** @@ -500,8 +501,9 @@ class ClassManifest * Completely regenerates the manifest file. * * @param bool $includeTests + * @param string[] $ignoredCILibs */ - public function regenerate($includeTests) + public function regenerate($includeTests, array $ignoredCILibs = []) { // Reset the manifest so stale info doesn't cause errors. $this->loadState([]); @@ -513,6 +515,7 @@ class ClassManifest 'name_regex' => '/^[^_].*\\.php$/', 'ignore_files' => ['index.php', 'cli-script.php'], 'ignore_tests' => !$includeTests, + 'ignore_ci_libraries' => $ignoredCILibs, 'file_callback' => function ($basename, $pathname, $depth) use ($includeTests, $finder) { $this->handleFile($basename, $pathname, $includeTests); }, diff --git a/src/Core/Manifest/ManifestFileFinder.php b/src/Core/Manifest/ManifestFileFinder.php index 2e737bd01..bf04a7002 100644 --- a/src/Core/Manifest/ManifestFileFinder.php +++ b/src/Core/Manifest/ManifestFileFinder.php @@ -33,7 +33,7 @@ class ManifestFileFinder extends FileFinder 'ignore_tests' => true, 'min_depth' => 1, 'ignore_dirs' => ['node_modules'], - 'ignore_ci_library' => [] + 'ignore_ci_libraries' => [] ]; public function acceptDir($basename, $pathname, $depth) @@ -75,7 +75,7 @@ class ManifestFileFinder extends FileFinder } // Skip if test dir inside vendor module with unexpected CI library - if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCILib = $this->getOption('ignore_ci_library')) { + if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCILib = $this->getOption('ignore_ci_libraries')) { $ciLib = $this->findModuleCILib($basename, $pathname, $depth); if (in_array($ciLib, $ignoreCILib)) { return false; @@ -264,7 +264,7 @@ class ManifestFileFinder extends FileFinder private function findModuleCILib(string $basename, string $pathname, int $depth): string { if ($depth < 1) { - return Module::CI_PHPUNIT_UNKNOWN; + return Module::CI_UNKNOWN; } $newBasename = basename($pathname); diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index 71a29cde3..fa90f60ac 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -33,7 +33,7 @@ class Module implements Serializable /** * Return value of getCILibrary() when module does not use any CI */ - const CI_PHPUNIT_UNKNOWN = 'NoPHPUnit'; + const CI_UNKNOWN = 'NoPHPUnit'; @@ -302,12 +302,12 @@ class Module implements Serializable { // We don't have any composer data at all if (empty($this->composerData)) { - return self::CI_PHPUNIT_UNKNOWN; + return self::CI_UNKNOWN; } // We don't have any dev dependencies if (empty($this->composerData['require-dev']) || !is_array($this->composerData['require-dev'])) { - return self::CI_PHPUNIT_UNKNOWN; + return self::CI_UNKNOWN; } // We are assuming a typical setup where the CI lib is defined in require-dev rather than require @@ -335,7 +335,7 @@ class Module implements Serializable } } - return self::CI_PHPUNIT_UNKNOWN; + return self::CI_UNKNOWN; } /** diff --git a/src/Core/Manifest/ModuleLoader.php b/src/Core/Manifest/ModuleLoader.php index c5f77d77b..0f5cf1ffd 100644 --- a/src/Core/Manifest/ModuleLoader.php +++ b/src/Core/Manifest/ModuleLoader.php @@ -91,11 +91,12 @@ class ModuleLoader * * @param bool $includeTests * @param bool $forceRegen + * @param string[] $ignoredCILibs */ - public function init($includeTests = false, $forceRegen = false) + public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) { foreach ($this->manifests as $manifest) { - $manifest->init($includeTests, $forceRegen); + $manifest->init($includeTests, $forceRegen, $ignoredCILibs); } } } diff --git a/src/Core/Manifest/ModuleManifest.php b/src/Core/Manifest/ModuleManifest.php index 38acc4324..d5177070e 100644 --- a/src/Core/Manifest/ModuleManifest.php +++ b/src/Core/Manifest/ModuleManifest.php @@ -121,8 +121,9 @@ class ModuleManifest /** * @param bool $includeTests * @param bool $forceRegen Force the manifest to be regenerated. + * @param string[] $ignoredCILibs */ - public function init($includeTests = false, $forceRegen = false) + public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) { // build cache from factory if ($this->cacheFactory) { @@ -137,7 +138,7 @@ class ModuleManifest $this->modules = $this->cache->get($this->cacheKey) ?: []; } if (empty($this->modules)) { - $this->regenerate($includeTests); + $this->regenerate($includeTests, $ignoredCILibs); } } @@ -162,8 +163,9 @@ class ModuleManifest * Does _not_ build the actual variant * * @param bool $includeTests + * @param string[] $ignoredCILibs */ - public function regenerate($includeTests = false) + public function regenerate($includeTests = false, array $ignoredCILibs = []) { $this->modules = []; @@ -171,6 +173,7 @@ class ModuleManifest $finder->setOptions([ 'min_depth' => 0, 'ignore_tests' => !$includeTests, + 'ignore_ci_libraries' => $ignoredCILibs, 'dir_callback' => function ($basename, $pathname, $depth) use ($finder) { if ($finder->isDirectoryModule($basename, $pathname, $depth)) { $this->addModule($pathname); diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index bca1c2f82..79601dd64 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -29,6 +29,7 @@ use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Core\Manifest\ClassLoader; +use SilverStripe\Core\Manifest\Module; use SilverStripe\Core\Manifest\ModuleResourceLoader; use SilverStripe\Dev\Constraint\SSListContains; use SilverStripe\Dev\Constraint\SSListContainsOnly; @@ -1010,6 +1011,9 @@ if (class_exists(IsEqualCanonicalizing::class)) { // Test application $kernel = new TestKernel(BASE_PATH); + // PHPUnit 9 only logic to exclude old test still targeting PHPUNit 5.7 + $kernel->setIgnoreCILibraries([Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); + if (class_exists(HTTPApplication::class)) { // Mock request $_SERVER['argv'] = ['vendor/bin/phpunit', '/']; diff --git a/src/Dev/TestKernel.php b/src/Dev/TestKernel.php index 67229f5ca..a1c9802a8 100644 --- a/src/Dev/TestKernel.php +++ b/src/Dev/TestKernel.php @@ -9,6 +9,11 @@ use SilverStripe\Core\CoreKernel; */ class TestKernel extends CoreKernel { + + /** @var string[] $ciLibs */ + private $ciLibs = []; + + public function __construct($basePath) { $this->setEnvironment(self::DEV); @@ -41,6 +46,21 @@ class TestKernel extends CoreKernel return true; } + + /** + * @param string[] $ciLibs + */ + public function setIgnoreCILibraries(array $ciLibs): self + { + $this->ciLibs = $ciLibs; + return $this; + } + + protected function getIgnoreCILibraries(): array + { + return $this->ciLibs; + } + protected function bootErrorHandling() { // Leave phpunit to capture errors diff --git a/src/View/ThemeManifest.php b/src/View/ThemeManifest.php index 4f36bac11..fd1c81b61 100644 --- a/src/View/ThemeManifest.php +++ b/src/View/ThemeManifest.php @@ -73,8 +73,9 @@ class ThemeManifest implements ThemeList /** * @param bool $includeTests Include tests in the manifest * @param bool $forceRegen Force the manifest to be regenerated. + * @param string[] $ignoredCILibs */ - public function init($includeTests = false, $forceRegen = false) + public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) { // build cache from factory if ($this->cacheFactory) { @@ -87,7 +88,7 @@ class ThemeManifest implements ThemeList if (!$forceRegen && $this->cache && ($data = $this->cache->get($this->cacheKey))) { $this->themes = $data; } else { - $this->regenerate($includeTests); + $this->regenerate($includeTests, $ignoredCILibs); } } @@ -129,14 +130,16 @@ class ThemeManifest implements ThemeList * Regenerates the manifest by scanning the base path. * * @param bool $includeTests + * @param string[] $ignoredCILibs */ - public function regenerate($includeTests = false) + public function regenerate($includeTests = false, array $ignoredCILibs = []) { $finder = new ManifestFileFinder(); $finder->setOptions([ 'include_themes' => false, 'ignore_dirs' => ['node_modules', THEMES_DIR], 'ignore_tests' => !$includeTests, + 'ignore_ci_libraries' => $ignoredCILibs, 'dir_callback' => [$this, 'handleDirectory'] ]); diff --git a/tests/php/Core/Manifest/ManifestFileFinderTest.php b/tests/php/Core/Manifest/ManifestFileFinderTest.php index e179eb049..36df22cba 100644 --- a/tests/php/Core/Manifest/ManifestFileFinderTest.php +++ b/tests/php/Core/Manifest/ManifestFileFinderTest.php @@ -91,7 +91,7 @@ class ManifestFileFinderTest extends SapphireTest $finder = new ManifestFileFinder(); $finder->setOption('name_regex', '/\.txt$/'); $finder->setOption('ignore_tests', false); - $finder->setOption('ignore_ci_library', [Module::CI_PHPUNIT_FIVE]); + $finder->setOption('ignore_ci_libraries', [Module::CI_PHPUNIT_FIVE]); $this->assertFinderFinds( $finder, @@ -115,7 +115,7 @@ class ManifestFileFinderTest extends SapphireTest $finder = new ManifestFileFinder(); $finder->setOption('name_regex', '/\.txt$/'); $finder->setOption('ignore_tests', false); - $finder->setOption('ignore_ci_library', [Module::CI_PHPUNIT_FIVE, Module::CI_PHPUNIT_UNKNOWN]); + $finder->setOption('ignore_ci_libraries', [Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); $this->assertFinderFinds( $finder, diff --git a/tests/php/Core/Manifest/ModuleTest.php b/tests/php/Core/Manifest/ModuleTest.php index 83af14cc6..8acf46285 100644 --- a/tests/php/Core/Manifest/ModuleTest.php +++ b/tests/php/Core/Manifest/ModuleTest.php @@ -35,11 +35,11 @@ class ModuleTest extends SapphireTest public function ciLibraryProvider() { return [ - 'empty require-dev' => ['empty-require-dev', Module::CI_PHPUNIT_UNKNOWN], - 'no require-dev' => ['no-require-dev', Module::CI_PHPUNIT_UNKNOWN], - 'older version of phpunit' => ['old-phpunit', Module::CI_PHPUNIT_UNKNOWN], - 'phpunit between 5 and 9' => ['inbetween-phpunit', Module::CI_PHPUNIT_UNKNOWN], - 'phpunit beyond 9' => ['future-phpunit', Module::CI_PHPUNIT_UNKNOWN], + 'empty require-dev' => ['empty-require-dev', Module::CI_UNKNOWN], + 'no require-dev' => ['no-require-dev', Module::CI_UNKNOWN], + 'older version of phpunit' => ['old-phpunit', Module::CI_UNKNOWN], + 'phpunit between 5 and 9' => ['inbetween-phpunit', Module::CI_UNKNOWN], + 'phpunit beyond 9' => ['future-phpunit', Module::CI_UNKNOWN], 'phpunit 5.0' => ['phpunit-five-zero', Module::CI_PHPUNIT_FIVE], 'phpunit 5.7' => ['phpunit-five-seven', Module::CI_PHPUNIT_FIVE],