From ca033952513633e182040d3d13e1caa9000ca184 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 13 Jun 2017 15:00:00 +1200 Subject: [PATCH] API Move extension management into test state --- _config/tests.yml | 1 + src/Core/Manifest/ClassManifest.php | 16 +- src/Core/TestKernel.php | 8 + src/Dev/ExtensionTestState.php | 122 ++++++++++++++++ src/Dev/SapphireTest.php | 146 +++++-------------- tests/php/Core/ObjectTest.php | 2 + tests/php/Forms/FormFactoryTest.php | 2 +- tests/php/ORM/DataListTest.php | 2 +- tests/php/ORM/DataObjectLazyLoadingTest.php | 2 +- tests/php/ORM/DataObjectTest.php | 2 +- tests/php/ORM/HasManyListTest.php | 2 +- tests/php/ORM/HierarchyTest.php | 2 +- tests/php/ORM/ManyManyListTest.php | 2 +- tests/php/ORM/MapTest.php | 2 +- tests/php/ORM/MarkedSetTest.php | 2 +- tests/php/ORM/PaginatedListTest.php | 2 +- tests/php/ORM/PolymorphicHasManyListTest.php | 2 +- tests/php/View/SSViewerCacheBlockTest.php | 2 +- tests/php/i18n/i18nTestManifest.php | 2 +- 19 files changed, 188 insertions(+), 133 deletions(-) create mode 100644 src/Dev/ExtensionTestState.php diff --git a/_config/tests.yml b/_config/tests.yml index 8b40c77e5..d4b53dda2 100644 --- a/_config/tests.yml +++ b/_config/tests.yml @@ -5,5 +5,6 @@ SilverStripe\Core\Injector\Injector: SilverStripe\Dev\SapphireTestState: properties: States: + extensions: %$SilverStripe\Dev\ExtensionTestState flushable: %$SilverStripe\Dev\FlushableTestState requirements: %$SilverStripe\View\Dev\RequirementsTestState diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 244e630a3..3dafb69d6 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -35,14 +35,6 @@ class ClassManifest */ protected $cacheFactory; - /** - * Set if including test classes - * - * @see TestOnly - * @var bool - */ - protected $tests; - /** * Cache to use, if caching. * Set to null if uncached. @@ -161,7 +153,7 @@ class ClassManifest $this->implementors = $data['implementors']; $this->traits = $data['traits']; } else { - $this->regenerate(); + $this->regenerate($includeTests); } } @@ -356,8 +348,10 @@ class ClassManifest /** * Completely regenerates the manifest file. + * + * @param bool $includeTests */ - public function regenerate() + public function regenerate($includeTests) { $resets = array( 'classes', 'roots', 'children', 'descendants', 'interfaces', @@ -373,7 +367,7 @@ class ClassManifest $finder->setOptions(array( 'name_regex' => '/^[^_].*\\.php$/', 'ignore_files' => array('index.php', 'main.php', 'cli-script.php'), - 'ignore_tests' => !$this->tests, + 'ignore_tests' => !$includeTests, 'file_callback' => array($this, 'handleFile'), )); $finder->find($this->base); diff --git a/src/Core/TestKernel.php b/src/Core/TestKernel.php index fd76ce2e6..6ef7433b4 100644 --- a/src/Core/TestKernel.php +++ b/src/Core/TestKernel.php @@ -23,6 +23,14 @@ class TestKernel extends AppKernel $this->bootPHP(); } + protected function bootPHP() + { + parent::bootPHP(); + + // Set default timezone consistently to avoid NZ-specific dependencies + date_default_timezone_set('UTC'); + } + protected function getIncludeTests() { return true; diff --git a/src/Dev/ExtensionTestState.php b/src/Dev/ExtensionTestState.php new file mode 100644 index 000000000..acd7c5c1e --- /dev/null +++ b/src/Dev/ExtensionTestState.php @@ -0,0 +1,122 @@ + $extensions) { + if (!class_exists($dataClass)) { + continue; + } + if ($extensions === '*') { + $extensions = $dataClass::get_extensions(); + } + foreach ($extensions as $extension) { + if (!class_exists($extension) || !$dataClass::has_extension($extension)) { + continue; + } + if (!isset($this->extensionsToReapply[$dataClass])) { + $this->extensionsToReapply[$dataClass] = array(); + } + $this->extensionsToReapply[$dataClass][] = $extension; + $dataClass::remove_extension($extension); + $isAltered = true; + } + } + + // Add any required extensions that aren't present + foreach ($class::getRequiredExtensions() as $dataClass => $extensions) { + if (!class_exists($dataClass)) { + throw new LogicException("Test {$class} requires dataClass {$dataClass} which doesn't exist"); + } + $this->extensionsToRemove[$dataClass] = array(); + foreach ($extensions as $extension) { + $dataClass = Extension::get_classname_without_arguments($extension); + if (!class_exists($dataClass)) { + $self = static::class; + throw new LogicException("Test {$self} requires extension {$extension} which doesn't exist"); + } + if (!$dataClass::has_extension($extension)) { + if (!isset($this->extensionsToRemove[$dataClass])) { + $this->extensionsToReapply[$dataClass] = array(); + } + $this->extensionsToRemove[$dataClass][] = $extension; + $dataClass::add_extension($extension); + $isAltered = true; + } + } + } + + // If we have made changes to the extensions present, then migrate the database schema. + if ($isAltered || $this->extensionsToReapply || $this->extensionsToRemove || $class::getExtraDataObjects()) { + DataObject::reset(); + if (!SapphireTest::using_temp_db()) { + SapphireTest::create_temp_db(); + } + SapphireTest::resetDBSchema(true); + } + + // clear singletons, they're caching old extension info + // which is used in DatabaseAdmin->doBuild() + Injector::inst()->unregisterObjects(DataObject::class); + } + + public function tearDownOnce($class) + { + // @todo: This isn't strictly necessary to restore extensions, but only to ensure that + // Object::$extra_methods is properly flushed. This should be replaced with a simple + // flush mechanism for each $class. + /** @var string|DataObject $dataClass */ + + // Remove extensions added for testing + foreach ($this->extensionsToRemove as $dataClass => $extensions) { + foreach ($extensions as $extension) { + $dataClass::remove_extension($extension); + } + } + + // Reapply ones removed + foreach ($this->extensionsToReapply as $dataClass => $extensions) { + foreach ($extensions as $extension) { + $dataClass::add_extension($extension); + } + } + } +} diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index 9db56c892..910ad4cce 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -15,7 +15,6 @@ use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Extension; use SilverStripe\Core\HTTPApplication; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\TestKernel; @@ -128,20 +127,6 @@ class SapphireTest extends PHPUnit_Framework_TestCase */ protected $backupGlobals = false; - /** - * Helper arrays for illegal_extensions/required_extensions code - */ - private static $extensions_to_reapply = []; - - private static $extensions_to_remove = []; - - /** - * Check flushables on setupBeforeClass() - * - * @var bool - */ - protected static $flushedFlushables = false; - /** * Test application kernel. * Note: This is always the root kernel. Use Injector to get the current kernel @@ -151,6 +136,33 @@ class SapphireTest extends PHPUnit_Framework_TestCase */ protected static $kernel = null; + /** + * State management container for SapphireTest + * + * @var TestState + */ + protected static $state = null; + + /** + * Gets illegal extensions for this class + * + * @return array + */ + public static function getIllegalExtensions() + { + return static::$illegal_extensions; + } + + /** + * Gets required extensions for this class + * + * @return array + */ + public static function getRequiredExtensions() + { + return static::$required_extensions; + } + /** * Check if test bootstrapping has been performed. Must not be relied on * outside of unit tests. @@ -180,11 +192,6 @@ class SapphireTest extends PHPUnit_Framework_TestCase return static::$fixture_file; } - /** - * @var TestState - */ - protected static $state = null; - /** * Setup the test. * Always sets up in order: @@ -286,74 +293,15 @@ class SapphireTest extends PHPUnit_Framework_TestCase */ public static function setUpBeforeClass() { + // Start tests static::start(); + // Reset kernel static::$kernel->reset(); //nest config and injector for each suite so they are effectively sandboxed Config::nest(); Injector::nest(); - $isAltered = false; - - // Remove any illegal extensions that are present - foreach (static::$illegal_extensions as $class => $extensions) { - if (!class_exists($class)) { - continue; - } - if ($extensions === '*') { - $extensions = $class::get_extensions(); - } - foreach ($extensions as $extension) { - if (!class_exists($extension) || !$class::has_extension($extension)) { - continue; - } - if (!isset(self::$extensions_to_reapply[$class])) { - self::$extensions_to_reapply[$class] = array(); - } - self::$extensions_to_reapply[$class][] = $extension; - $class::remove_extension($extension); - $isAltered = true; - } - } - - // Add any required extensions that aren't present - foreach (static::$required_extensions as $class => $extensions) { - if (!class_exists($class)) { - $self = static::class; - throw new LogicException("Test {$self} requires class {$class} which doesn't exist"); - } - self::$extensions_to_remove[$class] = array(); - foreach ($extensions as $extension) { - $extensionClass = Extension::get_classname_without_arguments($extension); - if (!class_exists($extensionClass)) { - $self = static::class; - throw new LogicException("Test {$self} requires extension {$extension} which doesn't exist"); - } - if (!$class::has_extension($extension)) { - if (!isset(self::$extensions_to_remove[$class])) { - self::$extensions_to_reapply[$class] = array(); - } - self::$extensions_to_remove[$class][] = $extension; - $class::add_extension($extension); - $isAltered = true; - } - } - } - - // If we have made changes to the extensions present, then migrate the database schema. - if ($isAltered || self::$extensions_to_reapply || self::$extensions_to_remove || static::getExtraDataObjects()) { - DataObject::reset(); - if (!self::using_temp_db()) { - self::create_temp_db(); - } - static::resetDBSchema(true); - } - // clear singletons, they're caching old extension info - // which is used in DatabaseAdmin->doBuild() - Injector::inst()->unregisterObjects(DataObject::class); - - // Set default timezone consistently to avoid NZ-specific dependencies - date_default_timezone_set('UTC'); // Call state helpers static::$state->setUpOnce(static::class); @@ -374,36 +322,12 @@ class SapphireTest extends PHPUnit_Framework_TestCase // Call state helpers static::$state->tearDownOnce(static::class); - // If we have made changes to the extensions present, then migrate the database schema. - if (self::$extensions_to_reapply || self::$extensions_to_remove) { - // @todo: This isn't strictly necessary to restore extensions, but only to ensure that - // Object::$extra_methods is properly flushed. This should be replaced with a simple - // flush mechanism for each $class. - // - // Remove extensions added for testing - foreach (self::$extensions_to_remove as $class => $extensions) { - foreach ($extensions as $extension) { - $class::remove_extension($extension); - } - } - - // Reapply ones removed - foreach (self::$extensions_to_reapply as $class => $extensions) { - foreach ($extensions as $extension) { - $class::add_extension($extension); - } - } - } - //unnest injector / config now that the test suite is over // this will reset all the extensions on the object too (see setUpBeforeClass) Injector::unnest(); Config::unnest(); - $extraDataObjects = static::getExtraDataObjects(); - if (!empty(self::$extensions_to_reapply) || !empty(self::$extensions_to_remove) || !empty($extraDataObjects)) { - static::resetDBSchema(); - } + static::resetDBSchema(); static::$kernel->reset(); } @@ -1013,6 +937,8 @@ class SapphireTest extends PHPUnit_Framework_TestCase /** * Returns true if we are currently using a temporary database + * + * @return bool */ public static function using_temp_db() { @@ -1021,6 +947,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase return 1 === preg_match(sprintf('/^%stmpdb_[0-9]+_[0-9]+$/i', preg_quote($prefix, '/')), $dbConn->getSelectedDatabase()); } + /** + * Destroy all temp databases + */ public static function kill_temp_db() { // Delete our temporary database @@ -1234,7 +1163,6 @@ class SapphireTest extends PHPUnit_Framework_TestCase */ protected $cache_generatedMembers = array(); - /** * Test against a theme. * @@ -1291,7 +1219,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase * Return all extra objects to scaffold for this test * @return array */ - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return static::$extra_dataobjects; } @@ -1301,7 +1229,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase * * @return array */ - protected static function getExtraControllers() + public static function getExtraControllers() { return static::$extra_controllers; } diff --git a/tests/php/Core/ObjectTest.php b/tests/php/Core/ObjectTest.php index 93b27f799..2008f2653 100644 --- a/tests/php/Core/ObjectTest.php +++ b/tests/php/Core/ObjectTest.php @@ -4,8 +4,10 @@ namespace SilverStripe\Core\Tests; use SilverStripe\Control\Controller; use SilverStripe\Core\ClassInfo; +use SilverStripe\Core\Config\Config; use SilverStripe\Core\Extension; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Core\Manifest\ClassLoader; use SilverStripe\Core\Tests\ObjectTest\BaseObject; use SilverStripe\Core\Tests\ObjectTest\ExtendTest1; use SilverStripe\Core\Tests\ObjectTest\ExtendTest2; diff --git a/tests/php/Forms/FormFactoryTest.php b/tests/php/Forms/FormFactoryTest.php index 52ac9e0d8..981db7779 100644 --- a/tests/php/Forms/FormFactoryTest.php +++ b/tests/php/Forms/FormFactoryTest.php @@ -19,7 +19,7 @@ class FormFactoryTest extends SapphireTest protected static $fixture_file = 'FormFactoryTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { // Prevent setup breaking if versioned module absent if (class_exists(Versioned::class)) { diff --git a/tests/php/ORM/DataListTest.php b/tests/php/ORM/DataListTest.php index 6b55bff21..eeb21fdd4 100755 --- a/tests/php/ORM/DataListTest.php +++ b/tests/php/ORM/DataListTest.php @@ -27,7 +27,7 @@ class DataListTest extends SapphireTest // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/DataObjectLazyLoadingTest.php b/tests/php/ORM/DataObjectLazyLoadingTest.php index 7f0429b31..e1c9e06b4 100644 --- a/tests/php/ORM/DataObjectLazyLoadingTest.php +++ b/tests/php/ORM/DataObjectLazyLoadingTest.php @@ -15,7 +15,7 @@ class DataObjectLazyLoadingTest extends SapphireTest 'DataObjectTest.yml', ); - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 8af3bed35..43b280177 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -57,7 +57,7 @@ class DataObjectTest extends SapphireTest DataObjectTest\RelationChildSecond::class, ); - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/HasManyListTest.php b/tests/php/ORM/HasManyListTest.php index 93216e0e1..2d4567026 100644 --- a/tests/php/ORM/HasManyListTest.php +++ b/tests/php/ORM/HasManyListTest.php @@ -11,7 +11,7 @@ class HasManyListTest extends SapphireTest // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/HierarchyTest.php b/tests/php/ORM/HierarchyTest.php index 330c07df4..51780bfba 100644 --- a/tests/php/ORM/HierarchyTest.php +++ b/tests/php/ORM/HierarchyTest.php @@ -16,7 +16,7 @@ class HierarchyTest extends SapphireTest HierarchyTest\HideTestSubObject::class, ); - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { // Prevent setup breaking if versioned module absent if (class_exists(Versioned::class)) { diff --git a/tests/php/ORM/ManyManyListTest.php b/tests/php/ORM/ManyManyListTest.php index dc1ec8a6d..ff684080b 100644 --- a/tests/php/ORM/ManyManyListTest.php +++ b/tests/php/ORM/ManyManyListTest.php @@ -20,7 +20,7 @@ class ManyManyListTest extends SapphireTest ManyManyListTest\Product::class, ]; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/MapTest.php b/tests/php/ORM/MapTest.php index 94345f954..1d4e31970 100755 --- a/tests/php/ORM/MapTest.php +++ b/tests/php/ORM/MapTest.php @@ -14,7 +14,7 @@ class MapTest extends SapphireTest // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/MarkedSetTest.php b/tests/php/ORM/MarkedSetTest.php index e85ebe025..1b43367af 100644 --- a/tests/php/ORM/MarkedSetTest.php +++ b/tests/php/ORM/MarkedSetTest.php @@ -22,7 +22,7 @@ class MarkedSetTest extends SapphireTest HierarchyTest\HideTestSubObject::class, ); - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { // Prevent setup breaking if versioned module absent if (class_exists(Versioned::class)) { diff --git a/tests/php/ORM/PaginatedListTest.php b/tests/php/ORM/PaginatedListTest.php index dafebcfc5..fa222cb77 100644 --- a/tests/php/ORM/PaginatedListTest.php +++ b/tests/php/ORM/PaginatedListTest.php @@ -18,7 +18,7 @@ class PaginatedListTest extends SapphireTest protected static $fixture_file = 'DataObjectTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/ORM/PolymorphicHasManyListTest.php b/tests/php/ORM/PolymorphicHasManyListTest.php index 61a00ca3f..7980274cc 100644 --- a/tests/php/ORM/PolymorphicHasManyListTest.php +++ b/tests/php/ORM/PolymorphicHasManyListTest.php @@ -19,7 +19,7 @@ class PolymorphicHasManyListTest extends SapphireTest // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return array_merge( DataObjectTest::$extra_data_objects, diff --git a/tests/php/View/SSViewerCacheBlockTest.php b/tests/php/View/SSViewerCacheBlockTest.php index 3c8d394a5..3e50fa38c 100644 --- a/tests/php/View/SSViewerCacheBlockTest.php +++ b/tests/php/View/SSViewerCacheBlockTest.php @@ -19,7 +19,7 @@ class SSViewerCacheBlockTest extends SapphireTest SSViewerCacheBlockTest\TestModel::class ); - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { $classes = parent::getExtraDataObjects(); diff --git a/tests/php/i18n/i18nTestManifest.php b/tests/php/i18n/i18nTestManifest.php index 186764368..874c39fe5 100644 --- a/tests/php/i18n/i18nTestManifest.php +++ b/tests/php/i18n/i18nTestManifest.php @@ -50,7 +50,7 @@ trait i18nTestManifest */ protected $moduleManifests = 0; - protected static function getExtraDataObjects() + public static function getExtraDataObjects() { return [ TestDataObject::class,