Skip to content

Tests: Cover wp_lostpassword_url() - #15227

Open
devmuhib009 wants to merge 1 commit into
WordPress:trunkfrom
devmuhib009:tests/65819-cover-wp-lostpassword-url
Open

devmuhib009 wants to merge 1 commit into
WordPress:trunkfrom
devmuhib009:tests/65819-cover-wp-lostpassword-url

Conversation

@devmuhib009

Copy link
Copy Markdown
Member

This PR adds comprehensive unit test coverage for wp_lostpassword_url():

  • Tests that wp_lostpassword_url() returns the default lost password URL on both single site and multisite installations.
  • Tests that wp_lostpassword_url() appends the redirect_to query argument when a redirect URL is passed.
  • Tests that the lostpassword_url filter hook receives ($lostpassword_url, $redirect) and modifies the returned URL.

Trac ticket: https://core.trac.wordpress.org/ticket/65819
Props devmuhib009.

Add unit tests for wp_lostpassword_url():
- Verify wp_lostpassword_url() returns the default lost password URL on single and multisite.
- Verify wp_lostpassword_url() appends redirect_to query parameter.
- Verify the 'lostpassword_url' filter hook receives expected arguments and modifies the output.

See #65819.
Props devmuhib009.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props devmuhib.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@huzaifaalmesbah huzaifaalmesbah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Clean unit test coverage for wp_lostpassword_url(). Properly accounts for multisite vs single-site paths, tests query string redirection args, and verifies the lostpassword_url filter with its arguments and overrides. All CI checks pass.

AI Review: Gemini 3.8 Flash

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the added wp_lostpassword_url() test coverage.

No open issues 🎉

What changed in this PR

Adds PHPUnit coverage for wp_lostpassword_url(), supporting Trac ticket #65819 by checking URL generation and filter behavior.

Changes:

  • Verifies the default lost-password URL using expectations for single-site and multisite environments.
  • Checks that a supplied redirect is preserved in the redirect_to query argument.
  • Confirms the lostpassword_url filter receives the expected arguments and can override the returned URL.
File Description
tests/​phpunit/​tests/​general/​wpLostpasswordUrl.php Adds tests for default URLs, redirect handling, and filter behavior.

Review effort: Balanced

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default URL, redirect, and filter arguments/return are already covered by Tests_General_WpLoginScreenUrls in r65056. It additionally tests a non-root multisite path and forced HTTPS with explicit expected URLs. Please avoid adding a duplicate class; extend the existing class only for a distinct missing regression. Otherwise this PR can close as already covered.

All proposed cases passed locally in randomized single-site and multisite runs; the concern is duplicate coverage, not a runtime failure.

AI review · gpt-6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants