From fa2e11105a24309f643454a5528f28ef252f9f58 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Sun, 7 Apr 2024 22:34:58 +0800 Subject: [PATCH] MDL-81456 core: Stop injecting test data into real hook manager --- lib/classes/hook/manager.php | 21 ++---------- lib/phpunit/classes/util.php | 3 -- lib/tests/hook/manager_test.php | 59 ++++++++++++--------------------- lib/upgrade.txt | 2 -- 4 files changed, 24 insertions(+), 61 deletions(-) diff --git a/lib/classes/hook/manager.php b/lib/classes/hook/manager.php index 259a7a27539..febb8bae0f7 100644 --- a/lib/classes/hook/manager.php +++ b/lib/classes/hook/manager.php @@ -82,34 +82,17 @@ final class manager implements * Factory method for testing of hook manager in PHPUnit tests. * * @param array $componentfiles list of hook callback files for each component. - * @param bool $persist If true, the test instance will be stored in self::$instance. Be sure to call $this->resetAfterTest() - * in your test if you use this. * @return self */ - public static function phpunit_get_instance(array $componentfiles, bool $persist = false): manager { + public static function phpunit_get_instance(array $componentfiles): manager { if (!PHPUNIT_TEST) { throw new \coding_exception('Invalid call of manager::phpunit_get_instance() outside of tests'); } $instance = new self(); $instance->load_callbacks($componentfiles); - if ($persist) { - self::$instance = $instance; - } return $instance; } - /** - * Reset self::$instance so that future calls to ::get_instance() will return a regular instance. - * - * @return void - */ - public static function phpunit_reset_instance(): void { - if (!PHPUNIT_TEST) { - throw new \coding_exception('Invalid call of manager::phpunit_reset_instance() outside of tests'); - } - self::$instance = null; - } - /** * Override hook callbacks for testing purposes. * @@ -599,7 +582,7 @@ final class manager implements public function is_deprecated_plugin_callback(string $plugincallback): bool { debugging( 'is_deprecated_plugin_callback method is deprecated, use get_hooks_deprecating_plugin_callback instead.', - DEBUG_DEVELOPER + DEBUG_DEVELOPER, ); return (bool)$this->get_hooks_deprecating_plugin_callback($plugincallback); } diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index f26f3c04a28..527036ce5be 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -112,9 +112,6 @@ class phpunit_util extends testing_util { // Stop all hook redirections. di::get(hook\manager::class)->phpunit_stop_redirections(); - // Reset the hook manager instance. - \core\hook\manager::phpunit_reset_instance(); - // Stop any message redirection. self::stop_message_redirection(); diff --git a/lib/tests/hook/manager_test.php b/lib/tests/hook/manager_test.php index 076a8a2abcc..574926ed51c 100644 --- a/lib/tests/hook/manager_test.php +++ b/lib/tests/hook/manager_test.php @@ -16,6 +16,8 @@ namespace core\hook; +use core\di; + /** * Hooks tests. * @@ -49,27 +51,8 @@ final class manager_test extends \advanced_testcase { $componentfiles = [ 'test_plugin1' => __DIR__ . '/../fixtures/hook/hooks1_valid.php', ]; - $testmanager = manager::phpunit_get_instance($componentfiles, true); + $testmanager = manager::phpunit_get_instance($componentfiles); $this->assertSame(['test_plugin\\hook\\hook'], $testmanager->get_hooks_with_callbacks()); - // With $persist = true, get_instance() returns the test instance until reset. - $manager = manager::get_instance(); - $this->assertSame($testmanager, $manager); - } - - /** - * Test resetting the manager test instance. - * - * @covers ::phpunit_reset_instance - * @return void - */ - public function test_phpunit_reset_instance(): void { - $testmanager = manager::phpunit_get_instance([], true); - $manager = manager::get_instance(); - $this->assertSame($testmanager, $manager); - - manager::phpunit_reset_instance(); - $manager = manager::get_instance(); - $this->assertNotSame($testmanager, $manager); } /** @@ -377,11 +360,13 @@ final class manager_test extends \advanced_testcase { $this->setup_hooktest_plugin(); // Register the fake plugin with the hook manager, but don't define any hook callbacks. - manager::phpunit_get_instance( - [ - 'fake_hooktest' => __DIR__ . '/../fixtures/fakeplugins/hooktest/db/hooks_nocallbacks.php', - ], - true + di::set( + manager::class, + manager::phpunit_get_instance( + [ + 'fake_hooktest' => __DIR__ . '/../fixtures/fakeplugins/hooktest/db/hooks_nocallbacks.php', + ], + ), ); // Confirm a non-deprecated callback is called as expected. @@ -419,11 +404,11 @@ final class manager_test extends \advanced_testcase { $this->setup_hooktest_plugin(); // Register the fake plugin with the hook manager, including the hook callback. - manager::phpunit_get_instance( - [ + di::set( + manager::class, + manager::phpunit_get_instance([ 'fake_hooktest' => __DIR__ . '/../fixtures/fakeplugins/hooktest/db/hooks.php', - ], - true + ]), ); // Confirm a non-deprecated callback is called as expected. @@ -455,11 +440,11 @@ final class manager_test extends \advanced_testcase { $this->setup_hooktest_plugin(); // Register the fake plugin with the hook manager, but don't define any hook callbacks. - manager::phpunit_get_instance( - [ + di::set( + manager::class, + manager::phpunit_get_instance([ 'fake_hooktest' => __DIR__ . '/../fixtures/fakeplugins/hooktest/db/hooks_nocallbacks.php', - ], - true + ]), ); // Confirm a non-deprecated class callback is called as expected. @@ -502,11 +487,11 @@ final class manager_test extends \advanced_testcase { $this->setup_hooktest_plugin(); // Register the fake plugin with the hook manager, including the hook callback. - manager::phpunit_get_instance( - [ + di::set( + manager::class, + manager::phpunit_get_instance([ 'fake_hooktest' => __DIR__ . '/../fixtures/fakeplugins/hooktest/db/hooks.php', - ], - true + ]), ); // Confirm a non-deprecated class callback is called as expected. diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 0ec2de0aade..09a2ec88eb5 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -103,8 +103,6 @@ information provided here is intended especially for developers. * Deprecated core\hook\manager::is_deprecated_plugin_callback() in favour of ::get_hooks_deprecating_plugin_callback(), which will return the classnames of hooks deprecating a callback, or null if it's not deprecated. The return value can be cast to bool if the original functionality is desired. -* core\hook\manager::phpunit_get_instance() now sets self::$instance to the mocked instance if the optional $persist argument is - true, so future calls to ::get_instance() will return it. * The triggerSelector method in the `core/comboboxsearch/search_combobox` JS module is deprecated. It was not used. * PHPUnit has been upgraded to 9.6 (see MDL-81266 for details). The main goal of the update is to allow developers to know in advance,