Repository navigation
Tests: Add unit tests for is_protected_ajax_action() - #14473
shail-mehta wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no correctness or isolation issues in this test-only change; tests were not executed.
No open issues 🎉
What changed in this PR
Adds dedicated PHPUnit coverage for is_protected_ajax_action() under ticket #65819, without changing production code.
Changes:
- Covers non-Ajax requests, missing actions, and protected versus unprotected actions.
- Tests filter-based protection and confirms the filter is skipped outside Ajax.
- Restores request state and isolates tests that define
DOING_AJAX.
| File | Description |
|---|---|
| tests/phpunit/tests/load/isProtectedAjaxAction.php | Adds six tests covering action protection and filter behavior. |
Review effort: Balanced
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
mukeshpanchal27
left a comment
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Thanks for the tests. The assertions match is_protected_ajax_action(). One suggestion inline that would remove the need for separate processes.
| * @preserveGlobalState disabled | ||
| */ | ||
| public function test_returns_false_when_doing_ajax_without_action() { | ||
| define( 'DOING_AJAX', true ); |
There was a problem hiding this comment.
wp_doing_ajax() filters this constant: apply_filters( 'wp_doing_ajax', defined( 'DOING_AJAX' ) && DOING_AJAX ). Could these tests use add_filter( 'wp_doing_ajax', '__return_true' ); instead of define()? Then they can drop @runInSeparateProcess and run in the normal process. The test framework restores hooks after each test. Each separate process starts WordPress again, so this keeps the suite faster.
lancewillett
left a comment
lancewillett
left a comment
There was a problem hiding this comment.
The checks cover Ajax preconditions, the protected-action list, and filter additions. Mukesh’s process-isolation feedback is addressed. Optional cleanup: the non-Ajax false-result test is subsumed by the adjacent filter-not-called test, which asserts the same result.
Unit-tests audit and Copilot review found no blockers. PHPCS passed. Each class passed separately, and the combined batch passed randomized single-site and multisite runs: 63 tests, 95 assertions each (seeds 65829 and 65830). CI is green on the reviewed commit.
AI review · gpt-6
Verify Ajax preconditions, built-in protected actions, unknown actions, and filtered additions. Use the Ajax filter to avoid unnecessary process isolation. Developed in: #14473 Props shailu25, mukesh27. See #65819. git-svn-id: https://develop.svn.wordpress.org/trunk@64552 602fd350-edb4-49c9-b593-d223f7449a82
Verify Ajax preconditions, built-in protected actions, unknown actions, and filtered additions. Use the Ajax filter to avoid unnecessary process isolation. Developed in: WordPress/wordpress-develop#14473 Props shailu25, mukesh27. See #65819. Built from https://develop.svn.wordpress.org/trunk@64552 git-svn-id: http://core.svn.wordpress.org/trunk@63702 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
PR #14473 landed in https://core.trac.wordpress.org/changeset/64552 |
What
Trac ticket: #65819
Adds unit tests for
is_protected_ajax_action()intests/phpunit/tests/load/isProtectedAjaxAction.php.Covers non-Ajax requests, missing actions, the default protected action list, and the
wp_protected_ajax_actionsfilter.Test-only change.
Why
This helper has had no dedicated PHPUnit coverage. Fits the #65819 missing-coverage track, and no open PR covers it.
How
Testing instructions
Use of AI Tools