From 220d1d7a230f2bb97d01f2b965366ebdabd26082 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Tue, 10 Jun 2025 11:12:27 +0200 Subject: [PATCH] MDL-85666 phpunit: add general plugin tests This defines new group which can be used to limit execution of these tests to one plugin only. For example to execute all tests for Label module use: phpunit --testsuite=mod_label_testsuite phpunit --group=plugin_checks --filter=mod_label or phpunit --filter=mod_label --- .../capability/tests/plugin_checks_test.php | 70 +++++++++++ lib/external/tests/plugin_checks_test.php | 53 ++++++++ lib/tests/classes/plugin_checks_testcase.php | 65 ++++++++++ lib/tests/db/plugin_checks_test.php | 58 +++++++++ lib/tests/event/plugin_checks_test.php | 60 +++++++++ lib/tests/task/plugin_checks_test.php | 56 +++++++++ message/tests/plugin_checks_test.php | 58 +++++++++ mod/lesson/classes/event/highscore_added.php | 6 +- .../classes/event/highscores_viewed.php | 6 +- .../tests/privacy/provider_advanced_test.php | 119 ++++++++++++++++++ privacy/tests/privacy/provider_test.php | 87 ++++++------- 11 files changed, 584 insertions(+), 54 deletions(-) create mode 100644 admin/tool/capability/tests/plugin_checks_test.php create mode 100644 lib/external/tests/plugin_checks_test.php create mode 100644 lib/tests/classes/plugin_checks_testcase.php create mode 100644 lib/tests/db/plugin_checks_test.php create mode 100644 lib/tests/event/plugin_checks_test.php create mode 100644 lib/tests/task/plugin_checks_test.php create mode 100644 message/tests/plugin_checks_test.php create mode 100644 privacy/tests/privacy/provider_advanced_test.php diff --git a/admin/tool/capability/tests/plugin_checks_test.php b/admin/tool/capability/tests/plugin_checks_test.php new file mode 100644 index 00000000000..59ffa264010 --- /dev/null +++ b/admin/tool/capability/tests/plugin_checks_test.php @@ -0,0 +1,70 @@ +. + +namespace tool_capability; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in capability definitions of plugins. + * + * @group plugin_checks + * @package tool_capability + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify contents of plugin db/access.php file. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_db_access_file(string $component, string $plugintype, string $pluginname, string $dir): void { + global $CFG; + + $stringmanager = get_string_manager(); + $corerolefile = "$CFG->dirroot/lang/en/role.php"; + $langfile = "$dir/lang/en/$component.php"; + + $file = "$dir/db/access.php"; + $capabilities = $this->fetch_array_from_file($file, 'capabilities'); + if (!$capabilities) { + $this->expectNotToPerformAssertions(); + return; + } + + foreach ($capabilities as $capname => $capability) { + if ($plugintype === 'qbank' && str_starts_with($capname, 'moodle/question:')) { + // Question bank capabilities are irregular. + $strname = explode('/', $capname, 2)[1]; + $this->assertTrue($stringmanager->string_exists($strname, 'core_role'), + "Missing capability name string '$strname' in $corerolefile"); + continue; + } + $this->assertMatchesRegularExpression("|^$plugintype/$pluginname:[a-z0-9_]+$|", $capname); + $strname = substr($capname, strlen($plugintype) + 1); + $this->assertTrue($stringmanager->string_exists($strname, $component), + "Missing capability name string '$strname' in $langfile"); + $this->assertSame($capname, clean_param($capname, PARAM_CAPABILITY)); + } + } +} diff --git a/lib/external/tests/plugin_checks_test.php b/lib/external/tests/plugin_checks_test.php new file mode 100644 index 00000000000..3118a252915 --- /dev/null +++ b/lib/external/tests/plugin_checks_test.php @@ -0,0 +1,53 @@ +. + +namespace core; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in plugin external API. + * + * @group plugin_checks + * @package core + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify plugin external API definition files. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_db_services_file(string $component, string $plugintype, string $pluginname, string $dir): void { + $file = "$dir/db/services.php"; + $functions = $this->fetch_array_from_file($file, 'functions'); + if (!$functions) { + $this->expectNotToPerformAssertions(); + return; + } + + foreach ($functions as $wsname => $definition) { + $this->assertStringStartsWith($component . '_', $wsname); + } + } +} diff --git a/lib/tests/classes/plugin_checks_testcase.php b/lib/tests/classes/plugin_checks_testcase.php new file mode 100644 index 00000000000..1585b03c63c --- /dev/null +++ b/lib/tests/classes/plugin_checks_testcase.php @@ -0,0 +1,65 @@ +. + +namespace core\tests; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Base class for general testing of plugin features and APIs. + * The test must not modify database or any global state. + * + * Following is required to allow filtering of test by Frankenstyle plugin name: + * - all providers used in the tests must use components as keys of provider data + * - all tests must include group "plugin_checks" + * + * @package core + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class plugin_checks_testcase extends \basic_testcase { + /** + * Data provider for testing of all available plugins. + * + * @return array as array of [component, plugintype, pluginname, dir] + */ + public static function all_plugins_provider(): array { + $result = []; + foreach (\core_component::get_plugin_types() as $plugintype => $unused) { + foreach (\core_component::get_plugin_list($plugintype) as $pluginname => $dir) { + $component = $plugintype . '_' . $pluginname; + $result[$component] = [$component, $plugintype, $pluginname, $dir]; + } + } + return $result; + } + + /** + * Include file and return an array variable defined in its global scope. + * This is intended primarily for files inside plugin /db/ subdirectory. + * + * @param string $phpfile + * @param string $variablename + * @return array|null NULL means file does not exist + */ + protected function fetch_array_from_file(string $phpfile, string $variablename): ?array { + if (!file_exists($phpfile)) { + return null; + } + require($phpfile); + return $$variablename; + } +} diff --git a/lib/tests/db/plugin_checks_test.php b/lib/tests/db/plugin_checks_test.php new file mode 100644 index 00000000000..90b2f2ce20f --- /dev/null +++ b/lib/tests/db/plugin_checks_test.php @@ -0,0 +1,58 @@ +. + +namespace core\db; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in plugin database structures. + * + * @group plugin_checks + * @package core + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify plugin db/install.xml file. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_db_install_file(string $component, string $plugintype, string $pluginname, string $dir): void { + global $DB; + $DB->get_manager(); // Preload XMLDB classes. + + $file = "$dir/db/install.xml"; + if (!file_exists($file)) { + $this->expectNotToPerformAssertions(); + return; + } + + $rawcontents = file_get_contents($file); + $xmldb = new \xmldb_file($file); + $xmldb->loadXMLStructure(); + $xmlcontents = $xmldb->getStructure()->xmlOutput(); + $this->assertSame($xmlcontents, $rawcontents, + "Unexpected install.xml format detected, reconciliation needed in $file"); + } +} diff --git a/lib/tests/event/plugin_checks_test.php b/lib/tests/event/plugin_checks_test.php new file mode 100644 index 00000000000..493c1e50ece --- /dev/null +++ b/lib/tests/event/plugin_checks_test.php @@ -0,0 +1,60 @@ +. + +namespace core\event; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in plugin events. + * + * @group plugin_checks + * @package core + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify all plugin events. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_event_classes(string $component, string $plugintype, string $pluginname, string $dir): void { + $events = \core_component::get_component_classes_in_namespace($component, 'event'); + if (!$events) { + $this->expectNotToPerformAssertions(); + return; + } + + foreach ($events as $eventclassname => $unused) { + $rc = new \ReflectionClass($eventclassname); + if ($rc->isAbstract()) { + continue; + } + if (!is_subclass_of($eventclassname, \core\event\base::class)) { + // Most likely an observer in irregular location, ignore for now. + continue; + } + $this->assertIsString($eventclassname::get_name()); + } + } +} diff --git a/lib/tests/task/plugin_checks_test.php b/lib/tests/task/plugin_checks_test.php new file mode 100644 index 00000000000..1969c535c4d --- /dev/null +++ b/lib/tests/task/plugin_checks_test.php @@ -0,0 +1,56 @@ +. + +namespace core\task; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in plugin tasks. + * + * @group plugin_checks + * @package core + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify plugin all plugin tasks. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_db_tasks_file(string $component, string $plugintype, string $pluginname, string $dir): void { + $file = "$dir/db/tasks.php"; + $tasks = $this->fetch_array_from_file($file, 'tasks'); + if (!$tasks) { + $this->expectNotToPerformAssertions(); + return; + } + + foreach ($tasks as $task) { + /** @var class-string<\core\task\task_base> $taskclassname */ + $taskclassname = $task['classname']; + $t = new $taskclassname(); + $this->assertIsString($t->get_name()); + } + } +} diff --git a/message/tests/plugin_checks_test.php b/message/tests/plugin_checks_test.php new file mode 100644 index 00000000000..2c52ab6e9b4 --- /dev/null +++ b/message/tests/plugin_checks_test.php @@ -0,0 +1,58 @@ +. + +namespace core_message; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound + +/** + * Detect common problems in message provider definitions of plugins. + * + * @group plugin_checks + * @package core_message + * @copyright 2025 Petr Skoda + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class plugin_checks_test extends \core\tests\plugin_checks_testcase { + /** + * Verify contents of plugin db/message.php file. + * + * @dataProvider all_plugins_provider + * @coversNothing + * + * @param string $component + * @param string $plugintype + * @param string $pluginname + * @param string $dir + */ + public function test_db_messages_file(string $component, string $plugintype, string $pluginname, string $dir): void { + $stringmanager = get_string_manager(); + $langfile = "$dir/lang/en/$component.php"; + + $file = "$dir/db/messages.php"; + $messageproviders = $this->fetch_array_from_file($file, 'messageproviders'); + if (!$messageproviders) { + $this->expectNotToPerformAssertions(); + return; + } + + foreach ($messageproviders as $providername => $provider) { + $strname = 'messageprovider:' . $providername; + $this->assertTrue($stringmanager->string_exists($strname, $component), + "Missing capability name string '$strname' in $langfile"); + } + } +} diff --git a/mod/lesson/classes/event/highscore_added.php b/mod/lesson/classes/event/highscore_added.php index 25d61bebc94..79835530b3b 100644 --- a/mod/lesson/classes/event/highscore_added.php +++ b/mod/lesson/classes/event/highscore_added.php @@ -27,8 +27,6 @@ namespace mod_lesson\event; defined('MOODLE_INTERNAL') || die(); -debugging('mod_lesson\event\highscore_added has been deprecated. Since the functionality no longer resides in the lesson module.', - DEBUG_DEVELOPER); /** * The mod_lesson highscore added event class. * @@ -51,6 +49,10 @@ class highscore_added extends \core\event\base { * Set basic properties for the event. */ protected function init() { + // phpcs:ignore moodle.Files.LineLength.TooLong + debugging('mod_lesson\event\highscore_added has been deprecated. Since the functionality no longer resides in the lesson module.', + DEBUG_DEVELOPER); + $this->data['objecttable'] = 'lesson_high_scores'; $this->data['crud'] = 'c'; $this->data['edulevel'] = self::LEVEL_PARTICIPATING; diff --git a/mod/lesson/classes/event/highscores_viewed.php b/mod/lesson/classes/event/highscores_viewed.php index 91fa720cd22..7d2648c0345 100644 --- a/mod/lesson/classes/event/highscores_viewed.php +++ b/mod/lesson/classes/event/highscores_viewed.php @@ -27,8 +27,6 @@ namespace mod_lesson\event; defined('MOODLE_INTERNAL') || die(); -debugging('mod_lesson\event\highscores_viewed has been deprecated. Since the functionality no longer resides in the lesson module.', - DEBUG_DEVELOPER); /** * The mod_lesson highscores viewed class. * @@ -43,6 +41,10 @@ class highscores_viewed extends \core\event\base { * Set basic properties for the event. */ protected function init() { + // phpcs:ignore moodle.Files.LineLength.TooLong + debugging('mod_lesson\event\highscores_viewed has been deprecated. Since the functionality no longer resides in the lesson module.', + DEBUG_DEVELOPER); + $this->data['objecttable'] = 'lesson'; $this->data['crud'] = 'r'; $this->data['edulevel'] = self::LEVEL_PARTICIPATING; diff --git a/privacy/tests/privacy/provider_advanced_test.php b/privacy/tests/privacy/provider_advanced_test.php new file mode 100644 index 00000000000..4524ad3af67 --- /dev/null +++ b/privacy/tests/privacy/provider_advanced_test.php @@ -0,0 +1,119 @@ +. + +namespace core_privacy\privacy; + +use core_privacy\manager; + +/** + * Slow unit tests for all Privacy Providers that require database modifications. + * + * @package core_privacy + * @copyright 2018 Andrew Nicols + * @copyright 2025 Petr Skoda + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class provider_advanced_test extends \advanced_testcase { + /** + * Returns a list of frankenstyle names of core components (plugins and subsystems). + * + * @return array the array of frankenstyle component names with the relevant class name. + */ + public static function get_component_list(): array { + $components = ['core' => [ + 'component' => 'core', + 'classname' => manager::get_provider_classname_for_component('core'), + ]]; + // Get all plugins. + $plugintypes = \core_component::get_plugin_types(); + foreach ($plugintypes as $plugintype => $typedir) { + $plugins = \core_component::get_plugin_list($plugintype); + foreach ($plugins as $pluginname => $plugindir) { + $frankenstyle = $plugintype . '_' . $pluginname; + $components[$frankenstyle] = [ + 'component' => $frankenstyle, + 'classname' => manager::get_provider_classname_for_component($frankenstyle), + ]; + + } + } + // Get all subsystems. + foreach (\core_component::get_core_subsystems() as $name => $path) { + if (isset($path)) { + $frankenstyle = 'core_' . $name; + $components[$frankenstyle] = [ + 'component' => $frankenstyle, + 'classname' => manager::get_provider_classname_for_component($frankenstyle), + ]; + } + } + return $components; + } + + /** + * Ensure that providers do not throw an error when processing a deleted user. + * + * @group plugin_checks + * @dataProvider is_user_data_provider + * @coversNothing + * @param string $component + */ + public function test_component_understands_deleted_users($component): void { + $this->resetAfterTest(); + + // Create a user. + $user = $this->getDataGenerator()->create_user(); + + // Delete the user and their context. + delete_user($user); + $usercontext = \context_user::instance($user->id); + $usercontext->delete(); + + $contextlist = manager::component_class_callback($component, \core_privacy\local\request\core_user_data_provider::class, + 'get_contexts_for_userid', [$user->id]); + + $this->assertInstanceOf(\core_privacy\local\request\contextlist::class, $contextlist); + } + + /** + * List of providers which implement the core_user_data_provider. + * + * @return array + */ + public static function is_user_data_provider(): array { + return array_filter(self::get_component_list(), function($component): bool { + return static::component_implements( + $component['classname'], + \core_privacy\local\request\core_user_data_provider::class + ); + }); + } + + /** + * Checks whether the component's provider class implements the specified interface, either directly or as a grandchild. + * + * @param string $providerclass The name of the class to test. + * @param string $interface the name of the interface we want to check. + * @return bool Whether the class implements the interface. + */ + protected static function component_implements($providerclass, $interface) { + if (class_exists($providerclass) && interface_exists($interface)) { + return is_subclass_of($providerclass, $interface); + } + + return false; + } +} diff --git a/privacy/tests/privacy/provider_test.php b/privacy/tests/privacy/provider_test.php index 44de755e5a5..a55c6ee6098 100644 --- a/privacy/tests/privacy/provider_test.php +++ b/privacy/tests/privacy/provider_test.php @@ -23,16 +23,13 @@ */ namespace core_privacy\privacy; -defined('MOODLE_INTERNAL') || die(); - use core_privacy\manager; use core_privacy\local\metadata\collection; use core_privacy\local\metadata\types\type; use core_privacy\local\metadata\types\database_table; -use core_privacy\local\metadata\types\external_location; -use core_privacy\local\metadata\types\plugin_type_link; use core_privacy\local\metadata\types\subsystem_link; -use core_privacy\local\metadata\types\user_preference; + +// phpcs:disable moodle.PHPUnit.TestCaseProvider.dataProviderSyntaxMethodNotFound /** * Unit tests for all Privacy Providers. @@ -40,7 +37,7 @@ use core_privacy\local\metadata\types\user_preference; * @copyright 2018 Andrew Nicols * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -final class provider_test extends \advanced_testcase { +final class provider_test extends \core\tests\plugin_checks_testcase { /** * Returns a list of frankenstyle names of core components (plugins and subsystems). * @@ -49,7 +46,7 @@ final class provider_test extends \advanced_testcase { public static function get_component_list(): array { $components = ['core' => [ 'component' => 'core', - 'classname' => manager::get_provider_classname_for_component('core') + 'classname' => manager::get_provider_classname_for_component('core'), ]]; // Get all plugins. $plugintypes = \core_component::get_plugin_types(); @@ -80,16 +77,17 @@ final class provider_test extends \advanced_testcase { /** * Test that the specified null_provider works as expected. * + * @group plugin_checks * @dataProvider null_provider_provider + * @coversNothing * @param string $component The name of the component. * @param string $classname The name of the class for privacy */ - public function test_null_provider($component, $classname) { + public function test_null_provider($component, $classname): void { $reason = $classname::get_reason(); $this->assertIsString($reason); $this->assertIsString(get_string($reason, $component)); - $this->assertDebuggingNotCalled(); } /** @@ -109,11 +107,13 @@ final class provider_test extends \advanced_testcase { /** * Test that the specified metadata_provider works as expected. * + * @group plugin_checks * @dataProvider metadata_provider_provider + * @coversNothing * @param string $component The name of the component. * @param string $classname The name of the class for privacy */ - public function test_metadata_provider($component, $classname) { + public function test_metadata_provider($component, $classname): void { global $DB; $collection = new collection($component); @@ -150,7 +150,6 @@ final class provider_test extends \advanced_testcase { // Check that the string is also correctly defined. $this->assertIsString(get_string($summary, $component)); - $this->assertDebuggingNotCalled(); } if ($fields = $item->get_privacy_fields()) { @@ -161,7 +160,6 @@ final class provider_test extends \advanced_testcase { // Check that the string is also correctly defined. $this->assertIsString(get_string($identifier, $component)); - $this->assertDebuggingNotCalled(); } } } @@ -170,11 +168,13 @@ final class provider_test extends \advanced_testcase { /** * Test that all providers implement some form of compliant provider. * + * @group plugin_checks * @dataProvider get_component_list + * @coversNothing * @param string $component frankenstyle component name, e.g. 'mod_assign' * @param string $classname the fully qualified provider classname */ - public function test_all_providers_compliant($component, $classname) { + public function test_all_providers_compliant($component, $classname): void { $manager = new manager(); $this->assertTrue($manager->component_is_compliant($component)); } @@ -182,33 +182,12 @@ final class provider_test extends \advanced_testcase { /** * Ensure that providers do not throw an error when processing a deleted user. * + * @group plugin_checks * @dataProvider is_user_data_provider + * @coversNothing * @param string $component */ - public function test_component_understands_deleted_users($component) { - $this->resetAfterTest(); - - // Create a user. - $user = $this->getDataGenerator()->create_user(); - - // Delete the user and their context. - delete_user($user); - $usercontext = \context_user::instance($user->id); - $usercontext->delete(); - - $contextlist = manager::component_class_callback($component, \core_privacy\local\request\core_user_data_provider::class, - 'get_contexts_for_userid', [$user->id]); - - $this->assertInstanceOf(\core_privacy\local\request\contextlist::class, $contextlist); - } - - /** - * Ensure that providers do not throw an error when processing a deleted user. - * - * @dataProvider is_user_data_provider - * @param string $component - */ - public function test_userdata_provider_implements_userlist($component) { + public function test_userdata_provider_implements_userlist($component): void { $classname = manager::get_provider_classname_for_component($component); $this->assertTrue(is_subclass_of($classname, \core_privacy\local\request\core_userlist_provider::class)); } @@ -289,24 +268,32 @@ final class provider_test extends \advanced_testcase { /** * Test that all tables with user fields are covered by metadata providers + * + * @group plugin_checks + * @dataProvider get_component_list + * @coversNothing + * @param string $component frankenstyle component name, e.g. 'mod_assign' + * @param string $classname the fully qualified provider classname */ - public function test_table_coverage() { + public function test_table_coverage(string $component, string $classname): void { global $DB; - $dbman = $DB->get_manager(); + $dbman = $DB->get_manager(); // Load DDL classes. $tables = []; - foreach ($dbman->get_install_xml_files() as $filename) { - $xmldbfile = new \xmldb_file($filename); - if (!$xmldbfile->loadXMLStructure()) { - continue; - } - $structure = $xmldbfile->getStructure(); - $tablelist = $structure->getTables(); + $filename = \core_component::get_component_directory($component) . '/db/install.xml'; + if (!file_exists($filename)) { + $this->expectNotToPerformAssertions(); + return; + } + $xmldbfile = new \xmldb_file($filename); + $this->assertTrue($xmldbfile->loadXMLStructure());; - foreach ($tablelist as $table) { - if ($fields = $this->get_userid_fields($table)) { - $tables[$table->getName()] = ' - ' . $table->getName() . ' (' . join(', ', $fields) . ')'; - } + $structure = $xmldbfile->getStructure(); + $tablelist = $structure->getTables(); + + foreach ($tablelist as $table) { + if ($fields = $this->get_userid_fields($table)) { + $tables[$table->getName()] = ' - ' . $table->getName() . ' (' . join(', ', $fields) . ')'; } }