From d7e1f2cef703608018c0d95a6e49555a368b91a8 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 8 Apr 2024 08:54:08 +0800 Subject: [PATCH] MDL-81456 core: Run tests modifying core_component in a separate process --- lib/phpunit/classes/advanced_testcase.php | 62 +++++++++++++++++++++++ lib/tests/hook/manager_test.php | 51 +++---------------- 2 files changed, 69 insertions(+), 44 deletions(-) diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index 598a37b82b7..6e4716ac78f 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -768,4 +768,66 @@ abstract class advanced_testcase extends base_testcase { return $clock; } + + /** + * Add a mocked plugintype to Moodle. + * + * A new plugintype name must be provided with a path to the plugintype's root. + * + * Please note that tests calling this method must be run in separate isolation mode. + * Please avoid using this if at all possible. + * + * @param string $plugintype The name of the plugintype + * @param string $path The path to the plugintype's root + */ + protected function add_mocked_plugintype( + string $plugintype, + string $path, + ): void { + require_phpunit_isolation(); + + $mockedcomponent = new \ReflectionClass(\core_component::class); + $plugintypes = $mockedcomponent->getStaticPropertyValue('plugintypes'); + + if (array_key_exists($plugintype, $plugintypes)) { + throw new \coding_exception("The plugintype '{$plugintype}' already exists."); + } + + $plugintypes[$plugintype] = $path; + $mockedcomponent->setStaticPropertyValue('plugintypes', $plugintypes); + + $this->resetDebugging(); + } + + /** + * Add a mocked plugin to Moodle. + * + * A new plugin name must be provided with a path to the plugin's root. + * The plugin type must already exist (or have been mocked separately). + * + * Please note that tests calling this method must be run in separate isolation mode. + * Please avoid using this if at all possible. + * + * @param string $plugintype The name of the plugintype + * @param string $pluginname The name of the plugin + * @param string $path The path to the plugin's root + */ + protected function add_mocked_plugin( + string $plugintype, + string $pluginname, + string $path, + ): void { + require_phpunit_isolation(); + + $mockedcomponent = new \ReflectionClass(\core_component::class); + $plugins = $mockedcomponent->getStaticPropertyValue('plugins'); + + if (!array_key_exists($plugintype, $plugins)) { + $plugins[$plugintype] = []; + } + + $plugins[$plugintype][$pluginname] = $path; + $mockedcomponent->setStaticPropertyValue('plugins', $plugins); + $this->resetDebugging(); + } } diff --git a/lib/tests/hook/manager_test.php b/lib/tests/hook/manager_test.php index 574926ed51c..577cc163407 100644 --- a/lib/tests/hook/manager_test.php +++ b/lib/tests/hook/manager_test.php @@ -303,42 +303,13 @@ final class manager_test extends \advanced_testcase { /** * Register a fake plugin called hooktest in the component manager. * - * @return void + * Tests consuming this helpers must run in a separate process. */ protected function setup_hooktest_plugin(): void { global $CFG; - $mockedcomponent = new \ReflectionClass(\core_component::class); - $mockedplugintypes = $mockedcomponent->getProperty('plugintypes'); - $mockedplugintypes->setAccessible(true); - $plugintypes = $mockedplugintypes->getValue(); - $plugintypes['fake'] = "{$CFG->dirroot}/lib/tests/fixtures/fakeplugins"; - $mockedplugintypes->setValue(null, $plugintypes); - $mockedplugins = $mockedcomponent->getProperty('plugins'); - $mockedplugins->setAccessible(true); - $plugins = $mockedplugins->getValue(); - $plugins['fake'] = ['hooktest' => "{$CFG->dirroot}/lib/tests/fixtures/fakeplugins/hooktest"]; - $mockedplugins->setValue(null, $plugins); - $this->resetDebugging(); - } - - /** - * Remove the fake plugin to avoid interference with other tests. - * - * @return void - */ - protected function remove_hooktest_plugin(): void { - $mockedcomponent = new \ReflectionClass(\core_component::class); - $mockedplugintypes = $mockedcomponent->getProperty('plugintypes'); - $mockedplugintypes->setAccessible(true); - $plugintypes = $mockedplugintypes->getValue(); - unset($plugintypes['fake']); - $mockedplugintypes->setValue(null, $plugintypes); - $mockedplugins = $mockedcomponent->getProperty('plugins'); - $mockedplugins->setAccessible(true); - $plugins = $mockedplugins->getValue(); - unset($plugins['fake']); - $mockedplugins->setValue(null, $plugins); + $this->add_mocked_plugintype('fake', "{$CFG->dirroot}/lib/tests/fixtures/fakeplugins"); + $this->add_mocked_plugin('fake', 'hooktest', "{$CFG->dirroot}/lib/tests/fixtures/fakeplugins/hooktest"); } /** @@ -348,8 +319,7 @@ final class manager_test extends \advanced_testcase { * * @covers ::get_hooks_deprecating_plugin_callback() * @covers ::is_deprecating_hook_present() - * @return void - * @throws \coding_exception + * @runInSeparateProcess */ public function test_migrated_callback(): void { $this->resetAfterTest(true); @@ -381,7 +351,6 @@ final class manager_test extends \advanced_testcase { 'Callback old_callback in fake_hooktest component should be migrated to new hook '. 'callback for fake_hooktest\hook\hook_replacing_callback' ); - $this->remove_hooktest_plugin(); } /** @@ -391,8 +360,7 @@ final class manager_test extends \advanced_testcase { * * @covers ::get_hooks_deprecating_plugin_callback() * @covers ::is_deprecating_hook_present() - * @return void - * @throws \coding_exception + * @runInSeparateProcess */ public function test_migrated_callback_with_replacement(): void { $this->resetAfterTest(true); @@ -417,7 +385,6 @@ final class manager_test extends \advanced_testcase { // Confirm the deprecated callback is not called, as expected. $this->assertNull(component_callback('fake_hooktest', 'old_callback', [], null, true)); $this->assertDebuggingNotCalled(); - $this->remove_hooktest_plugin(); } /** @@ -427,8 +394,7 @@ final class manager_test extends \advanced_testcase { * * @covers ::get_hooks_deprecating_plugin_callback() * @covers ::is_deprecating_hook_present() - * @return void - * @throws \coding_exception + * @runInSeparateProcess */ public function test_migrated_class_callback(): void { $this->resetAfterTest(true); @@ -462,7 +428,6 @@ final class manager_test extends \advanced_testcase { 'Callback callbacks::old_class_callback in fake_hooktest component should be migrated to new hook '. 'callback for fake_hooktest\hook\hook_replacing_class_callback' ); - $this->remove_hooktest_plugin(); } /** @@ -472,8 +437,7 @@ final class manager_test extends \advanced_testcase { * * @covers ::get_hooks_deprecating_plugin_callback() * @covers ::is_deprecating_hook_present() - * @return void - * @throws \coding_exception + * @runInSeparateProcess */ public function test_migrated_class_callback_with_replacement(): void { $this->resetAfterTest(true); @@ -503,7 +467,6 @@ final class manager_test extends \advanced_testcase { // Confirm the deprecated class callback is not called, as expected. $this->assertNull(component_class_callback('fake_hooktest\callbacks', 'old_class_callback', [], null, true)); $this->assertDebuggingNotCalled(); - $this->remove_hooktest_plugin(); } /**