From 9327bbedb49e5c6f3e561d861ba082fe5b7182e2 Mon Sep 17 00:00:00 2001 From: David Stone Date: Mon, 13 Apr 2026 12:07:36 -0600 Subject: [PATCH] test: harden Dashboard_Widgets_Test global cleanup with try/finally Addresses CodeRabbit review feedback from PR #801: - Capture $original_done and screen state before mutating globals - Wrap test body in try/finally so globals always restored on failure - Restore $wp_scripts->done and screen context in finally block Applies to both test_enqueue_scripts_enqueues_activity_stream_on_index and test_enqueue_scripts_skips_activity_stream_on_per_site_dashboard to prevent cross-test leakage when assertions throw. Fixes #806 --- tests/WP_Ultimo/Dashboard_Widgets_Test.php | 130 ++++++++++++--------- 1 file changed, 75 insertions(+), 55 deletions(-) diff --git a/tests/WP_Ultimo/Dashboard_Widgets_Test.php b/tests/WP_Ultimo/Dashboard_Widgets_Test.php index e32fb708..d3a2a640 100644 --- a/tests/WP_Ultimo/Dashboard_Widgets_Test.php +++ b/tests/WP_Ultimo/Dashboard_Widgets_Test.php @@ -124,39 +124,49 @@ public function test_enqueue_scripts_enqueues_activity_stream_on_index(): void { global $pagenow, $wp_scripts; - $original = $pagenow; - $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; - $pagenow = 'index.php'; - - if (isset($wp_scripts)) { - $wp_scripts->queue = []; - $wp_scripts->done = []; - } - - // Simulate the network admin so is_network_admin() returns true. - set_current_screen('dashboard-network'); - - // Ensure wu-functions is registered so the dependency chain resolves. - \WP_Ultimo\Scripts::get_instance()->register_default_scripts(); - - $instance = $this->get_instance(); - $instance->enqueue_scripts(); - - $this->assertTrue( - wp_script_is('wu-activity-stream', 'enqueued'), - 'wu-activity-stream should be enqueued on the network admin index.php' - ); - - // Verify wu-functions and moment are declared dependencies. - $script = $wp_scripts->registered['wu-activity-stream'] ?? null; - $this->assertNotNull($script, 'wu-activity-stream should be registered'); - $this->assertContains('wu-functions', $script->deps); - $this->assertContains('moment', $script->deps); - - if (isset($wp_scripts)) { - $wp_scripts->queue = $original_queue; + $original = $pagenow; + $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; + $original_done = isset($wp_scripts) ? $wp_scripts->done : []; + $original_screen = function_exists('get_current_screen') ? get_current_screen() : null; + $original_screen_id = $original_screen ? $original_screen->id : null; + $pagenow = 'index.php'; + + try { + if (isset($wp_scripts)) { + $wp_scripts->queue = []; + $wp_scripts->done = []; + } + + // Simulate the network admin so is_network_admin() returns true. + set_current_screen('dashboard-network'); + + // Ensure wu-functions is registered so the dependency chain resolves. + \WP_Ultimo\Scripts::get_instance()->register_default_scripts(); + + $instance = $this->get_instance(); + $instance->enqueue_scripts(); + + $this->assertTrue( + wp_script_is('wu-activity-stream', 'enqueued'), + 'wu-activity-stream should be enqueued on the network admin index.php' + ); + + // Verify wu-functions and moment are declared dependencies. + $script = $wp_scripts->registered['wu-activity-stream'] ?? null; + $this->assertNotNull($script, 'wu-activity-stream should be registered'); + $this->assertContains('wu-functions', $script->deps); + $this->assertContains('moment', $script->deps); + + } finally { + if (isset($wp_scripts)) { + $wp_scripts->queue = $original_queue; + $wp_scripts->done = $original_done; + } + if ($original_screen_id) { + set_current_screen($original_screen_id); + } + $pagenow = $original; } - $pagenow = $original; } /** @@ -170,30 +180,40 @@ public function test_enqueue_scripts_skips_activity_stream_on_per_site_dashboard global $pagenow, $wp_scripts; - $original = $pagenow; - $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; - $pagenow = 'index.php'; - - if (isset($wp_scripts)) { - $wp_scripts->queue = []; - $wp_scripts->done = []; - } - - // Simulate the per-site dashboard (not network admin). - set_current_screen('dashboard'); - - $instance = $this->get_instance(); - $instance->enqueue_scripts(); - - $this->assertFalse( - wp_script_is('wu-activity-stream', 'enqueued'), - 'wu-activity-stream should NOT be enqueued on the per-site dashboard' - ); - - if (isset($wp_scripts)) { - $wp_scripts->queue = $original_queue; + $original = $pagenow; + $original_queue = isset($wp_scripts) ? $wp_scripts->queue : []; + $original_done = isset($wp_scripts) ? $wp_scripts->done : []; + $original_screen = function_exists('get_current_screen') ? get_current_screen() : null; + $original_screen_id = $original_screen ? $original_screen->id : null; + $pagenow = 'index.php'; + + try { + if (isset($wp_scripts)) { + $wp_scripts->queue = []; + $wp_scripts->done = []; + } + + // Simulate the per-site dashboard (not network admin). + set_current_screen('dashboard'); + + $instance = $this->get_instance(); + $instance->enqueue_scripts(); + + $this->assertFalse( + wp_script_is('wu-activity-stream', 'enqueued'), + 'wu-activity-stream should NOT be enqueued on the per-site dashboard' + ); + + } finally { + if (isset($wp_scripts)) { + $wp_scripts->queue = $original_queue; + $wp_scripts->done = $original_done; + } + if ($original_screen_id) { + set_current_screen($original_screen_id); + } + $pagenow = $original; } - $pagenow = $original; } /**