From 7d77854fd6655e2d6fb6e5b7e81757a019886630 Mon Sep 17 00:00:00 2001 From: Martin Gauk Date: Thu, 20 Jun 2024 17:24:33 +0200 Subject: [PATCH 01/80] MDL-80345 core_lock: deal with hash collisions in postgres_lock_factory --- lib/classes/lock/postgres_lock_factory.php | 63 +++++++++++--- lib/tests/lock/postgres_lock_factory_test.php | 87 +++++++++++++++++++ 2 files changed, 136 insertions(+), 14 deletions(-) create mode 100644 lib/tests/lock/postgres_lock_factory_test.php diff --git a/lib/classes/lock/postgres_lock_factory.php b/lib/classes/lock/postgres_lock_factory.php index a4deb15cf4425..124d9fe8bf264 100644 --- a/lib/classes/lock/postgres_lock_factory.php +++ b/lib/classes/lock/postgres_lock_factory.php @@ -34,7 +34,9 @@ * 2 different forms of lock functions, some accepting a single int, and some accepting 2 ints. This implementation * uses the 2 int version so that it uses a separate namespace from the session locking. The second note, * is because postgres uses integer keys for locks, we first need to map strings to a unique integer. This is done - * using a prefix of a sha1 hash converted to an integer. + * using a prefix of a sha1 hash converted to an integer. There is a realistic chance of collisions by using this + * prefix when locking multiple resources at the same time (multiple resource identifiers leading to the + * same token/prefix). We need to deal with that. * * @package core * @category lock @@ -55,8 +57,11 @@ class postgres_lock_factory implements lock_factory { /** @var string $type Used to prefix lock keys */ protected $type; - /** @var array $openlocks - List of held locks - used by auto-release */ - protected $openlocks = array(); + /** @var int[] $resourcetokens Mapping of held locks (resource identifier => internal token) */ + protected $resourcetokens = []; + + /** @var int[] $locksbytoken Mapping of held locks (db connection => internal token => number of locks held) */ + static protected $locksbytoken = []; /** * Calculate a unique instance id based on the database name and prefix. @@ -153,17 +158,26 @@ protected function get_index_from_key($key) { * @param string $resource - The identifier for the lock. Should use frankenstyle prefix. * @param int $timeout - The number of seconds to wait for a lock before giving up. * @param int $maxlifetime - Unused by this lock type. - * @return boolean - true if a lock was obtained. + * @return \core\lock\lock|boolean - An instance of \core\lock\lock if the lock was obtained, or false. */ public function get_lock($resource, $timeout, $maxlifetime = 86400) { + $dbid = spl_object_id($this->db); $giveuptime = time() + $timeout; + $resourcekey = $this->type . '_' . $resource; + $token = $this->get_index_from_key($resourcekey); - $token = $this->get_index_from_key($this->type . '_' . $resource); - - if (isset($this->openlocks[$token])) { + if (isset($this->resourcetokens[$resourcekey])) { return false; } + if (isset(self::$locksbytoken[$dbid][$token])) { + // There is a hash collision, another resource identifier leads to the same token. + // As we already hold an advisory lock for this token, we just increase the counter. + self::$locksbytoken[$dbid][$token]++; + $this->resourcetokens[$resourcekey] = $token; + return new lock($resourcekey, $this); + } + $params = [ 'locktype' => $this->dblockid, 'token' => $token @@ -181,8 +195,9 @@ public function get_lock($resource, $timeout, $maxlifetime = 86400) { } while (!$locked && time() < $giveuptime); if ($locked) { - $this->openlocks[$token] = 1; - return new lock($token, $this); + self::$locksbytoken[$dbid][$token] = 1; + $this->resourcetokens[$resourcekey] = $token; + return new lock($resourcekey, $this); } return false; } @@ -193,12 +208,32 @@ public function get_lock($resource, $timeout, $maxlifetime = 86400) { * @return boolean - true if the lock is no longer held (including if it was never held). */ public function release_lock(lock $lock) { - $params = array('locktype' => $this->dblockid, - 'token' => $lock->get_key()); + $dbid = spl_object_id($this->db); + $resourcekey = $lock->get_key(); + + if (isset($this->resourcetokens[$resourcekey])) { + $token = $this->resourcetokens[$resourcekey]; + } else { + return true; + } + + if (self::$locksbytoken[$dbid][$token] > 1) { + // There are multiple resource identifiers that lead to the same token. + // We will unlock the token later when the last resource is released. + unset($this->resourcetokens[$resourcekey]); + self::$locksbytoken[$dbid][$token]--; + return true; + } + + $params = [ + 'locktype' => $this->dblockid, + 'token' => $token, + ]; $result = $this->db->get_record_sql('SELECT pg_advisory_unlock(:locktype, :token) AS unlocked', $params); $result = $result->unlocked === 't'; if ($result) { - unset($this->openlocks[$lock->get_key()]); + unset($this->resourcetokens[$resourcekey]); + unset(self::$locksbytoken[$dbid][$token]); } return $result; } @@ -224,8 +259,8 @@ public function extend_lock(lock $lock, $maxlifetime = 86400) { */ public function auto_release() { // Called from the shutdown handler. Must release all open locks. - foreach ($this->openlocks as $key => $unused) { - $lock = new lock($key, $this); + foreach ($this->resourcetokens as $resourcekey => $unused) { + $lock = new lock($resourcekey, $this); $lock->release(); } } diff --git a/lib/tests/lock/postgres_lock_factory_test.php b/lib/tests/lock/postgres_lock_factory_test.php new file mode 100644 index 0000000000000..3bc37a8a32c55 --- /dev/null +++ b/lib/tests/lock/postgres_lock_factory_test.php @@ -0,0 +1,87 @@ +. + +namespace core\lock; + +/** + * Unit tests for the postgres lock factory. + * + * @covers \core\lock\postgres_lock_factory + * @package core + * @copyright 2024 Martin Gauk + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class postgres_lock_factory_test extends \advanced_testcase { + /** + * Set up. + */ + public function setUp(): void { + global $DB; + parent::setUp(); + // Skip tests if not using Postgres. + if (!($DB instanceof \pgsql_native_moodle_database)) { + $this->markTestSkipped('Postgres-only test'); + } + } + + /** + * Test locking resources that lead to the same token computed by postgres_lock_factory::get_index_from_key. + * + * It is known that get_index_from_key computes the same token for the strings 'cron_core_cron' and + * 'cron_adhoc_82795762'. + */ + public function test_resources_with_same_hash(): void { + $cronlockfactory = new postgres_lock_factory('cron'); + $lock1 = $cronlockfactory->get_lock('core_cron', 0); + $this->assertNotFalse($lock1); + $lock2 = $cronlockfactory->get_lock('adhoc_82795762', 0); + $this->assertNotFalse($lock2); + + $lock2->release(); + $lock1->release(); + } + + /** + * Test auto_release() method. + * + * Check that all locks were released after calling auto_release(). + */ + public function test_auto_release(): void { + $cronlockfactory = new postgres_lock_factory('test'); + $lock1 = $cronlockfactory->get_lock('1', 0); + $this->assertNotFalse($lock1); + $lock2 = $cronlockfactory->get_lock('2', 0); + $this->assertNotFalse($lock2); + + // Trying to get the lock again should fail as the lock is already held. + $this->assertFalse($cronlockfactory->get_lock('1', 0)); + + $cronlockfactory->auto_release(); + + // We can now lock the resources again. + $lock1again = $cronlockfactory->get_lock('1', 0); + $this->assertNotFalse($lock1again); + $lock2again = $cronlockfactory->get_lock('2', 0); + $this->assertNotFalse($lock2again); + + $lock1again->release(); + $lock2again->release(); + + // Need to explicitly release the locks (although they were already released) to avoid the debug message. + $lock1->release(); + $lock2->release(); + } +} From 3e81ebecb642a0236972ed0e1521fde23020ab93 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 7 Feb 2024 21:04:57 +0800 Subject: [PATCH 02/80] MDL-66903 testing: Reset CFG and component after test This change moves the reset of global test state to the finally section rather than doing it only if the test passes. Previously if a test which modifies the `core_component` internals failed, it would not reset the internal state and impact subsequent tests. --- lib/phpunit/classes/advanced_testcase.php | 27 +++++++++++++---------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index c6b713293350b..b07083698b5a3 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -64,7 +64,7 @@ final public function __construct($name = null, array $data = array(), $dataName * @return void */ final public function runBare(): void { - global $DB; + global $CFG, $DB; if (phpunit_util::$lastdbwrites != $DB->perf_get_writes()) { // this happens when previous test does not reset, we can not use transactions @@ -78,21 +78,17 @@ final public function runBare(): void { try { $this->setCurrentTimeStart(); parent::runBare(); - // set DB reference in case somebody mocked it in test - $DB = phpunit_util::get_global_backup('DB'); - - // Deal with any debugging messages. - $debugerror = phpunit_util::display_debugging_messages(true); - $this->resetDebugging(); - if (!empty($debugerror)) { - trigger_error('Unexpected debugging() call detected.'."\n".$debugerror, E_USER_NOTICE); - } - } catch (Exception $ex) { $e = $ex; } catch (Throwable $ex) { // Engine errors in PHP7 throw exceptions of type Throwable (this "catch" will be ignored in PHP5). $e = $ex; + } finally { + // Reset global state after test and test failure. + $CFG = phpunit_util::get_global_backup('CFG'); + $DB = phpunit_util::get_global_backup('DB'); + // This is _hacky_. We need to reset the autoloader, and this is the only way to do so right now. + (new ReflectionProperty(\core_component::class, 'plugintypes'))->setValue(null, null); } if (isset($e)) { @@ -101,7 +97,14 @@ final public function runBare(): void { throw $e; } - if (!$this->testdbtransaction or $this->testdbtransaction->is_disposed()) { + // Deal with any debugging messages. + $debugerror = phpunit_util::display_debugging_messages(true); + $this->resetDebugging(); + if (!empty($debugerror)) { + trigger_error('Unexpected debugging() call detected.' . "\n" . $debugerror, E_USER_NOTICE); + } + + if (!$this->testdbtransaction || $this->testdbtransaction->is_disposed()) { $this->testdbtransaction = null; } From e6966b7ae786312a28f61cc8996982416474f57c Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 7 Feb 2024 21:07:05 +0800 Subject: [PATCH 03/80] MDL-66903 testing: Fix whitespace --- lib/tests/component_test.php | 96 ++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index b7154fcb0f275..65c5d0def1653 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -645,54 +645,54 @@ public function classloader_provider() { 'overlap' => 'lib/tests/fixtures/component/overlap' ]; return [ - 'PSR-0 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_main', - 'includedfiles' => "{$directory}psr0/main.php", - ], - 'PSR-0 Classloading - Sub namespace - underscores' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_example', - 'includedfiles' => "{$directory}psr0/subnamespace/example.php", - ], - 'PSR-0 Classloading - Sub namespace - slashes' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0\\subnamespace\\slashes', - 'includedfiles' => "{$directory}psr0/subnamespace/slashes.php", - ], - 'PSR-4 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\main', - 'includedfiles' => "{$directory}psr4/main.php", - ], - 'PSR-4 Classloading - Sub namespace' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\example', - 'includedfiles' => "{$directory}psr4/subnamespace/example.php", - ], - 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\underscore_example', - 'includedfiles' => "{$directory}psr4/subnamespace/underscore_example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap\\subnamespace\\example', - 'includedfiles' => "{$directory}overlap/subnamespace/example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap_subnamespace_example2', - 'includedfiles' => "{$directory}overlap/subnamespace/example2.php", - ], + 'PSR-0 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_main', + 'includedfiles' => "{$directory}psr0/main.php", + ], + 'PSR-0 Classloading - Sub namespace - underscores' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_example', + 'includedfiles' => "{$directory}psr0/subnamespace/example.php", + ], + 'PSR-0 Classloading - Sub namespace - slashes' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0\\subnamespace\\slashes', + 'includedfiles' => "{$directory}psr0/subnamespace/slashes.php", + ], + 'PSR-4 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\main', + 'includedfiles' => "{$directory}psr4/main.php", + ], + 'PSR-4 Classloading - Sub namespace' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\example', + 'includedfiles' => "{$directory}psr4/subnamespace/example.php", + ], + 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\underscore_example', + 'includedfiles' => "{$directory}psr4/subnamespace/underscore_example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap\\subnamespace\\example', + 'includedfiles' => "{$directory}overlap/subnamespace/example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap_subnamespace_example2', + 'includedfiles' => "{$directory}overlap/subnamespace/example2.php", + ], ]; } From 7d7cfee0b7ded8a363fdcc52d8526f66681c08ce Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 21 May 2024 10:59:35 +0100 Subject: [PATCH 04/80] MDL-81803 tool_dataprivacy: observe disabled expiration time. A non-positive integer value should indicate "No time limit". --- .../tool/dataprivacy/classes/data_request.php | 11 +++-- .../tests/expired_data_requests_test.php | 49 ++++++++++++++++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index 02edc7c64e557..0a62e6cab87a6 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -155,9 +155,9 @@ public static function is_expired(data_request $request) { case api::DATAREQUEST_STATUS_EXPIRED: $result = true; break; - // Complete requests are expired if the expiry time has elapsed. + // Complete requests are expired if the expiry time is a positive value, and has elapsed. case api::DATAREQUEST_STATUS_DOWNLOAD_READY: - $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry'); + $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry'); if ($expiryseconds > 0 && time() >= ($request->get('timemodified') + $expiryseconds)) { $result = true; } @@ -178,7 +178,12 @@ public static function is_expired(data_request $request) { public static function get_expired_requests($userid = 0) { global $DB; - $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry'); + // Complete requests are expired if the expiry time is a positive value, and has elapsed. + $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry'); + if ($expiryseconds <= 0) { + return []; + } + $expirytime = strtotime("-{$expiryseconds} second"); $table = self::TABLE; $sqlwhere = 'type = :export_type AND status = :completestatus AND timemodified <= :expirytime'; diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php index 4fd6aebdd812a..cfb2583605053 100644 --- a/admin/tool/dataprivacy/tests/expired_data_requests_test.php +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -26,10 +26,11 @@ * Expired data requests tests. * * @package tool_dataprivacy + * @covers \tool_dataprivacy\data_request * @copyright 2018 Michael Hawkins * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class expired_data_requests_test extends data_privacy_testcase { +final class expired_data_requests_test extends data_privacy_testcase { /** * Test tearDown. @@ -107,6 +108,52 @@ public function test_data_request_expiry() { $this->assertEquals(0, $DB->count_records('files', $fileconditions)); } + /** + * Test that data requests are not expired when expiration is disabled (set to zero) + */ + public function test_data_request_expiry_never(): void { + global $DB; + + $this->resetAfterTest(); + + \core_privacy\local\request\writer::setup_real_writer_instance(); + + // Disable request expiry. + set_config('privacyrequestexpiry', 0, 'tool_dataprivacy'); + + // Create and approve data request. + $user = $this->getDataGenerator()->create_user(); + $usercontext = \context_user::instance($user->id); + + $this->setUser($user->id); + $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT); + $requestid = $datarequest->get('id'); + + $this->setAdminUser(); + api::approve_data_request($requestid); + + ob_start(); + $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task'); + ob_end_clean(); + + // Run expiry deletion - should not affect test export. + $expiredrequests = data_request::get_expired_requests(); + $this->assertEmpty($expiredrequests); + data_request::expire($expiredrequests); + + // Confirm approved and exported. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status')); + $fileconditions = [ + 'userid' => $user->id, + 'component' => 'tool_dataprivacy', + 'filearea' => 'export', + 'itemid' => $requestid, + 'contextid' => $usercontext->id, + ]; + $this->assertEquals(2, $DB->count_records('files', $fileconditions)); + } + /** * Test for \tool_dataprivacy\data_request::is_expired() * Tests for the expected request status to protect from false positive/negative, From 7b0946129ce36c6481fa22edb3f929d7897b95ad Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 19 Dec 2023 22:59:15 +0800 Subject: [PATCH 05/80] MDL-66903 testing: Add support for a \tests\ namespace during tests This commit: - introduces a \tests\ sub-namespace for use in unit tests only - the path to this the tests/classes directory of the owning parent - files here are excluded from unit test runs This is agreed per policy in MDL-80855. --- lib/classes/component.php | 32 ++++++ lib/phpunit/classes/util.php | 2 + lib/tests/component_test.php | 186 +++++++++++++++++++++++------------ lib/upgrade.txt | 7 +- phpunit.xml.dist | 68 +++++++++++++ 5 files changed, 232 insertions(+), 63 deletions(-) diff --git a/lib/classes/component.php b/lib/classes/component.php index 194acf0404dc4..06758ce36705d 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -154,6 +154,38 @@ class_alias($newclassname, $classname); require($file); return; } + + if (PHPUNIT_TEST) { + // For unit tests we support classes in `\frankenstyle_component\tests\` to be loaded from + // `path/to/frankenstyle/component/tests/classes` directory. + // Note: We do *not* support the legacy `\frankenstyle_component_tests_style_classnames`. + if ($component = self::get_component_from_classname($classname)) { + $pathoptions = [ + '/tests/classes' => "{$component}\\tests\\", + '/tests/behat' => "{$component}\\behat\\", + ]; + foreach ($pathoptions as $path => $testnamespace) { + if (preg_match("#^" . preg_quote($testnamespace) . "#", $classname)) { + $path = self::get_component_directory($component) . $path; + $relativeclassname = str_replace( + $testnamespace, + '', + $classname, + ); + $file = sprintf( + "%s/%s.php", + $path, + str_replace('\\', '/', $relativeclassname), + ); + if (!empty($file) && file_exists($file)) { + require($file); + return; + } + break; + } + } + } + } } /** diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index ea6aee02ad6fb..934179b68ce9a 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -505,6 +505,7 @@ public static function build_config_file() { $template = << @dir@ + @dir@/classes EOF; @@ -597,6 +598,7 @@ public static function build_component_config_files() { . + ./classes EOT; diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index 65c5d0def1653..b73b4b311329f 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -40,13 +40,17 @@ public function setUp(): void { $this->oldpsr4namespaces = $psr4namespaces->getValue(null); } public function tearDown(): void { - $psr0namespaces = new ReflectionProperty('core_component', 'psr0namespaces'); + $psr0namespaces = new ReflectionProperty(\core_component::class, 'psr0namespaces'); $psr0namespaces->setAccessible(true); $psr0namespaces->setValue(null, $this->oldpsr0namespaces); - $psr4namespaces = new ReflectionProperty('core_component', 'psr4namespaces'); + $psr4namespaces = new ReflectionProperty(\core_component::class, 'psr4namespaces'); $psr4namespaces->setAccessible(true); $psr4namespaces->setValue(null, $this->oldpsr4namespaces); + + $plugintypes = new ReflectionProperty(\core_component::class, 'plugintypes'); + $plugintypes->setAccessible(true); + $plugintypes->setValue(null, null); } public function test_get_core_subsystems() { @@ -744,69 +748,127 @@ public function psr_classloader_provider() { 'overlap' => 'lib/tests/fixtures/component/overlap' ]; return [ - 'PSR-0 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_main', - 'file' => "{$directory}psr0/main.php", - ], - 'PSR-0 Classloading - Sub namespace - underscores' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_example', - 'file' => "{$directory}psr0/subnamespace/example.php", - ], - 'PSR-0 Classloading - Sub namespace - slashes' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0\\subnamespace\\slashes', - 'file' => "{$directory}psr0/subnamespace/slashes.php", - ], - 'PSR-0 Classloading - non-existant file' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_nonexistant_file', - 'file' => false, - ], - 'PSR-4 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\main', - 'file' => "{$directory}psr4/main.php", - ], - 'PSR-4 Classloading - Sub namespace' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\example', - 'file' => "{$directory}psr4/subnamespace/example.php", - ], - 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\underscore_example', - 'file' => "{$directory}psr4/subnamespace/underscore_example.php", - ], - 'PSR-4 Classloading - non-existant file' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\nonexistant', - 'file' => false, - ], - 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap\\subnamespace\\example', - 'file' => "{$directory}overlap/subnamespace/example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap_subnamespace_example2', - 'file' => "{$directory}overlap/subnamespace/example2.php", - ], + 'PSR-0 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_main', + 'file' => "{$directory}psr0/main.php", + ], + 'PSR-0 Classloading - Sub namespace - underscores' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_example', + 'file' => "{$directory}psr0/subnamespace/example.php", + ], + 'PSR-0 Classloading - Sub namespace - slashes' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0\\subnamespace\\slashes', + 'file' => "{$directory}psr0/subnamespace/slashes.php", + ], + 'PSR-0 Classloading - non-existant file' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_nonexistant_file', + 'file' => false, + ], + 'PSR-4 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\main', + 'file' => "{$directory}psr4/main.php", + ], + 'PSR-4 Classloading - Sub namespace' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\example', + 'file' => "{$directory}psr4/subnamespace/example.php", + ], + 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\underscore_example', + 'file' => "{$directory}psr4/subnamespace/underscore_example.php", + ], + 'PSR-4 Classloading - non-existant file' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\nonexistant', + 'file' => false, + ], + 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap\\subnamespace\\example', + 'file' => "{$directory}overlap/subnamespace/example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap_subnamespace_example2', + 'file' => "{$directory}overlap/subnamespace/example2.php", + ], ]; } + /** + * Test that the classloader can load from the test namespaces. + */ + public function test_classloader_tests_namespace(): void { + global $CFG; + + $this->resetAfterTest(); + + $getclassfilecontent = function (string $classname, ?string $namespace): string { + if ($namespace) { + $content = " [ + 'classes' => [ + 'example.php' => $getclassfilecontent('example', 'core'), + ], + 'tests' => [ + 'classes' => [ + 'example_classname.php' => $getclassfilecontent('example_classname', \core\tests::class), + ], + 'behat' => [ + 'example_classname.php' => $getclassfilecontent('example_classname', \core\behat::class), + ], + ], + ], + ]); + + // Note: This is pretty hacky, but it's the only way to test the classloader. + // We have to override the dirroot and libdir, and then reset the plugintypes property. + $CFG->dirroot = $vfileroot->url(); + $CFG->libdir = $vfileroot->url() . '/lib'; + (new ReflectionProperty('core_component', 'plugintypes'))->setValue(null, null); + + // Existing classes do not break. + $this->assertTrue( + class_exists(\core\example::class), + ); + + // Test and behat classes work. + $this->assertTrue( + class_exists(\core\tests\example_classname::class), + ); + $this->assertTrue( + class_exists(\core\behat\example_classname::class), + ); + + // Non-existent classes do not do anything. + $this->assertFalse( + class_exists(\core\tests\example_classname_not_found::class), + ); + } + /** * Test the PSR classloader. * diff --git a/lib/upgrade.txt b/lib/upgrade.txt index a0126c455feb7..85cb42d122b53 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -1,7 +1,12 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. -=== 4.1.9 +=== 4.1.12 === + +* Added the ability for unit tests to autoload classes in the `\[component]\tests\` namespace from the `[path/to/component]/tests/classes` directory. + +=== 4.1.9 === + * New Behat `heading` named selector to more easily assert the presence of H1-H6 elements on the page === 4.1.8 === diff --git a/phpunit.xml.dist b/phpunit.xml.dist index dfa0b7060ae42..7b45f80dd25bf 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -35,51 +35,70 @@ lib/phpunit/tests + lib/phpunit/tests/classes lib/testing/tests + lib/testing/tests/classes lib/ddl/tests + lib/ddl/tests/classes lib/dml/tests + lib/dml/tests/classes lib/tests + lib/tests/classes favourites/tests + favourites/tests/classes lib/form/tests + lib/form/tests/classes lib/filestorage/tests lib/filebrowser/tests files/tests + lib/filestorage/tests/classes + lib/filebrowser/tests/classes + files/tests/classes filter/tests + filter/tests/classes admin/roles/tests + admin/roles/tests/classes cohort/tests + cohort/tests/classes lib/grade/tests grade/tests grade/grading/tests grade/import/csv/tests + lib/grade/tests/classes + grade/tests/classes + grade/grading/tests/classes + grade/import/csv/tests/classes analytics/tests + analytics/tests/classes availability/tests + availability/tests/classes backup/controller/tests @@ -87,135 +106,184 @@ backup/moodle2/tests backup/tests backup/util + backup/controller/tests/classes + backup/converter/moodle1/tests/classes + backup/moodle2/tests/classes + backup/tests/classes + backup/util/classes badges/tests + badges/tests/classes blog/tests + blog/tests/classes customfield/tests + customfield/tests/classes iplookup/tests + iplookup/tests/classes course/tests + course/tests/classes course/format/tests + course/format/tests/classes privacy/tests + privacy/tests/classes question/engine/tests question/tests question/type/tests question/engine/upgrade/tests + question/engine/tests/classes + question/tests/classes + question/type/tests/classes + question/engine/upgrade/tests/classes cache/tests + cache/tests/classes calendar/tests + calendar/tests/classes enrol/tests + enrol/tests/classes group/tests + group/tests/classes lib/external/tests message/tests + message/tests/classes notes/tests + notes/tests/classes tag/tests + tag/tests/classes rating/tests + rating/tests/classes repository/tests + repository/tests/classes lib/userkey/tests + lib/userkey/tests/classes user/tests + user/tests/classes webservice/tests + webservice/tests/classes mnet/tests + mnet/tests/classes completion/tests + completion/tests/classes comment/tests + comment/tests/classes search/tests + search/tests/classes competency/tests + competency/tests/classes my/tests + my/tests/classes auth/tests + auth/tests/classes blocks/tests + blocks/tests/classes login/tests + login/tests/classes plagiarism/tests + plagiarism/tests/classes portfolio/tests + portfolio/tests/classes lib/editor/tests + lib/editor/tests/classes rss/tests + rss/tests/classes lib/table/tests + lib/table/tests/classes h5p/tests + h5p/tests/classes lib/xapi/tests + lib/xapi/tests/classes contentbank/tests + contentbank/tests/classes payment/tests + payment/tests/classes reportbuilder/tests + reportbuilder/tests/classes admin/presets/tests + admin/presets/tests/classes admin/tests + admin/tests/classes