Reduce state spill over between tests

A PHPUnit listener unsets SSP environmental variables and attempts
to restore globals.

lib/SimpleSAML/Configuration.php

@@ -7,7 +7,7 @@
  * @author Andreas Aakre Solberg, UNINETT AS. <andreas.solberg@uninett.no>
  * @package SimpleSAMLphp
  */
-class SimpleSAML_Configuration
+class SimpleSAML_Configuration implements \SimpleSAML\Utils\ClearableState
 {
 
     /**
@@ -309,7 +309,6 @@ class SimpleSAML_Configuration
             } catch (SimpleSAML\Error\ConfigurationError $e) {
                 throw \SimpleSAML\Error\CriticalConfigurationError::fromException($e);
             }
-
         }
 
         throw new \SimpleSAML\Error\CriticalConfigurationError(
@@ -1366,4 +1365,16 @@ class SimpleSAML_Configuration
             return null;
         }
     }
+
+    /**
+     * Clear any configuration information cached.
+     * Allows for configuration files to be changed and reloaded during a given request. Most useful
+     * when running phpunit tests and needing to alter config.php between test cases
+     */
+    public static function clearInternalState()
+    {
+        self::$configDirs = array();
+        self::$instance = array();
+        self::$loadedConfigs = array();
+    }
 }

lib/SimpleSAML/Utils/ClearableState.php

+<?php
+
+namespace SimpleSAML\Utils;
+
+/**
+ * Indicates an implementation caches state internally and may be cleared.
+ *
+ * Primarily designed to allow SSP state to be cleared between unit tests.
+ * @package SimpleSAML\Utils
+ */
+interface ClearableState
+{
+    /**
+     * Clear any cached internal state.
+     */
+    public static function clearInternalState();
+}

tests/Utils/ClearStateTestListener.php

+<?php
+namespace SimpleSAML\Test\Utils;
+
+use PHPUnit_Framework_Test;
+use PHPUnit_Framework_TestSuite;
+
+/**
+ * A PHPUnit test listener that attempts to clear global state and cached SSP configuration between test run
+ */
+class ClearStateTestListener extends \PHPUnit_Framework_BaseTestListener
+{
+
+    /**
+     * Global state to restore between test runs
+     * @var array
+     */
+    private static $backups = array();
+
+    /**
+     * Class that implement \SimpleSAML\Utils\ClearableState and should have clearInternalState called between tests
+     * @var array
+     */
+    private static $clearableState = array('SimpleSAML_Configuration');
+
+    /**
+     * Variables
+     * @var array
+     */
+    private static $vars_to_unset = array('SIMPLESAMLPHP_CONFIG_DIR');
+
+    public function __construct()
+    {
+        // Backup any state that is needed as part of processing, so we can restore it later.
+        // TODO: phpunit's backupGlobals = false, yet we are trying to do a similar thing here. Is that an issue?
+        if (!self::$backups) {
+            self::$backups['$_COOKIE'] = $_COOKIE;
+            self::$backups['$_ENV'] = $_ENV;
+            self::$backups['$_FILES'] = $_FILES;
+            self::$backups['$_GET'] = $_GET;
+            self::$backups['$_POST'] = $_POST;
+            self::$backups['$_SERVER'] = $_SERVER;
+            self::$backups['$_SESSION'] = isset($_SESSION) ? $_SESSION : [];
+            self::$backups['$_REQUEST'] = $_REQUEST;
+        }
+    }
+
+    /**
+     * Clear any state needed prior to a test file running
+     */
+    public function startTestSuite(PHPUnit_Framework_TestSuite $suite)
+    {
+        // TODO: decide how to handle tests that want to set suite level settings with setUpBeforeClass()
+    }
+
+    /**
+     * Clear any state needed prior to a test case
+     * @param PHPUnit_Framework_Test $test
+     */
+    public function startTest(PHPUnit_Framework_Test $test)
+    {
+        $_COOKIE = self::$backups['$_COOKIE'];
+        $_ENV = self::$backups['$_ENV'];
+        $_FILES = self::$backups['$_FILES'];
+        $_GET = self::$backups['$_GET'];
+        $_POST = self::$backups['$_POST'];
+        $_SERVER = self::$backups['$_SERVER'];
+        $_SESSION = self::$backups['$_SESSION'];
+        $_REQUEST = self::$backups['$_REQUEST'];
+
+        foreach (self::$clearableState as $var) {
+            $var::clearInternalState();
+        }
+
+        foreach (self::$vars_to_unset as $var) {
+            putenv($var);
+        }
+    }
+}

tests/config/no-options/config.php

+<?php
+/* A configuration desigend for tests that need no configuration options set yet the SSP requires a config.php to have
+been loaded.
+*/
+$config = array(
+    // We need to set at least one key=value pair to avoid validation issues loading the file
+    'some_example_option' => 'a',
+);
\ No newline at end of file

tests/lib/SimpleSAML/ConfigurationTest.php

@@ -31,6 +31,13 @@ class Test_SimpleSAML_Configuration extends PHPUnit_Framework_TestCase
      */
     public function testCriticalConfigurationError()
     {
+        try {
+            SimpleSAML_Configuration::getInstance();
+            $this->fail('Exception expected');
+        } catch (\SimpleSAML\Error\CriticalConfigurationError $var) {
+            // This exception is expected.
+        }
+        //TODO: not sure this is correct. First call to getInstance() throw exception and the 2nd works?...
         $c = SimpleSAML_Configuration::getInstance();
         $this->assertNotEmpty($c->toArray());
     }
@@ -97,6 +104,9 @@ class Test_SimpleSAML_Configuration extends PHPUnit_Framework_TestCase
      * Test SimpleSAML_Configuration::getBaseURL()
      */
     public function testGetBaseURL() {
+
+        // Need to set a configuration file because the SSP Logger attempts to load it.
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=tests/config/no-options');
         $c = SimpleSAML_Configuration::loadFromArray(array());
         $this->assertEquals($c->getBaseURL(), 'simplesaml/');
 

tests/lib/SimpleSAML/Locale/LocalizationTest.php

@@ -6,6 +6,11 @@ use SimpleSAML\Locale\Localization;
 
 class LocalizationTest extends \PHPUnit_Framework_TestCase
 {
+    protected function setUp()
+    {
+        // Localization/Language code attempts to load a cookie, and looks to config.php for a name of the cookie
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=tests/config/no-options');
+    }
 
 
     /**

tests/modules/consent/lib/Auth/Process/ConsentTest.php

@@ -37,6 +37,9 @@ class ConsentTest extends \PHPUnit_Framework_TestCase
      */
     public function testCheckDisable()
     {
+        // Consent code path attempts to load metadata, which tries to find a config.php
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=tests/config/no-options');
+
         // test consent disable regex with match
         $config = array();
 

tests/modules/saml/lib/Auth/Source/Auth_Source_SP_Test.php

@@ -125,6 +125,7 @@ class SP_Test extends \PHPUnit_Framework_TestCase
         );
 
         $this->config = Configuration::loadFromArray(array(), '[ARRAY]', 'simplesaml');
+
     }
 
 

tools/phpunit/phpunit.xml

@@ -31,4 +31,8 @@
         <log type="coverage-html" target="build/coverage" title="PHP Coveralls" charset="UTF-8" yui="true" highlight="true" lowUpperBound="35" highLowerBound="70"/>
         <log type="coverage-clover" target="build/logs/clover.xml"/>
     </logging>
+    <listeners>
+        <listener class="\SimpleSAML\Test\Utils\ClearStateTestListener" file="./tests/Utils/ClearStateTestListener.php">
+        </listener>
+    </listeners>
 </phpunit>