Merge pull request #3979 from dhensby/pulls/test-nest

Nest and unnest Config and Controller for each test

dev/SapphireTest.php

@@ -166,6 +166,11 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	protected $model;
 
 	public function setUp() {
+
+		//nest config and injector for each test so they are effectively sandboxed per test
+		Config::nest();
+		Injector::nest();
+
 		// We cannot run the tests on this abstract class.
 		if(get_class($this) == "SapphireTest") $this->skipTest = true;
 
@@ -285,6 +290,10 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * for tearing down the state again.
 	 */
 	public function setUpOnce() {
+
+		//nest config and injector for each suite so they are effectively sandboxed
+		Config::nest();
+		Injector::nest();
 		$isAltered = false;
 
 		if(!Director::isDev()) {
@@ -333,24 +342,12 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 	 * tearDown method that's called once per test class rather once per test method.
 	 */
 	public function tearDownOnce() {
-		// If we have made changes to the extensions present, then migrate the database schema.
-		if($this->extensionsToReapply || $this->extensionsToRemove) {
-			// Remove extensions added for testing
-			foreach($this->extensionsToRemove as $class => $extensions) {
-				foreach($extensions as $extension) {
-					$class::remove_extension($extension);
-				}
-			}
+		//unnest injector / config now that the test suite is over
+		// this will reset all the extensions on the object too (see setUpOnce)
+		Injector::unnest();
+		Config::unnest();
 
-			// Reapply ones removed
-			foreach($this->extensionsToReapply as $class => $extensions) {
-				foreach($extensions as $extension) {
-					$class::add_extension($extension);
-				}
-			}
-		}
-
-		if($this->extensionsToReapply || $this->extensionsToRemove || $this->extraDataObjects) {
+		if(!empty($this->extensionsToReapply) || !empty($this->extensionsToRemove) || !empty($this->extraDataObjects)) {
 			$this->resetDBSchema();
 		}
 	}
@@ -501,6 +498,9 @@ class SapphireTest extends PHPUnit_Framework_TestCase {
 			$controller->response->setStatusCode(200);
 			$controller->response->removeHeader('Location');
 		}
+		//unnest injector / config now that tests are over
+		Injector::unnest();
+		Config::unnest();
 	}
 
 	public static function assertContains(

docs/en/02_Developer_Guides/06_Testing/00_Unit_Testing.md

@@ -182,18 +182,10 @@ end of each test.
 				$page->publish('Stage', 'Live');
 			}
 
-			// reset configuration for the test.
-			Config::nest();
+			// set custom configuration for the test.
 			Config::inst()->update('Foo', 'bar', 'Hello!');
 		}
 
-		public function tearDown() {
-			// restores the config variables
-			Config::unnest();
-
-			parent::tearDown();
-		}
-
 		public function testMyMethod() {
 			// ..
 		}
@@ -224,6 +216,32 @@ individual test case.
 		}
 	}
 	
+### Config and Injector Nesting
+
+A powerful feature of both [`Config`](/developer_guides/configuration/configuration/) and [`Injector`](/developer_guides/extending/injector/) is the ability to "nest" them so that you can make changes that can easily be discarded without having to manage previous values.
+
+The testing suite makes use of this to "sandbox" each of the unit tests as well as each suite to prevent leakage between tests.
+
+If you need to make changes to `Config` (or `Injector) for each test (or the whole suite) you can safely update `Config` (or `Injector`) settings in the `setUp` or `tearDown` functions.
+
+It's important to remember that the `parent::setUp();` functions will need to be called first to ensure the nesting feature works as expected.
+
+	:::php
+	function setUpOnce() {
+		parent::setUpOnce();
+		//this will remain for the whole suite and be removed for any other tests
+		Config::inst()->update('ClassName', 'var_name', 'var_value');
+	}
+	
+	function testFeatureDoesAsExpected() {
+		//this will be reset to 'var_value' at the end of this test function
+		Config::inst()->update('ClassName', 'var_name', 'new_var_value');
+	}
+	
+	function testAnotherFeatureDoesAsExpected() {
+		Config::inst()->get('ClassName', 'var_name'); // this will be 'var_value'
+	}
+
 ## Generating a Coverage Report
 
 PHPUnit can generate a code coverage report ([docs](http://www.phpunit.de/manual/current/en/code-coverage-analysis.html))

tests/control/DirectorTest.php

@@ -18,9 +18,6 @@ class DirectorTest extends SapphireTest {
 	public function setUp() {
 		parent::setUp();
 
-		// Required for testRequestFilterInDirectorTest
-		Injector::nest();
-
 		// Hold the original request URI once so it doesn't get overwritten
 		if(!self::$originalRequestURI) {
 			self::$originalRequestURI = $_SERVER['REQUEST_URI'];
@@ -53,9 +50,6 @@ class DirectorTest extends SapphireTest {
 	public function tearDown() {
 		// TODO Remove director rule, currently API doesnt allow this
 
-		// Remove base URL override (setting to false reverts to default behaviour)
-		Config::inst()->update('Director', 'alternate_base_url', false);
-		
 		$_GET = $this->originalGet;
 		$_SESSION = $this->originalSession;
 
@@ -68,8 +62,6 @@ class DirectorTest extends SapphireTest {
 			}
 		}
 
-		Injector::unnest();
-
 		parent::tearDown();
 	}
 

tests/core/ConfigTest.php

@@ -189,8 +189,7 @@ class ConfigTest extends SapphireTest {
 		// But it won't affect subclasses - this is *uninherited* static
 		$this->assertNotContains('test_2b',
 			Config::inst()->get('ConfigStaticTest_Third', 'first', Config::UNINHERITED));
-		$this->assertNotContains('test_2b',
-			Config::inst()->get('ConfigStaticTest_Fourth', 'first', Config::UNINHERITED));
+		$this->assertNull(Config::inst()->get('ConfigStaticTest_Fourth', 'first', Config::UNINHERITED));
 
 		// Subclasses that don't have the static explicitly defined should allow definition, also
 		// This also checks that set can be called after the first uninherited get()

tests/core/manifest/ConfigManifestTest.php

@@ -391,7 +391,7 @@ class ConfigManifestTest extends SapphireTest {
 
 	public function testEnvironmentRules() {
 		foreach (array('dev', 'test', 'live') as $env) {
-			Config::inst()->nest();
+			Config::nest();
 
 			Config::inst()->update('Director', 'environment_type', $env);
 			$config = $this->getConfigFixtureValue('Environment');
@@ -403,13 +403,11 @@ class ConfigManifestTest extends SapphireTest {
 				);
 			}
 
-			Config::inst()->unnest();
+			Config::unnest();
 		}
 	}
 
 	public function testDynamicEnvironmentRules() {
-		Config::inst()->nest();
-
 		// First, make sure environment_type is live
 		Config::inst()->update('Director', 'environment_type', 'live');
 		$this->assertEquals('live', Config::inst()->get('Director', 'environment_type'));
@@ -423,8 +421,6 @@ class ConfigManifestTest extends SapphireTest {
 
 		// And that the dynamic rule was calculated correctly
 		$this->assertEquals('dev', Config::inst()->get('ConfigManifestTest', 'DynamicEnvironment'));
-
-		Config::inst()->unnest();
 	}
 
 	public function testMultipleRules() {

tests/dev/DevAdminControllerTest.php

@@ -11,7 +11,7 @@ class DevAdminControllerTest extends FunctionalTest {
 	public function setUp(){
 		parent::setUp();
 
-		Config::nest()->update('DevelopmentAdmin', 'registered_controllers', array(
+		Config::inst()->update('DevelopmentAdmin', 'registered_controllers', array(
 			'x1' => array(
 				'controller' => 'DevAdminControllerTest_Controller1',
 				'links' => array(
@@ -28,11 +28,6 @@ class DevAdminControllerTest extends FunctionalTest {
 		));
 	}
 
-	public function tearDown(){
-		parent::tearDown();
-		Config::unnest();
-	}
-	
 
 	public function testGoodRegisteredControllerOutput(){
 		// Check for the controller running from the registered url above

tests/model/DataObjectSchemaGenerationTest.php

@@ -43,7 +43,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		// Table will have been initially created by the $extraDataObjects setting
 
 		// Let's insert a new field here
-		Config::nest();
 		Config::inst()->update('DataObjectSchemaGenerationTest_DO', 'db', array(
 			'SecretField' => 'Varchar(100)'
 		));
@@ -55,9 +54,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		$needsUpdating = $db->doesSchemaNeedUpdating();
 		$db->cancelSchemaUpdate();
 		$this->assertTrue($needsUpdating);
-		
-		// Restore db configuration
-		Config::unnest();
 	}
 
 	/**
@@ -78,7 +74,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		$this->assertFalse($needsUpdating);
 
 		// Test with alternate index format, although these indexes are the same
-		Config::nest();
 		Config::inst()->remove('DataObjectSchemaGenerationTest_IndexDO', 'indexes');
 		Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'indexes',
 			Config::inst()->get('DataObjectSchemaGenerationTest_IndexDO', 'indexes_alt')
@@ -91,9 +86,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		$needsUpdating = $db->doesSchemaNeedUpdating();
 		$db->cancelSchemaUpdate();
 		$this->assertFalse($needsUpdating);
-		
-		// Restore old index format
-		Config::unnest();
 	}
 
 	/**
@@ -106,7 +98,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		// Table will have been initially created by the $extraDataObjects setting
 
 		// Update the SearchFields index here
-		Config::nest();
 		Config::inst()->update('DataObjectSchemaGenerationTest_IndexDO', 'indexes', array(
 			'SearchFields' => array(
 				'value' => 'Title'
@@ -120,9 +111,6 @@ class DataObjectSchemaGenerationTest extends SapphireTest {
 		$needsUpdating = $db->doesSchemaNeedUpdating();
 		$db->cancelSchemaUpdate();
 		$this->assertTrue($needsUpdating);
-		
-		// Restore old indexes
-		Config::unnest();
 	}
 
 	/**

tests/model/MySQLDatabaseTest.php

@@ -10,6 +10,7 @@ class MySQLDatabaseTest extends SapphireTest {
 	);
 
 	public function setUp() {
+		parent::setUp();
 		if(DB::getConn() instanceof MySQLDatabase) {
 			MySQLDatabaseTest_DO::config()->db = array(
 				'MultiEnum1' => 'MultiEnum("A, B, C, D","")',
@@ -18,7 +19,6 @@ class MySQLDatabaseTest extends SapphireTest {
 			);
 		}
 		$this->markTestSkipped('This test requires the Config API to be immutable');
-		parent::setUp();
 	}
 
 	/**

tests/oembed/OembedTest.php

@@ -1,16 +1,6 @@
 <?php
 
 class OembedTest extends SapphireTest {
-	public function setUp() {
-		parent::setUp();
-		Config::nest();
-	}
-
-	public function tearDown() {
-		Config::unnest();
-		parent::tearDown();
-	}
-
 	public function testGetOembedFromUrl() {
 		Config::inst()->update('Oembed', 'providers', array(
 			'http://*.silverstripe.com/watch*'=>'http://www.silverstripe.com/oembed/'

tests/security/BasicAuthTest.php

@@ -14,16 +14,10 @@ class BasicAuthTest extends FunctionalTest {
 		parent::setUp();
 
 		// Fixtures assume Email is the field used to identify the log in identity
-		Config::nest();
 		Member::config()->unique_identifier_field = 'Email';
 		Member::config()->lock_out_after_incorrect_logins = 10;
 	}
 
-	public function tearDown() {
-		Config::unnest();
-		parent::tearDown();
-	}
-
 	public function testBasicAuthEnabledWithoutLogin() {
 		$origUser = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null;
 		$origPw = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null;