From 1bc1e85f6893338b4aa91a3a2fd740763c9654a8 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 16 Nov 2021 11:36:34 +1300 Subject: [PATCH 1/6] DOC Fix minor typos in Module --- src/Core/Manifest/Module.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index f06faae1e..4337ddf25 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -9,7 +9,7 @@ use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; /** - * Abstraction of a PHP Package. Can be used to retrieve information about SilverStripe modules, and other packages + * Abstraction of a PHP Package. Can be used to retrieve information about Silverstripe CMS modules, and other packages * managed via composer, by reading their `composer.json` file. */ class Module implements Serializable @@ -64,7 +64,7 @@ class Module implements Serializable * Gets name of this module. Used as unique key and identifier for this module. * * If installed by composer, this will be the full composer name (vendor/name). - * If not insalled by composer this will default to the basedir() + * If not installed by composer this will default to the `basedir()` * * @return string */ @@ -74,7 +74,7 @@ class Module implements Serializable } /** - * Get full composer name. Will be null if no composer.json is available + * Get full composer name. Will be `null` if no composer.json is available * * @return string|null */ @@ -130,7 +130,7 @@ class Module implements Serializable /** * Name of the resource directory where vendor resources should be exposed as defined by the `extra.resources-dir` - * key in the composer file. A blank string will will be returned if the key is undefined. + * key in the composer file. A blank string will be returned if the key is undefined. * * Only applicable when reading the composer file for the main project. * @return string From 2922370d81418f6ae8dae3ca1e677981e454ad91 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 16 Nov 2021 16:52:32 +1300 Subject: [PATCH 2/6] API Add Module::getCILibrary function --- composer.json | 1 + src/Core/Manifest/Module.php | 91 +++++++++++++++++++ tests/php/Core/Manifest/ModuleTest.php | 37 ++++++++ .../empty-require-dev/composer.json | 11 +++ .../future-phpunit/composer.json | 13 +++ .../inbetween-phpunit/composer.json | 13 +++ .../no-require-dev/composer.json | 10 ++ .../old-phpunit/composer.json | 13 +++ .../phpunit-five-seven/composer.json | 13 +++ .../phpunit-five-zero/composer.json | 13 +++ .../phpunit-nine-five/composer.json | 13 +++ .../phpunit-nine-x/composer.json | 13 +++ .../phpunit-nine/composer.json | 13 +++ .../recipe-testing-one-two-x/composer.json | 13 +++ .../recipe-testing-one-x/composer.json | 13 +++ .../recipe-testing-one/composer.json | 13 +++ .../recipe-testing-two-x/composer.json | 13 +++ .../recipe-testing-two/composer.json | 13 +++ .../sminnee-five-seven/composer.json | 13 +++ .../sminnee-five/composer.json | 13 +++ 20 files changed, 345 insertions(+) create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/empty-require-dev/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/future-phpunit/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/inbetween-phpunit/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/no-require-dev/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/old-phpunit/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-zero/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-five/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-x/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-two-x/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-x/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-x/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-seven/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five/composer.json diff --git a/composer.json b/composer.json index 2d77c3929..5619aac8d 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ "require": { "bramus/monolog-colored-line-formatter": "^2", "composer/installers": "^1 || ^2", + "composer/semver": "^1 || ^3", "embed/embed": "^3", "league/csv": "^8 || ^9", "m1/env": "^2.1", diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index 4337ddf25..22bd08e46 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -2,8 +2,10 @@ namespace SilverStripe\Core\Manifest; +use Composer\Semver\Semver; use Exception; use InvalidArgumentException; +use RuntimeException; use Serializable; use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; @@ -19,6 +21,23 @@ class Module implements Serializable */ const TRIM_CHARS = ' /\\'; + /** + * Return value of getCILibrary() when module uses PHPUNit 9 + */ + const CI_PHPUNIT_NINE = 'PHPUnit9'; + + /** + * Return value of getCILibrary() when module uses PHPUNit 5 + */ + const CI_PHPUNIT_FIVE = 'PHPUnit5'; + + /** + * Return value of getCILibrary() when module does not use any CI + */ + const CI_PHPUNIT_UNKNOWN = 'NoPHPUnit'; + + + /** * Full directory path to this module with no trailing slash * @@ -275,6 +294,78 @@ class Module implements Serializable ->getResource($path) ->exists(); } + + /** + * Determine what CI library the module is using. + * @internal + */ + public function getCILibrary(): string + { + if (empty($this->composerData)) { + throw new RuntimeException('No composer data at all'); + } + + // We don't have any dev dependencies + if (empty($this->composerData['require-dev']) || !is_array($this->composerData['require-dev'])) { + return self::CI_PHPUNIT_UNKNOWN; + } + + // We are assuming a typical setup where the CI lib is defined in require-dev rather than require + $requireDev = $this->composerData['require-dev']; + + // Try to pick which CI we are using based on phpunit constraint + $phpUnitConstraint = $this->requireDevConstraint(['sminnee/phpunit', 'phpunit/phpunit']); + if ($phpUnitConstraint) { + if ($this->satisfiesAtLeastOne(['5.7.0', '5.0.0', '5.x-dev', '5.7.x-dev'], $phpUnitConstraint)) { + return self::CI_PHPUNIT_FIVE; + } + if ($this->satisfiesAtLeastOne(['9.0.0', '9.5.0', '9.x-dev', '9.5.x-dev'], $phpUnitConstraint)) { + return self::CI_PHPUNIT_NINE; + } + } + + // Try to pick which CI we are using based on recipe-testing constraint + $recipeTestingConstraint = $this->requireDevConstraint(['silverstripe/recipe-testing']); + if ($recipeTestingConstraint) { + if ($this->satisfiesAtLeastOne(['1.0.0', '1.1.0', '1.2.0', '1.1.x-dev', '1.2.x-dev', '1.x-dev'], $recipeTestingConstraint)) { + return self::CI_PHPUNIT_FIVE; + } + if ($this->satisfiesAtLeastOne(['2.0.0', '2.0.x-dev', '2.x-dev'], $recipeTestingConstraint)) { + return self::CI_PHPUNIT_NINE; + } + } + + return self::CI_PHPUNIT_UNKNOWN; + } + + /** + * Retrieve the constraint for the first module that is found in the require-dev section + * @param string[] $modules + * @return false|string + */ + private function requireDevConstraint(array $modules) + { + if (empty($this->composerData['require-dev']) || !is_array($this->composerData['require-dev'])) { + return false; + } + + $requireDev = $this->composerData['require-dev']; + foreach ($modules as $module) { + if (isset($requireDev[$module])) { + return $requireDev[$module]; + } + } + + return false; + } + + /** + * Determines if the provided constraint allows at least one of the version provided + */ + private function satisfiesAtLeastOne(array $versions, string $constraint): bool + { + return !empty(Semver::satisfiedBy($versions, $constraint)); + } } /** diff --git a/tests/php/Core/Manifest/ModuleTest.php b/tests/php/Core/Manifest/ModuleTest.php index b62a28941..83af14cc6 100644 --- a/tests/php/Core/Manifest/ModuleTest.php +++ b/tests/php/Core/Manifest/ModuleTest.php @@ -21,4 +21,41 @@ class ModuleTest extends SapphireTest $module = new Module($path, $path); $this->assertEquals('customised-resources-dir', $module->getResourcesDir()); } + + /** + * @dataProvider ciLibraryProvider + */ + public function testGetCILibrary($fixture, $expected) + { + $path = __DIR__ . '/fixtures/phpunit-detection/' . $fixture; + $module = new Module($path, $path); + $this->assertEquals($expected, $module->getCILibrary()); + } + + 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], + + 'phpunit 5.0' => ['phpunit-five-zero', Module::CI_PHPUNIT_FIVE], + 'phpunit 5.7' => ['phpunit-five-seven', Module::CI_PHPUNIT_FIVE], + 'sminnee 5.7' => ['sminnee-five-seven', Module::CI_PHPUNIT_FIVE], + 'sminnee 5' => ['sminnee-five-seven', Module::CI_PHPUNIT_FIVE], + + 'phpunit 9' => ['phpunit-nine', Module::CI_PHPUNIT_NINE], + 'phpunit 9.5' => ['phpunit-nine-five', Module::CI_PHPUNIT_NINE], + 'future phpunit 9' => ['phpunit-nine-x', Module::CI_PHPUNIT_NINE], + + 'recipe-testing 1' => ['recipe-testing-one', Module::CI_PHPUNIT_FIVE], + 'recipe-testing 1.x' => ['recipe-testing-one-x', Module::CI_PHPUNIT_FIVE], + 'recipe-testing 1.2.x' => ['recipe-testing-one-two-x', Module::CI_PHPUNIT_FIVE], + + 'recipe-testing 2' => ['recipe-testing-two', Module::CI_PHPUNIT_NINE], + 'recipe-testing 2.x' => ['recipe-testing-two-x', Module::CI_PHPUNIT_NINE], + ]; + } } diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/empty-require-dev/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/empty-require-dev/composer.json new file mode 100644 index 000000000..6d1ee8cbb --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/empty-require-dev/composer.json @@ -0,0 +1,11 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module does not define a CI library in it's dev dependency", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": {} +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/future-phpunit/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/future-phpunit/composer.json new file mode 100644 index 000000000..276608198 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/future-phpunit/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a future version of phpunit", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^10.0.0" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/inbetween-phpunit/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/inbetween-phpunit/composer.json new file mode 100644 index 000000000..925cea0b0 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/inbetween-phpunit/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a version of phpunit in between 5 and 9", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^6" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/no-require-dev/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/no-require-dev/composer.json new file mode 100644 index 000000000..7ff1d854e --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/no-require-dev/composer.json @@ -0,0 +1,10 @@ +{ + "name": "silverstripe/no-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module does not define any dev dependency", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/old-phpunit/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/old-phpunit/composer.json new file mode 100644 index 000000000..f4fe84a50 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/old-phpunit/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/old-phpunit", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a version of phpunit prior to 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^4" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json new file mode 100644 index 000000000..36ef4a8fa --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of sminnee/PHPUnit 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "sminnee/phpunit": "^5" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-zero/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-zero/composer.json new file mode 100644 index 000000000..c3015d1aa --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-zero/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of PHPUnit 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.0" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-five/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-five/composer.json new file mode 100644 index 000000000..eca645ddd --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-five/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses version of phpunit 9.5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^9.5" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-x/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-x/composer.json new file mode 100644 index 000000000..117485132 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-x/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a future version of phpunit 9", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^9.999999" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine/composer.json new file mode 100644 index 000000000..7b4ddcbe8 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of phpunit 9", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^9" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-two-x/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-two-x/composer.json new file mode 100644 index 000000000..fb74e6524 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-two-x/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses unstable version of recipe testing 1.2", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "1.2.x-dev" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-x/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-x/composer.json new file mode 100644 index 000000000..69a489f53 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-x/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses unstable version of recipe testing 1", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "1.x-dev" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one/composer.json new file mode 100644 index 000000000..44a07e8dc --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of recipe testing 1", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "^1" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-x/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-x/composer.json new file mode 100644 index 000000000..074213b06 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-x/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses unreleased version recipe testing 2", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "2.x-dev" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two/composer.json new file mode 100644 index 000000000..0cec3ae86 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of recipe testing 2", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "^2.0.0" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-seven/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-seven/composer.json new file mode 100644 index 000000000..2bb3f2e30 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-seven/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of PHPUnit 5.7", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.7" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five/composer.json new file mode 100644 index 000000000..e656472a1 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses any version of sminnee/PHPUnit 5.7", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "sminnee/phpunit": "^5" + } +} From 640a7e3eeaaf6b51869f7e15d3aff1da2ccafd6d Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 16 Nov 2021 18:44:48 +1300 Subject: [PATCH 3/6] ENH Improve ManifestFileFinder so it can ignore test based on the testing library --- src/Core/Manifest/ManifestFileFinder.php | 29 +++++++++- src/Core/Manifest/Module.php | 4 +- .../Core/Manifest/ManifestFileFinderTest.php | 55 +++++++++++++++++++ .../myvendor/phpunit5module/_config.php | 1 + .../myvendor/phpunit5module/code/logic.txt | 1 + .../myvendor/phpunit5module/composer.json | 6 ++ .../phpunit5module/tests/phpunit5tests.txt | 1 + .../myvendor/phpunit9module/_config.php | 1 + .../myvendor/phpunit9module/code/logic.txt | 1 + .../myvendor/phpunit9module/composer.json | 6 ++ .../phpunit9module/tests/phpunit9tests.txt | 1 + 11 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/_config.php create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/code/logic.txt create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/tests/phpunit5tests.txt create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit9module/_config.php create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit9module/code/logic.txt create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit9module/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit9module/tests/phpunit9tests.txt diff --git a/src/Core/Manifest/ManifestFileFinder.php b/src/Core/Manifest/ManifestFileFinder.php index c4c248e23..2e737bd01 100644 --- a/src/Core/Manifest/ManifestFileFinder.php +++ b/src/Core/Manifest/ManifestFileFinder.php @@ -32,7 +32,8 @@ class ManifestFileFinder extends FileFinder 'include_themes' => false, 'ignore_tests' => true, 'min_depth' => 1, - 'ignore_dirs' => ['node_modules'] + 'ignore_dirs' => ['node_modules'], + 'ignore_ci_library' => [] ]; public function acceptDir($basename, $pathname, $depth) @@ -73,6 +74,14 @@ class ManifestFileFinder extends FileFinder return false; } + // Skip if test dir inside vendor module with unexpected CI library + if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCILib = $this->getOption('ignore_ci_library')) { + $ciLib = $this->findModuleCILib($basename, $pathname, $depth); + if (in_array($ciLib, $ignoreCILib)) { + return false; + } + } + return parent::acceptDir($basename, $pathname, $depth); } @@ -251,4 +260,22 @@ class ManifestFileFinder extends FileFinder return false; } + + private function findModuleCILib(string $basename, string $pathname, int $depth): string + { + if ($depth < 1) { + return Module::CI_PHPUNIT_UNKNOWN; + } + + $newBasename = basename($pathname); + $newPathname = dirname($pathname); + $newDepth = $depth - 1; + + if ($this->isDirectoryModule($newBasename, $newPathname, $newDepth)) { + $module = new Module($newPathname, $this->upLevels($newPathname, $newDepth)); + return $module->getCILibrary(); + } + + return $this->findModuleCILib($newBasename, $newPathname, $newDepth); + } } diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index 22bd08e46..71a29cde3 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -5,7 +5,6 @@ namespace SilverStripe\Core\Manifest; use Composer\Semver\Semver; use Exception; use InvalidArgumentException; -use RuntimeException; use Serializable; use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; @@ -301,8 +300,9 @@ class Module implements Serializable */ public function getCILibrary(): string { + // We don't have any composer data at all if (empty($this->composerData)) { - throw new RuntimeException('No composer data at all'); + return self::CI_PHPUNIT_UNKNOWN; } // We don't have any dev dependencies diff --git a/tests/php/Core/Manifest/ManifestFileFinderTest.php b/tests/php/Core/Manifest/ManifestFileFinderTest.php index 7c0909b10..e179eb049 100644 --- a/tests/php/Core/Manifest/ManifestFileFinderTest.php +++ b/tests/php/Core/Manifest/ManifestFileFinderTest.php @@ -4,6 +4,7 @@ namespace SilverStripe\Core\Tests\Manifest; use SilverStripe\Core\Manifest\ManifestFileFinder; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Core\Manifest\Module; /** * Tests for the {@link ManifestFileFinder} class. @@ -55,6 +56,8 @@ class ManifestFileFinderTest extends SapphireTest [ 'module/module.txt', 'vendor/myvendor/thismodule/module.txt', + 'vendor/myvendor/phpunit5module/code/logic.txt', + 'vendor/myvendor/phpunit9module/code/logic.txt', ] ); } @@ -75,6 +78,56 @@ class ManifestFileFinderTest extends SapphireTest 'vendor/myvendor/thismodule/module.txt', 'vendor/myvendor/thismodule/tests/tests.txt', 'vendor/myvendor/thismodule/code/tests/tests2.txt', + 'vendor/myvendor/phpunit5module/code/logic.txt', + 'vendor/myvendor/phpunit5module/tests/phpunit5tests.txt', + 'vendor/myvendor/phpunit9module/code/logic.txt', + 'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt', + ] + ); + } + + public function testIgnorePHPUnit5Tests() + { + $finder = new ManifestFileFinder(); + $finder->setOption('name_regex', '/\.txt$/'); + $finder->setOption('ignore_tests', false); + $finder->setOption('ignore_ci_library', [Module::CI_PHPUNIT_FIVE]); + + $this->assertFinderFinds( + $finder, + null, + [ + 'module/module.txt', + 'module/tests/tests.txt', + 'module/code/tests/tests2.txt', + 'vendor/myvendor/thismodule/module.txt', + 'vendor/myvendor/thismodule/tests/tests.txt', + 'vendor/myvendor/thismodule/code/tests/tests2.txt', + 'vendor/myvendor/phpunit5module/code/logic.txt', + 'vendor/myvendor/phpunit9module/code/logic.txt', + 'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt', + ] + ); + } + + public function testIgnoreNonePHPUnit9Tests() + { + $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]); + + $this->assertFinderFinds( + $finder, + null, + [ + 'module/module.txt', + 'module/tests/tests.txt', + 'module/code/tests/tests2.txt', + 'vendor/myvendor/thismodule/module.txt', + 'vendor/myvendor/phpunit5module/code/logic.txt', + 'vendor/myvendor/phpunit9module/code/logic.txt', + 'vendor/myvendor/phpunit9module/tests/phpunit9tests.txt', ] ); } @@ -92,6 +145,8 @@ class ManifestFileFinderTest extends SapphireTest 'module/module.txt', 'themes/themes.txt', 'vendor/myvendor/thismodule/module.txt', + 'vendor/myvendor/phpunit5module/code/logic.txt', + 'vendor/myvendor/phpunit9module/code/logic.txt', ] ); } diff --git a/tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/_config.php b/tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/_config.php new file mode 100644 index 000000000..b3d9bbc7f --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/manifestfilefinder/vendor/myvendor/phpunit5module/_config.php @@ -0,0 +1 @@ + Date: Wed, 17 Nov 2021 12:37:16 +1300 Subject: [PATCH 4/6] 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], From 7c3fddfc8a52fbad4a28256d80b8ba9ff6b83007 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Thu, 18 Nov 2021 23:16:03 +1300 Subject: [PATCH 5/6] Anwser Peer review feedback --- src/Core/CoreKernel.php | 8 +-- src/Core/Manifest/ClassLoader.php | 6 +- src/Core/Manifest/ClassManifest.php | 12 ++-- src/Core/Manifest/ManifestFileFinder.php | 35 ++++++--- src/Core/Manifest/Module.php | 72 +++++++++++++++---- src/Core/Manifest/ModuleLoader.php | 6 +- src/Core/Manifest/ModuleManifest.php | 12 ++-- src/Dev/SapphireTest.php | 2 +- src/Dev/TestKernel.php | 11 +-- src/View/ThemeManifest.php | 12 ++-- .../Core/Manifest/ManifestFileFinderTest.php | 4 +- tests/php/Core/Manifest/ModuleTest.php | 20 ++++-- .../phpunit-five-exact-version/composer.json | 13 ++++ .../phpunit-five-seven/composer.json | 4 +- .../phpunit-five-tilde/composer.json | 13 ++++ .../phpunit-nine-exact/composer.json | 13 ++++ .../recipe-testing-one-flag/composer.json | 13 ++++ .../recipe-testing-two-exact/composer.json | 13 ++++ .../sminnee-five-star/composer.json | 13 ++++ 19 files changed, 217 insertions(+), 65 deletions(-) create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-exact-version/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-tilde/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-exact/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-flag/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-exact/composer.json create mode 100644 tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-star/composer.json diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index 27af9a7a1..fb958f8d2 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -516,7 +516,7 @@ class CoreKernel implements Kernel * * @return string[] List of CI types to ignore as defined by `Module`. */ - protected function getIgnoreCILibraries(): array + protected function getIgnoreCIConfigs(): array { return []; } @@ -532,14 +532,14 @@ class CoreKernel implements Kernel $this->getClassLoader()->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCILibraries() + $this->getIgnoreCIConfigs() ); // Find modules $this->getModuleLoader()->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCILibraries() + $this->getIgnoreCIConfigs() ); // Flush config @@ -561,7 +561,7 @@ class CoreKernel implements Kernel $defaultSet->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCILibraries() + $this->getIgnoreCIConfigs() ); } } diff --git a/src/Core/Manifest/ClassLoader.php b/src/Core/Manifest/ClassLoader.php index f25800909..f36c2520d 100644 --- a/src/Core/Manifest/ClassLoader.php +++ b/src/Core/Manifest/ClassLoader.php @@ -120,14 +120,14 @@ class ClassLoader * * @param bool $includeTests * @param bool $forceRegen - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) + public function init($includeTests = false, $forceRegen = false, array $ignoredCIConfigs = []) { foreach ($this->manifests as $manifest) { /** @var ClassManifest $instance */ $instance = $manifest['instance']; - $instance->init($includeTests, $forceRegen, $ignoredCILibs); + $instance->init($includeTests, $forceRegen, $ignoredCIConfigs); } $this->registerAutoloader(); diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 99c02a1ff..8167a6826 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -260,9 +260,9 @@ class ClassManifest * * @param bool $includeTests * @param bool $forceRegen - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) + public function init($includeTests = false, $forceRegen = false, array $ignoredCIConfigs = []) { $this->cache = $this->buildCache($includeTests); @@ -276,7 +276,7 @@ class ClassManifest } // Build - $this->regenerate($includeTests, $ignoredCILibs); + $this->regenerate($includeTests, $ignoredCIConfigs); } /** @@ -501,9 +501,9 @@ class ClassManifest * Completely regenerates the manifest file. * * @param bool $includeTests - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function regenerate($includeTests, array $ignoredCILibs = []) + public function regenerate($includeTests, array $ignoredCIConfigs = []) { // Reset the manifest so stale info doesn't cause errors. $this->loadState([]); @@ -515,7 +515,7 @@ class ClassManifest 'name_regex' => '/^[^_].*\\.php$/', 'ignore_files' => ['index.php', 'cli-script.php'], 'ignore_tests' => !$includeTests, - 'ignore_ci_libraries' => $ignoredCILibs, + 'ignore_ci_configs' => $ignoredCIConfigs, '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 bf04a7002..1007ba23f 100644 --- a/src/Core/Manifest/ManifestFileFinder.php +++ b/src/Core/Manifest/ManifestFileFinder.php @@ -2,6 +2,7 @@ namespace SilverStripe\Core\Manifest; +use RuntimeException; use SilverStripe\Assets\FileFinder; /** @@ -33,7 +34,7 @@ class ManifestFileFinder extends FileFinder 'ignore_tests' => true, 'min_depth' => 1, 'ignore_dirs' => ['node_modules'], - 'ignore_ci_libraries' => [] + 'ignore_ci_configs' => [] ]; public function acceptDir($basename, $pathname, $depth) @@ -74,10 +75,10 @@ class ManifestFileFinder extends FileFinder return false; } - // Skip if test dir inside vendor module with unexpected 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)) { + // Skip if test dir inside vendor module with unexpected CI Configuration + if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCIConfig = $this->getOption('ignore_ci_configs')) { + $ciLib = $this->findModuleCIPhpConfiguration($basename, $pathname, $depth); + if (in_array($ciLib, $ignoreCIConfig)) { return false; } } @@ -261,21 +262,39 @@ class ManifestFileFinder extends FileFinder return false; } - private function findModuleCILib(string $basename, string $pathname, int $depth): string + /** + * Find out the root of the current module and read the PHP CI configuration from tho composer file + * + * @param string $basename Name of the current folder + * @param string $pathname Full path the parent folder + * @param string $depth Depth of the current folder + */ + private function findModuleCIPhpConfiguration(string $basename, string $pathname, int $depth): string { if ($depth < 1) { + // We went all the way back to the root of the project return Module::CI_UNKNOWN; } + // We pop the current folder and use the next entry the pathname $newBasename = basename($pathname); $newPathname = dirname($pathname); $newDepth = $depth - 1; if ($this->isDirectoryModule($newBasename, $newPathname, $newDepth)) { + // We've reached the root of the module folder, we can read the PHP CI config now $module = new Module($newPathname, $this->upLevels($newPathname, $newDepth)); - return $module->getCILibrary(); + $config = $module->getCIConfig(); + + if (empty($config['PHP'])) { + // This should never happen + throw new RuntimeException('Module::getCIConfig() did not return a PHP CI value'); + } + + return $config['PHP']; } - return $this->findModuleCILib($newBasename, $newPathname, $newDepth); + // We haven't reach our module root yet ... let's look up one more level + return $this->findModuleCIPhpConfiguration($newBasename, $newPathname, $newDepth); } } diff --git a/src/Core/Manifest/Module.php b/src/Core/Manifest/Module.php index fa90f60ac..23a974fb3 100644 --- a/src/Core/Manifest/Module.php +++ b/src/Core/Manifest/Module.php @@ -5,6 +5,7 @@ namespace SilverStripe\Core\Manifest; use Composer\Semver\Semver; use Exception; use InvalidArgumentException; +use RuntimeException; use Serializable; use SilverStripe\Core\Path; use SilverStripe\Dev\Deprecation; @@ -21,19 +22,19 @@ class Module implements Serializable const TRIM_CHARS = ' /\\'; /** - * Return value of getCILibrary() when module uses PHPUNit 9 + * Return value of getCIConfig() when module uses PHPUNit 9 */ - const CI_PHPUNIT_NINE = 'PHPUnit9'; + const CI_PHPUNIT_NINE = 'CI_PHPUNIT_NINE'; /** - * Return value of getCILibrary() when module uses PHPUNit 5 + * Return value of getCIConfig() when module uses PHPUNit 5 */ - const CI_PHPUNIT_FIVE = 'PHPUnit5'; + const CI_PHPUNIT_FIVE = 'CI_PHPUNIT_FIVE'; /** - * Return value of getCILibrary() when module does not use any CI + * Return value of getCIConfig() when module does not use any CI */ - const CI_UNKNOWN = 'NoPHPUnit'; + const CI_UNKNOWN = 'CI_UNKNOWN'; @@ -295,10 +296,22 @@ class Module implements Serializable } /** - * Determine what CI library the module is using. + * Determine what configurations the module is using to run various aspects of its CI. THe only aspect + * that is observed is `PHP` + * @return array List of configuration aspects e.g.: `['PHP' => 'CI_PHPUNIT_NINE']` * @internal */ - public function getCILibrary(): string + public function getCIConfig(): array + { + return [ + 'PHP' => $this->getPhpCiConfig() + ]; + } + + /** + * Determine what CI Configuration the module uses to test its PHP code. + */ + private function getPhpCiConfig(): string { // We don't have any composer data at all if (empty($this->composerData)) { @@ -316,10 +329,18 @@ class Module implements Serializable // Try to pick which CI we are using based on phpunit constraint $phpUnitConstraint = $this->requireDevConstraint(['sminnee/phpunit', 'phpunit/phpunit']); if ($phpUnitConstraint) { - if ($this->satisfiesAtLeastOne(['5.7.0', '5.0.0', '5.x-dev', '5.7.x-dev'], $phpUnitConstraint)) { + if ($this->constraintSatisfies( + $phpUnitConstraint, + ['5.7.0', '5.0.0', '5.x-dev', '5.7.x-dev'], + 5 + )) { return self::CI_PHPUNIT_FIVE; } - if ($this->satisfiesAtLeastOne(['9.0.0', '9.5.0', '9.x-dev', '9.5.x-dev'], $phpUnitConstraint)) { + if ($this->constraintSatisfies( + $phpUnitConstraint, + ['9.0.0', '9.5.0', '9.x-dev', '9.5.x-dev'], + 9 + )) { return self::CI_PHPUNIT_NINE; } } @@ -327,10 +348,18 @@ class Module implements Serializable // Try to pick which CI we are using based on recipe-testing constraint $recipeTestingConstraint = $this->requireDevConstraint(['silverstripe/recipe-testing']); if ($recipeTestingConstraint) { - if ($this->satisfiesAtLeastOne(['1.0.0', '1.1.0', '1.2.0', '1.1.x-dev', '1.2.x-dev', '1.x-dev'], $recipeTestingConstraint)) { + if ($this->constraintSatisfies( + $recipeTestingConstraint, + ['1.0.0', '1.1.0', '1.2.0', '1.1.x-dev', '1.2.x-dev', '1.x-dev'], + 1 + )) { return self::CI_PHPUNIT_FIVE; } - if ($this->satisfiesAtLeastOne(['2.0.0', '2.0.x-dev', '2.x-dev'], $recipeTestingConstraint)) { + if ($this->constraintSatisfies( + $recipeTestingConstraint, + ['2.0.0', '2.0.x-dev', '2.x-dev'], + 2 + )) { return self::CI_PHPUNIT_NINE; } } @@ -362,9 +391,22 @@ class Module implements Serializable /** * Determines if the provided constraint allows at least one of the version provided */ - private function satisfiesAtLeastOne(array $versions, string $constraint): bool - { - return !empty(Semver::satisfiedBy($versions, $constraint)); + private function constraintSatisfies( + string $constraint, + array $possibleVersions, + int $majorVersionFallback + ): bool { + // Let's see of any of our possible versions is allowed by the constraint + if (!empty(Semver::satisfiedBy($possibleVersions, $constraint))) { + return true; + } + + // Let's see if we are using an exact version constraint. e.g. ~1.2.3 or 1.2.3 or ~1.2 or 1.2.* + if (preg_match("/^~?$majorVersionFallback(\.(\d+)|\*){0,2}/", $constraint)) { + return true; + } + + return false; } } diff --git a/src/Core/Manifest/ModuleLoader.php b/src/Core/Manifest/ModuleLoader.php index 0f5cf1ffd..a27fccb51 100644 --- a/src/Core/Manifest/ModuleLoader.php +++ b/src/Core/Manifest/ModuleLoader.php @@ -91,12 +91,12 @@ class ModuleLoader * * @param bool $includeTests * @param bool $forceRegen - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) + public function init($includeTests = false, $forceRegen = false, array $ignoredCIConfigs = []) { foreach ($this->manifests as $manifest) { - $manifest->init($includeTests, $forceRegen, $ignoredCILibs); + $manifest->init($includeTests, $forceRegen, $ignoredCIConfigs); } } } diff --git a/src/Core/Manifest/ModuleManifest.php b/src/Core/Manifest/ModuleManifest.php index d5177070e..75be570a9 100644 --- a/src/Core/Manifest/ModuleManifest.php +++ b/src/Core/Manifest/ModuleManifest.php @@ -121,9 +121,9 @@ class ModuleManifest /** * @param bool $includeTests * @param bool $forceRegen Force the manifest to be regenerated. - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) + public function init($includeTests = false, $forceRegen = false, array $ignoredCIConfigs = []) { // build cache from factory if ($this->cacheFactory) { @@ -138,7 +138,7 @@ class ModuleManifest $this->modules = $this->cache->get($this->cacheKey) ?: []; } if (empty($this->modules)) { - $this->regenerate($includeTests, $ignoredCILibs); + $this->regenerate($includeTests, $ignoredCIConfigs); } } @@ -163,9 +163,9 @@ class ModuleManifest * Does _not_ build the actual variant * * @param bool $includeTests - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function regenerate($includeTests = false, array $ignoredCILibs = []) + public function regenerate($includeTests = false, array $ignoredCIConfigs = []) { $this->modules = []; @@ -173,7 +173,7 @@ class ModuleManifest $finder->setOptions([ 'min_depth' => 0, 'ignore_tests' => !$includeTests, - 'ignore_ci_libraries' => $ignoredCILibs, + 'ignore_ci_configs' => $ignoredCIConfigs, '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 79601dd64..be9a0e221 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -1012,7 +1012,7 @@ if (class_exists(IsEqualCanonicalizing::class)) { $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]); + $kernel->setIgnoreCIConfigs([Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); if (class_exists(HTTPApplication::class)) { // Mock request diff --git a/src/Dev/TestKernel.php b/src/Dev/TestKernel.php index a1c9802a8..1e7708680 100644 --- a/src/Dev/TestKernel.php +++ b/src/Dev/TestKernel.php @@ -10,8 +10,8 @@ use SilverStripe\Core\CoreKernel; class TestKernel extends CoreKernel { - /** @var string[] $ciLibs */ - private $ciLibs = []; + /** @var string[] $ciConfigs */ + private $ciConfigs = []; public function __construct($basePath) @@ -48,15 +48,16 @@ class TestKernel extends CoreKernel /** - * @param string[] $ciLibs + * Set a list of CI configurations that should cause a module's test not to be added to a manifest + * @param string[] $ciConfigs */ - public function setIgnoreCILibraries(array $ciLibs): self + public function setIgnoreCIConfigs(array $ciLibs): self { $this->ciLibs = $ciLibs; return $this; } - protected function getIgnoreCILibraries(): array + protected function getIgnoreCIConfigs(): array { return $this->ciLibs; } diff --git a/src/View/ThemeManifest.php b/src/View/ThemeManifest.php index fd1c81b61..9c2818985 100644 --- a/src/View/ThemeManifest.php +++ b/src/View/ThemeManifest.php @@ -73,9 +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 + * @param string[] $ignoredCIConfigs */ - public function init($includeTests = false, $forceRegen = false, array $ignoredCILibs = []) + public function init($includeTests = false, $forceRegen = false, array $ignoredCIConfigs = []) { // build cache from factory if ($this->cacheFactory) { @@ -88,7 +88,7 @@ class ThemeManifest implements ThemeList if (!$forceRegen && $this->cache && ($data = $this->cache->get($this->cacheKey))) { $this->themes = $data; } else { - $this->regenerate($includeTests, $ignoredCILibs); + $this->regenerate($includeTests, $ignoredCIConfigs); } } @@ -130,16 +130,16 @@ class ThemeManifest implements ThemeList * Regenerates the manifest by scanning the base path. * * @param bool $includeTests - * @param string[] $ignoredCILibs + * @param string[] $ignoredCIConfigs */ - public function regenerate($includeTests = false, array $ignoredCILibs = []) + public function regenerate($includeTests = false, array $ignoredCIConfigs = []) { $finder = new ManifestFileFinder(); $finder->setOptions([ 'include_themes' => false, 'ignore_dirs' => ['node_modules', THEMES_DIR], 'ignore_tests' => !$includeTests, - 'ignore_ci_libraries' => $ignoredCILibs, + 'ignore_ci_configs' => $ignoredCIConfigs, 'dir_callback' => [$this, 'handleDirectory'] ]); diff --git a/tests/php/Core/Manifest/ManifestFileFinderTest.php b/tests/php/Core/Manifest/ManifestFileFinderTest.php index 36df22cba..4f90ed3aa 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_libraries', [Module::CI_PHPUNIT_FIVE]); + $finder->setOption('ignore_ci_configs', [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_libraries', [Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); + $finder->setOption('ignore_ci_configs', [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 8acf46285..f4ea2db5a 100644 --- a/tests/php/Core/Manifest/ModuleTest.php +++ b/tests/php/Core/Manifest/ModuleTest.php @@ -23,16 +23,22 @@ class ModuleTest extends SapphireTest } /** - * @dataProvider ciLibraryProvider + * @dataProvider ciConfigProvider + * @param string $fixture The folder containing our test composer file + * @param string $expectedPhpConfig */ - public function testGetCILibrary($fixture, $expected) + public function testGetCIConfig($fixture, $expectedPhpConfig) { $path = __DIR__ . '/fixtures/phpunit-detection/' . $fixture; $module = new Module($path, $path); - $this->assertEquals($expected, $module->getCILibrary()); + $this->assertEquals( + $expectedPhpConfig, + $module->getCIConfig()['PHP'], + 'PHP config is set to ' . $expectedPhpConfig + ); } - public function ciLibraryProvider() + public function ciConfigProvider() { return [ 'empty require-dev' => ['empty-require-dev', Module::CI_UNKNOWN], @@ -43,19 +49,25 @@ class ModuleTest extends SapphireTest 'phpunit 5.0' => ['phpunit-five-zero', Module::CI_PHPUNIT_FIVE], 'phpunit 5.7' => ['phpunit-five-seven', Module::CI_PHPUNIT_FIVE], + 'phpunit 5 exact version' => ['phpunit-five-exact-version', Module::CI_PHPUNIT_FIVE], + 'phpunit 5 tilde' => ['phpunit-five-tilde', Module::CI_PHPUNIT_FIVE], 'sminnee 5.7' => ['sminnee-five-seven', Module::CI_PHPUNIT_FIVE], 'sminnee 5' => ['sminnee-five-seven', Module::CI_PHPUNIT_FIVE], + 'sminnee 5 star' => ['sminnee-five-star', Module::CI_PHPUNIT_FIVE], 'phpunit 9' => ['phpunit-nine', Module::CI_PHPUNIT_NINE], 'phpunit 9.5' => ['phpunit-nine-five', Module::CI_PHPUNIT_NINE], 'future phpunit 9' => ['phpunit-nine-x', Module::CI_PHPUNIT_NINE], + 'phpunit 9 exact version' => ['phpunit-nine-exact', Module::CI_PHPUNIT_NINE], 'recipe-testing 1' => ['recipe-testing-one', Module::CI_PHPUNIT_FIVE], 'recipe-testing 1.x' => ['recipe-testing-one-x', Module::CI_PHPUNIT_FIVE], 'recipe-testing 1.2.x' => ['recipe-testing-one-two-x', Module::CI_PHPUNIT_FIVE], + 'recipe-testing 1 with stability flag' => ['recipe-testing-one-flag', Module::CI_PHPUNIT_FIVE], 'recipe-testing 2' => ['recipe-testing-two', Module::CI_PHPUNIT_NINE], 'recipe-testing 2.x' => ['recipe-testing-two-x', Module::CI_PHPUNIT_NINE], + 'recipe-testing 2 exact' => ['recipe-testing-two-x', Module::CI_PHPUNIT_NINE], ]; } } diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-exact-version/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-exact-version/composer.json new file mode 100644 index 000000000..421a65976 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-exact-version/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses an exact version of phpunit 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "5.7.1234" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json index 36ef4a8fa..072d568ff 100644 --- a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-seven/composer.json @@ -1,13 +1,13 @@ { "name": "silverstripe/empty-require-dev", "type": "silverstripe-vendor-module", - "description": "This fake module uses any version of sminnee/PHPUnit 5", + "description": "This fake module uses any version of PHPUnit 5", "homepage": "https://www.silverstripe.org", "license": "BSD-3-Clause", "require": { "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" }, "require-dev": { - "sminnee/phpunit": "^5" + "phpunit/phpunit": "^5" } } diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-tilde/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-tilde/composer.json new file mode 100644 index 000000000..981ee4b71 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-five-tilde/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a tilde constraint of phpunit 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "~5.9.1234" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-exact/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-exact/composer.json new file mode 100644 index 000000000..b784f436c --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/phpunit-nine-exact/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses an exact version of phpunit 9", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "phpunit/phpunit": "9.9.9" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-flag/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-flag/composer.json new file mode 100644 index 000000000..c05013696 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-one-flag/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a version of recipe testing 1 with a stability flag", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "~1.8.0@stable" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-exact/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-exact/composer.json new file mode 100644 index 000000000..65e23cbe8 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/recipe-testing-two-exact/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses an exact version of recipe testing 2", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "silverstripe/recipe-testing": "2.34.56" + } +} diff --git a/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-star/composer.json b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-star/composer.json new file mode 100644 index 000000000..e022b67a1 --- /dev/null +++ b/tests/php/Core/Manifest/fixtures/phpunit-detection/sminnee-five-star/composer.json @@ -0,0 +1,13 @@ +{ + "name": "silverstripe/empty-require-dev", + "type": "silverstripe-vendor-module", + "description": "This fake module uses a star constraint of sminnee 5", + "homepage": "https://www.silverstripe.org", + "license": "BSD-3-Clause", + "require": { + "silverstripe/recipe-cms": "4.4.x-dev as 4.4.0" + }, + "require-dev": { + "sminnee/phpunit": "5.9.*" + } +} From e0197191b8879c7393805fdddca4c7e60591f5ad Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 22 Nov 2021 11:02:27 +1300 Subject: [PATCH 6/6] Rename "Ignore CI Configs" to "Ignored CI Config" --- src/Core/CoreKernel.php | 8 ++++---- src/Core/Manifest/ClassManifest.php | 2 +- src/Core/Manifest/ManifestFileFinder.php | 13 +++++++------ src/Core/Manifest/ModuleManifest.php | 2 +- src/Dev/SapphireTest.php | 2 +- src/Dev/TestKernel.php | 8 ++++---- src/View/ThemeManifest.php | 2 +- tests/php/Core/Manifest/ManifestFileFinderTest.php | 4 ++-- 8 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/Core/CoreKernel.php b/src/Core/CoreKernel.php index fb958f8d2..d28a98692 100644 --- a/src/Core/CoreKernel.php +++ b/src/Core/CoreKernel.php @@ -516,7 +516,7 @@ class CoreKernel implements Kernel * * @return string[] List of CI types to ignore as defined by `Module`. */ - protected function getIgnoreCIConfigs(): array + protected function getIgnoredCIConfigs(): array { return []; } @@ -532,14 +532,14 @@ class CoreKernel implements Kernel $this->getClassLoader()->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCIConfigs() + $this->getIgnoredCIConfigs() ); // Find modules $this->getModuleLoader()->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCIConfigs() + $this->getIgnoredCIConfigs() ); // Flush config @@ -561,7 +561,7 @@ class CoreKernel implements Kernel $defaultSet->init( $this->getIncludeTests(), $flush, - $this->getIgnoreCIConfigs() + $this->getIgnoredCIConfigs() ); } } diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 8167a6826..a09fd06d8 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -515,7 +515,7 @@ class ClassManifest 'name_regex' => '/^[^_].*\\.php$/', 'ignore_files' => ['index.php', 'cli-script.php'], 'ignore_tests' => !$includeTests, - 'ignore_ci_configs' => $ignoredCIConfigs, + 'ignored_ci_configs' => $ignoredCIConfigs, '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 1007ba23f..46fa5ea09 100644 --- a/src/Core/Manifest/ManifestFileFinder.php +++ b/src/Core/Manifest/ManifestFileFinder.php @@ -6,13 +6,14 @@ use RuntimeException; use SilverStripe\Assets\FileFinder; /** - * An extension to the default file finder with some extra filters to faciliate + * An extension to the default file finder with some extra filters to facilitate * autoload and template manifest generation: * - Only modules with _config.php files are scanned. * - If a _manifest_exclude file is present inside a directory it is ignored. * - Assets and module language directories are ignored. - * - Module tests directories are skipped if the ignore_tests option is not - * set to false. + * - Module tests directories are skipped if either of these conditions is meant: + * - the `ignore_tests` option is not set to false. + * - the module PHP CI configuration matches one of the `ignored_ci_configs` */ class ManifestFileFinder extends FileFinder { @@ -34,7 +35,7 @@ class ManifestFileFinder extends FileFinder 'ignore_tests' => true, 'min_depth' => 1, 'ignore_dirs' => ['node_modules'], - 'ignore_ci_configs' => [] + 'ignored_ci_configs' => [] ]; public function acceptDir($basename, $pathname, $depth) @@ -76,9 +77,9 @@ class ManifestFileFinder extends FileFinder } // Skip if test dir inside vendor module with unexpected CI Configuration - if ($depth > 3 && $basename === self::TESTS_DIR && $ignoreCIConfig = $this->getOption('ignore_ci_configs')) { + if ($depth > 3 && $basename === self::TESTS_DIR && $ignoredCIConfig = $this->getOption('ignored_ci_configs')) { $ciLib = $this->findModuleCIPhpConfiguration($basename, $pathname, $depth); - if (in_array($ciLib, $ignoreCIConfig)) { + if (in_array($ciLib, $ignoredCIConfig)) { return false; } } diff --git a/src/Core/Manifest/ModuleManifest.php b/src/Core/Manifest/ModuleManifest.php index 75be570a9..3aca00e82 100644 --- a/src/Core/Manifest/ModuleManifest.php +++ b/src/Core/Manifest/ModuleManifest.php @@ -173,7 +173,7 @@ class ModuleManifest $finder->setOptions([ 'min_depth' => 0, 'ignore_tests' => !$includeTests, - 'ignore_ci_configs' => $ignoredCIConfigs, + 'ignored_ci_configs' => $ignoredCIConfigs, '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 be9a0e221..ffcf459a0 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -1012,7 +1012,7 @@ if (class_exists(IsEqualCanonicalizing::class)) { $kernel = new TestKernel(BASE_PATH); // PHPUnit 9 only logic to exclude old test still targeting PHPUNit 5.7 - $kernel->setIgnoreCIConfigs([Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); + $kernel->setIgnoredCIConfigs([Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); if (class_exists(HTTPApplication::class)) { // Mock request diff --git a/src/Dev/TestKernel.php b/src/Dev/TestKernel.php index 1e7708680..3b4f9541d 100644 --- a/src/Dev/TestKernel.php +++ b/src/Dev/TestKernel.php @@ -51,15 +51,15 @@ class TestKernel extends CoreKernel * Set a list of CI configurations that should cause a module's test not to be added to a manifest * @param string[] $ciConfigs */ - public function setIgnoreCIConfigs(array $ciLibs): self + public function setIgnoredCIConfigs(array $ciConfigs): self { - $this->ciLibs = $ciLibs; + $this->ciConfigs = $ciConfigs; return $this; } - protected function getIgnoreCIConfigs(): array + protected function getIgnoredCIConfigs(): array { - return $this->ciLibs; + return $this->ciConfigs; } protected function bootErrorHandling() diff --git a/src/View/ThemeManifest.php b/src/View/ThemeManifest.php index 9c2818985..d68fb8d92 100644 --- a/src/View/ThemeManifest.php +++ b/src/View/ThemeManifest.php @@ -139,7 +139,7 @@ class ThemeManifest implements ThemeList 'include_themes' => false, 'ignore_dirs' => ['node_modules', THEMES_DIR], 'ignore_tests' => !$includeTests, - 'ignore_ci_configs' => $ignoredCIConfigs, + 'ignored_ci_configs' => $ignoredCIConfigs, 'dir_callback' => [$this, 'handleDirectory'] ]); diff --git a/tests/php/Core/Manifest/ManifestFileFinderTest.php b/tests/php/Core/Manifest/ManifestFileFinderTest.php index 4f90ed3aa..f39a1bbc1 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_configs', [Module::CI_PHPUNIT_FIVE]); + $finder->setOption('ignored_ci_configs', [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_configs', [Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); + $finder->setOption('ignored_ci_configs', [Module::CI_PHPUNIT_FIVE, Module::CI_UNKNOWN]); $this->assertFinderFinds( $finder,