fix: unset current_screen global when restoring null screen state in tests (#841)#846
Conversation
…t teardown Both finally blocks in Dashboard_Widgets_Test were skipping screen restoration when $original_screen_id was null/falsy, allowing the forced screen state set by each test (dashboard-network / dashboard) to leak into subsequent tests. Changed the guard from `if ($original_screen_id)` to `if (null !== $original_screen_id)` and added an explicit `unset($GLOBALS['current_screen'])` in the else branch so that a previously-null screen state is fully restored after each test. Resolves #841
Merge SummaryIssue: #841 — Dashboard_Widgets_Test screen state leakage Change:
Testing: Follows WordPress core pattern from wordpress-develop PR #1380; no behavior change when screen was previously set, explicit cleanup when it was not. aidevops.sh v3.8.24 plugin for OpenCode v1.4.3 with claude-sonnet-4-6 spent 1m on this as a headless worker. Overall, 13s since this issue was created. |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Performance Test Results Performance test results for 791aa8b are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
1 participant
Summary
Fixes screen state leakage in
Dashboard_Widgets_Test— bothfinallyblocks were silently skipping screen restoration when$original_screen_idwas null, allowing the forceddashboard-network/dashboardscreen states to leak into subsequent tests.Changes
File:
tests/WP_Ultimo/Dashboard_Widgets_Test.phpBoth
finallyblocks intest_enqueue_scripts_enqueues_activity_stream_on_indexandtest_enqueue_scripts_skips_activity_stream_on_per_site_dashboardupdated:if ($original_screen_id)→if (null !== $original_screen_id)(strict null check avoids falsy string '' or 0 bypassing restoration)else { unset($GLOBALS['current_screen']); }to explicitly clear the global when there was no original screenThis follows the WordPress core test practice recommended in wordpress-develop PR #1380 and
abstract-testcase.php.Verification
The fix matches the pattern prescribed in the CodeRabbit review finding on PR #821. Both test methods now properly restore the global screen state regardless of whether a screen was active before the test ran.
Resolves #841