API Move extension management into test state

_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

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);

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;

src/Dev/ExtensionTestState.php

@@ -0,0 +1,122 @@
+<?php
+
+namespace SilverStripe\Dev;
+
+use LogicException;
+use SilverStripe\Core\Extension;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\ORM\DataObject;
+
+/**
+ * Manages illegal and required extensions for sapphiretest
+ */
+class ExtensionTestState implements TestState
+{
+    /**
+     * @var array
+     */
+    protected $extensionsToReapply = [];
+
+    /**
+     * @var array
+     */
+    protected $extensionsToRemove = [];
+
+    /**
+     * Called on setup
+     *
+     * @param SapphireTest $test
+     */
+    public function setUp(SapphireTest $test)
+    {
+    }
+
+    public function tearDown(SapphireTest $test)
+    {
+    }
+
+    public function setUpOnce($class)
+    {
+        $isAltered = false;
+        /** @var string|SapphireTest $class */
+        /** @var string|DataObject $dataClass */
+        // Remove any illegal extensions that are present
+        foreach ($class::getIllegalExtensions() as $dataClass => $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);
+            }
+        }
+    }
+}

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::$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;
     }

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;

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)) {

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,

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,

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,

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,

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)) {

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,

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,

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)) {

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,

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,

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();
 

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,