diff --git a/cache/classes/dummystore.php b/cache/classes/dummystore.php index 1b925a4096a..c11e197cd0c 100644 --- a/cache/classes/dummystore.php +++ b/cache/classes/dummystore.php @@ -262,6 +262,15 @@ class cachestore_dummy extends cache_store { return $cache; } + /** + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). + */ + public static function unit_test_configuration() { + return []; + } + /** * Returns the name of this instance. * @return string diff --git a/cache/classes/factory.php b/cache/classes/factory.php index ac20becfd5f..4ba65657a25 100644 --- a/cache/classes/factory.php +++ b/cache/classes/factory.php @@ -277,6 +277,9 @@ class cache_factory { if (!array_key_exists($name, $this->stores)) { // Properties: name, plugin, configuration, class. $class = $details['class']; + if (!$class::are_requirements_met()) { + return false; + } $store = new $class($details['name'], $details['configuration']); $this->stores[$name] = $store; } diff --git a/cache/classes/helper.php b/cache/classes/helper.php index c25369e02da..30f82350699 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -502,15 +502,18 @@ class cache_helper { $store = $stores[$storename]; $class = $store['class']; + + // We check are_requirements_met although we expect is_ready is going to check as well. + if (!$class::are_requirements_met()) { + return false; + } // Found the store: is it ready? /* @var cache_store $instance */ $instance = new $class($store['name'], $store['configuration']); - // We check are_requirements_met although we expect is_ready is going to check as well. - if (!$instance::are_requirements_met() || !$instance->is_ready()) { + if (!$instance->is_ready()) { unset($instance); return false; } - foreach ($config->get_definitions_by_store($storename) as $id => $definition) { $definition = cache_definition::load($id, $definition); $definitioninstance = clone($instance); diff --git a/cache/classes/store.php b/cache/classes/store.php index eec660f663c..4fcb03ff20b 100644 --- a/cache/classes/store.php +++ b/cache/classes/store.php @@ -81,15 +81,11 @@ interface cache_store_interface { public static function initialise_test_instance(cache_definition $definition); /** - * Initialises a test instance for unit tests. + * Generates the appropriate configuration required for unit testing. * - * This differs from initialise_test_instance in that it doesn't rely on interacting with the config table. - * - * @since 2.8 - * @param cache_definition $definition - * @return cache_store|false + * @return array Array of unit test configuration data to be used by initialise(). */ - public static function initialise_unit_test_instance(cache_definition $definition); + public static function unit_test_configuration(); } /** @@ -369,20 +365,6 @@ abstract class cache_store implements cache_store_interface { return clone($this); } - /** - * Initialises a test instance for unit tests. - * - * This differs from initialise_test_instance in that it doesn't rely on interacting with the config table. - * By default however it calls initialise_test_instance to support backwards compatibility. - * - * @since 2.8 - * @param cache_definition $definition - * @return cache_store|false - */ - public static function initialise_unit_test_instance(cache_definition $definition) { - return static::initialise_test_instance($definition); - } - /** * Can be overridden to return any warnings this store instance should make to the admin. * diff --git a/cache/locallib.php b/cache/locallib.php index e97dd56eeb4..90a8a8b4b27 100644 --- a/cache/locallib.php +++ b/cache/locallib.php @@ -684,33 +684,36 @@ abstract class cache_administration_helper extends cache_helper { $locks = $instance->get_locks(); foreach ($stores as $name => $details) { $class = $details['class']; - $store = new $class($details['name'], $details['configuration']); + $store = false; + if ($class::are_requirements_met()) { + $store = new $class($details['name'], $details['configuration']); + } $lock = (isset($details['lock'])) ? $locks[$details['lock']] : $instance->get_default_lock(); $record = array( 'name' => $name, 'plugin' => $details['plugin'], 'default' => $details['default'], - 'isready' => $store->is_ready(), + 'isready' => $store ? $store->is_ready() : false, 'requirementsmet' => $class::are_requirements_met(), 'mappings' => 0, 'lock' => $lock, 'modes' => array( cache_store::MODE_APPLICATION => - ($store->get_supported_modes($return) & cache_store::MODE_APPLICATION) == cache_store::MODE_APPLICATION, + ($class::get_supported_modes($return) & cache_store::MODE_APPLICATION) == cache_store::MODE_APPLICATION, cache_store::MODE_SESSION => - ($store->get_supported_modes($return) & cache_store::MODE_SESSION) == cache_store::MODE_SESSION, + ($class::get_supported_modes($return) & cache_store::MODE_SESSION) == cache_store::MODE_SESSION, cache_store::MODE_REQUEST => - ($store->get_supported_modes($return) & cache_store::MODE_REQUEST) == cache_store::MODE_REQUEST, + ($class::get_supported_modes($return) & cache_store::MODE_REQUEST) == cache_store::MODE_REQUEST, ), 'supports' => array( - 'multipleidentifiers' => $store->supports_multiple_identifiers(), - 'dataguarantee' => $store->supports_data_guarantee(), - 'nativettl' => $store->supports_native_ttl(), + 'multipleidentifiers' => $store ? $store->supports_multiple_identifiers() : false, + 'dataguarantee' => $store ? $store->supports_data_guarantee() : false, + 'nativettl' => $store ? $store->supports_native_ttl() : false, 'nativelocking' => ($store instanceof cache_is_lockable), 'keyawareness' => ($store instanceof cache_is_key_aware), 'searchable' => ($store instanceof cache_is_searchable) ), - 'warnings' => $store->get_warnings() + 'warnings' => $store ? $store->get_warnings() : array() ); if (empty($details['default'])) { $return[$name] = $record; diff --git a/cache/stores/apcu/lib.php b/cache/stores/apcu/lib.php index 27cd2e65371..cbb20db9110 100644 --- a/cache/stores/apcu/lib.php +++ b/cache/stores/apcu/lib.php @@ -369,23 +369,12 @@ class cachestore_apcu extends cache_store implements cache_is_key_aware, cache_i } /** - * Generates an instance of the cache store that can be used for testing. + * Generates the appropriate configuration required for unit testing. * - * @param cache_definition $definition - * @return cachestore_apcu|false + * @return array Array of unit test configuration data to be used by initialise(). */ - public static function initialise_unit_test_instance(cache_definition $definition) { - if (!self::are_requirements_met()) { - return false; - } - - $store = new cachestore_apcu('Test APCu', array('prefix' => 'phpunit')); - if (!$store->is_ready()) { - return false; - } - $store->initialise($definition); - - return $store; + public static function unit_test_configuration() { + return array('prefix' => 'phpunit'); } /** @@ -416,4 +405,16 @@ class cachestore_apcu extends cache_store implements cache_is_key_aware, cache_i } $editform->set_data($data); } + + /** + * Returns true if this cache store instance is both suitable for testing, and ready for testing. + * + * Cache stores that support being used as the default store for unit and acceptance testing should + * override this function and return true if there requirements have been met. + * + * @return bool + */ + public static function ready_to_be_used_for_testing() { + return true; + } } diff --git a/cache/stores/apcu/tests/apcu_test.php b/cache/stores/apcu/tests/apcu_test.php index 36fc1ae8c3e..4903d3bbb3d 100644 --- a/cache/stores/apcu/tests/apcu_test.php +++ b/cache/stores/apcu/tests/apcu_test.php @@ -57,7 +57,8 @@ class cachestore_apcu_test extends cachestore_tests { */ public function test_cross_application_interaction() { $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_apcu', 'phpunit_test'); - $instance = cachestore_apcu::initialise_unit_test_instance($definition); + $instance = new cachestore_apcu('Test', cachestore_apcu::unit_test_configuration()); + $instance->initialise($definition); // Test purge with custom data. $this->assertTrue($instance->set('test', 'monster')); @@ -75,9 +76,12 @@ class cachestore_apcu_test extends cachestore_tests { public function test_different_caches_have_different_prefixes() { $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_apcu', 'phpunit_test'); - $instance = cachestore_apcu::initialise_unit_test_instance($definition); + $instance = new cachestore_apcu('Test', cachestore_apcu::unit_test_configuration()); + $instance->initialise($definition); + $definition2 = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_apcu', 'phpunit_test2'); - $instance2 = cachestore_apcu::initialise_unit_test_instance($definition2); + $instance2 = new cachestore_apcu('Test', cachestore_apcu::unit_test_configuration()); + $instance2->initialise($definition2); $instance->set('test1', 1); $this->assertFalse($instance2->get('test1')); diff --git a/cache/stores/file/lib.php b/cache/stores/file/lib.php index 67a6642a641..91a0839b3d5 100644 --- a/cache/stores/file/lib.php +++ b/cache/stores/file/lib.php @@ -677,6 +677,15 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i return $cache; } + /** + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). + */ + public static function unit_test_configuration() { + return array(); + } + /** * Writes your madness to a file. * diff --git a/cache/stores/memcache/lib.php b/cache/stores/memcache/lib.php index e4ead7fe3bb..6f89d1ace95 100644 --- a/cache/stores/memcache/lib.php +++ b/cache/stores/memcache/lib.php @@ -582,24 +582,16 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { } /** - * Creates a test instance for unit tests if possible. - * @param cache_definition $definition - * @return bool|cachestore_memcache + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). */ - public static function initialise_unit_test_instance(cache_definition $definition) { - if (!self::are_requirements_met()) { - return false; - } + public static function unit_test_configuration() { + // If the configuration is not defined correctly, return only the configuration know about. if (!defined('TEST_CACHESTORE_MEMCACHE_TESTSERVERS')) { - return false; + return []; } - $configuration = array(); - $configuration['servers'] = explode("\n", TEST_CACHESTORE_MEMCACHE_TESTSERVERS); - - $store = new cachestore_memcache('Test memcache', $configuration); - $store->initialise($definition); - - return $store; + return ['servers' => explode("\n", TEST_CACHESTORE_MEMCACHE_TESTSERVERS)]; } /** diff --git a/cache/stores/memcache/tests/memcache_test.php b/cache/stores/memcache/tests/memcache_test.php index 0cba975a949..f88f5fb3ce3 100644 --- a/cache/stores/memcache/tests/memcache_test.php +++ b/cache/stores/memcache/tests/memcache_test.php @@ -57,7 +57,8 @@ class cachestore_memcache_test extends cachestore_tests { $this->resetAfterTest(true); $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_memcache', 'phpunit_test'); - $instance = cachestore_memcache::initialise_unit_test_instance($definition); + $instance = new cachestore_memcache('Memcache Test', cachestore_memcache::unit_test_configuration()); + $instance->initialise($definition); if (!$instance) { // Something prevented memcache store to be inited (extension, TEST_CACHESTORE_MEMCACHE_TESTSERVERS...). $this->markTestSkipped(); diff --git a/cache/stores/memcached/lib.php b/cache/stores/memcached/lib.php index 84ec315fc6c..0c7be7bb9ca 100644 --- a/cache/stores/memcached/lib.php +++ b/cache/stores/memcached/lib.php @@ -739,25 +739,16 @@ class cachestore_memcached extends cache_store implements cache_is_configurable } /** - * Creates a test instance for unit tests if possible. - * @param cache_definition $definition - * @return bool|cachestore_memcached + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). */ - public static function initialise_unit_test_instance(cache_definition $definition) { - if (!self::are_requirements_met()) { - return false; - } + public static function unit_test_configuration() { + // If the configuration is not defined correctly, return only the configuration know about. if (!defined('TEST_CACHESTORE_MEMCACHED_TESTSERVERS')) { - return false; + return []; } - - $configuration = array(); - $configuration['servers'] = explode("\n", TEST_CACHESTORE_MEMCACHED_TESTSERVERS); - - $store = new cachestore_memcached('Test memcached', $configuration); - $store->initialise($definition); - - return $store; + return ['servers' => explode("\n", TEST_CACHESTORE_MEMCACHED_TESTSERVERS)]; } /** diff --git a/cache/stores/memcached/tests/memcached_test.php b/cache/stores/memcached/tests/memcached_test.php index 6a58844bbc3..08ce677314c 100644 --- a/cache/stores/memcached/tests/memcached_test.php +++ b/cache/stores/memcached/tests/memcached_test.php @@ -61,9 +61,11 @@ class cachestore_memcached_test extends cachestore_tests { $this->resetAfterTest(true); $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_memcached', 'phpunit_test'); - $instance = cachestore_memcached::initialise_unit_test_instance($definition); + $instance = new cachestore_memcached('Memcached Test', cachestore_memcached::unit_test_configuration()); + $instance->initialise($definition); - if (!$instance) { // Something prevented memcached store to be inited (extension, TEST_CACHESTORE_MEMCACHED_TESTSERVERS...). + if (!$instance->is_ready()) { + // Something prevented memcached store to be inited (extension, TEST_CACHESTORE_MEMCACHED_TESTSERVERS...). $this->markTestSkipped(); } diff --git a/cache/stores/mongodb/lib.php b/cache/stores/mongodb/lib.php index 641b05165a3..0f1d352d24d 100644 --- a/cache/stores/mongodb/lib.php +++ b/cache/stores/mongodb/lib.php @@ -571,25 +571,16 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { * @param cache_definition $definition * @return false */ - public static function initialise_unit_test_instance(cache_definition $definition) { - if (!self::are_requirements_met()) { - return false; - } - if (!defined('TEST_CACHESTORE_MONGODB_TESTSERVER')) { - return false; - } - + public static function unit_test_configuration() { $configuration = array(); - $configuration['servers'] = explode("\n", TEST_CACHESTORE_MONGODB_TESTSERVER); $configuration['usesafe'] = 1; - $store = new cachestore_mongodb('Test mongodb', $configuration); - if (!$store->is_ready()) { - return false; + // If the configuration is not defined correctly, return only the configuration know about. + if (defined('TEST_CACHESTORE_MONGODB_TESTSERVER')) { + $configuration['servers'] = explode("\n", TEST_CACHESTORE_MONGODB_TESTSERVER); } - $store->initialise($definition); - return $store; + return $configuration; } /** diff --git a/cache/stores/mongodb/tests/mongodb_test.php b/cache/stores/mongodb/tests/mongodb_test.php index 51861753a72..edb7f76ad20 100644 --- a/cache/stores/mongodb/tests/mongodb_test.php +++ b/cache/stores/mongodb/tests/mongodb_test.php @@ -56,9 +56,10 @@ class cachestore_mongodb_test extends cachestore_tests { public function test_collection_name() { // This generates a definition that has a hash starting with a number. MDL-46208. $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_mongodb', 'abc'); - $instance = cachestore_mongodb::initialise_unit_test_instance($definition); + $instance = new cachestore_mongodb('MongoDB_Test', cachestore_mongodb::unit_test_configuration()); + $instance->initialise($definition); - if (!$instance) { + if (!$instance->is_ready()) { $this->markTestSkipped(); } diff --git a/cache/stores/session/lib.php b/cache/stores/session/lib.php index d320b8c1088..f1284f1bdd0 100644 --- a/cache/stores/session/lib.php +++ b/cache/stores/session/lib.php @@ -516,6 +516,14 @@ class cachestore_session extends session_data_store implements cache_is_key_awar return $cache; } + /** + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). + */ + public static function unit_test_configuration() { + return array(); + } /** * Returns the name of this instance. * @return string diff --git a/cache/stores/static/lib.php b/cache/stores/static/lib.php index d1f6f71a298..ecf06ddebb4 100644 --- a/cache/stores/static/lib.php +++ b/cache/stores/static/lib.php @@ -492,6 +492,15 @@ class cachestore_static extends static_data_store implements cache_is_key_aware, return $cache; } + /** + * Generates the appropriate configuration required for unit testing. + * + * @return array Array of unit test configuration data to be used by initialise(). + */ + public static function unit_test_configuration() { + return array(); + } + /** * Returns the name of this instance. * @return string diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 255cbf32c13..eedc37d3112 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -1315,8 +1315,10 @@ class core_cache_testcase extends advanced_testcase { $configfile = $CFG->dataroot.'/muc/config.php'; - // That's right, we're deleting the config file. - $this->assertTrue(@unlink($configfile)); + // The config file will not exist yet as we've not done anything with the cache. + // reset_all_data removes the file and without a call to create a configuration it doesn't exist + // as yet. + $this->assertFileNotExists($configfile); // Disable the cache cache_phpunit_factory::phpunit_disable(); diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index 1bdf7d753c6..c6b405b3e66 100644 --- a/cache/tests/fixtures/lib.php +++ b/cache/tests/fixtures/lib.php @@ -73,13 +73,13 @@ class cache_config_testing extends cache_config_writer { if (class_exists($class) && $class::ready_to_be_used_for_testing()) { /* @var cache_store $class */ $writer->configstores['test_application'] = array( - 'use_test_store' => true, 'name' => 'test_application', 'plugin' => $expectedstore, - 'alt' => $writer->configstores[$defaultapplication], 'modes' => $class::get_supported_modes(), - 'features' => $class::get_supported_features() + 'features' => $class::get_supported_features(), + 'configuration' => $class::unit_test_configuration() ); + $defaultapplication = 'test_application'; } } @@ -535,47 +535,4 @@ class cache_phpunit_factory extends cache_factory { public static function phpunit_disable() { parent::disable(); } - - /** - * @var bool Whether the warning notice about alternative cache store used has been displayed. - */ - protected $altcachestorenotice = false; - - /** - * Creates a store instance given its name and configuration. - * - * If the store has already been instantiated then the original object will be returned. (reused) - * - * @param string $name The name of the store (must be unique remember) - * @param array $details - * @param cache_definition $definition The definition to instantiate it for. - * @return boolean|cache_store - */ - public function create_store_from_config($name, array $details, cache_definition $definition) { - - if (isset($details['use_test_store'])) { - // name, plugin, alt - $class = 'cachestore_'.$details['plugin']; - $method = 'initialise_unit_test_instance'; - if (class_exists($class) && method_exists($class, $method)) { - $instance = $class::$method($definition); - - if ($instance) { - return $instance; - } - } - - // Notify user that alternative store is being used, so action can be taken. - if (!$this->altcachestorenotice) { - echo PHP_EOL . "++ WARNING: " . 'Failed to use "' . $details['plugin'] . '" cache store, alt "' . - $details['alt']['plugin'] . '" cache store is used.' . PHP_EOL . PHP_EOL; - $this->altcachestorenotice = true; - } - $details = $details['alt']; - $details['class'] = 'cachestore_'.$details['plugin']; - $name = $details['name']; - } - - return parent::create_store_from_config($name, $details, $definition); - } } \ No newline at end of file diff --git a/cache/tests/fixtures/stores.php b/cache/tests/fixtures/stores.php index 1dbe75a70eb..56f94e74595 100644 --- a/cache/tests/fixtures/stores.php +++ b/cache/tests/fixtures/stores.php @@ -42,19 +42,29 @@ abstract class cachestore_tests extends advanced_testcase { */ abstract protected function get_class_name(); + /** + * Sets up the fixture, for example, open a network connection. + * This method is called before a test is executed. + */ + public function setUp() { + $class = $this->get_class_name(); + if (!class_exists($class) || !$class::are_requirements_met()) { + $this->markTestSkipped('Could not test '.$class.'. Requirements are not met.'); + } + parent::setUp(); + } /** * Run the unit tests for the store. */ public function test_test_instance() { $class = $this->get_class_name(); - if (!class_exists($class) || !method_exists($class, 'initialise_test_instance') || !$class::are_requirements_met()) { - $this->markTestSkipped('Could not test '.$class.'. Requirements are not met.'); - } $modes = $class::get_supported_modes(); if ($modes & cache_store::MODE_APPLICATION) { $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, $class, 'phpunit_test'); - $instance = $class::initialise_unit_test_instance($definition); + $instance = new $class($class.'_test', $class::unit_test_configuration()); + $instance->initialise($definition); + if (!$instance) { $this->markTestSkipped('Could not test '.$class.'. No test instance configured for application caches.'); } else { @@ -63,7 +73,9 @@ abstract class cachestore_tests extends advanced_testcase { } if ($modes & cache_store::MODE_SESSION) { $definition = cache_definition::load_adhoc(cache_store::MODE_SESSION, $class, 'phpunit_test'); - $instance = $class::initialise_unit_test_instance($definition); + $instance = new $class($class.'_test', $class::unit_test_configuration()); + $instance->initialise($definition); + if (!$instance) { $this->markTestSkipped('Could not test '.$class.'. No test instance configured for session caches.'); } else { @@ -72,7 +84,9 @@ abstract class cachestore_tests extends advanced_testcase { } if ($modes & cache_store::MODE_REQUEST) { $definition = cache_definition::load_adhoc(cache_store::MODE_REQUEST, $class, 'phpunit_test'); - $instance = $class::initialise_unit_test_instance($definition); + $instance = new $class($class.'_test', $class::unit_test_configuration()); + $instance->initialise($definition); + if (!$instance) { $this->markTestSkipped('Could not test '.$class.'. No test instance configured for request caches.'); } else { diff --git a/cache/upgrade.txt b/cache/upgrade.txt index c2649c01c35..e995cfff8eb 100644 --- a/cache/upgrade.txt +++ b/cache/upgrade.txt @@ -13,6 +13,8 @@ Information provided here is intended especially for developers. - cache_store::cleanup() * cachestore_dummy::cleanup() has been deprecated. * cachestore_dummy::instance_deleted() implemented in lieu of cachestore_dummy::cleanup(). +* Added cache_store::unit_test_configuration() to calculate unit testing configuration. +* Remove cache_store:initialise_unit_test_instance() as it is incompatible with cache_helper purge functions. === 3.1 === * Cache stores has a new feature DEREFERENCES_OBJECTS. diff --git a/lib/testing/classes/util.php b/lib/testing/classes/util.php index f3c04a3ed08..4a8b682e133 100644 --- a/lib/testing/classes/util.php +++ b/lib/testing/classes/util.php @@ -814,12 +814,13 @@ abstract class testing_util { make_temp_directory(''); make_cache_directory(''); make_localcache_directory(''); + // Purge all data from the caches. This is required for consistency between tests. + // Any file caches that happened to be within the data root will have already been clearer (because we just deleted cache) + // and now we will purge any other caches as well. This must be done before the cache_factory::reset() as that + // removes all definitions of caches and purge does not have valid caches to operate on. + cache_helper::purge_all(); // Reset the cache API so that it recreates it's required directories as well. cache_factory::reset(); - // Purge all data from the caches. This is required for consistency. - // Any file caches that happened to be within the data root will have already been clearer (because we just deleted cache) - // and now we will purge any other caches as well. - cache_helper::purge_all(); } /**