Add tests for wp_nonce_url().
#2653
Conversation
costdev
force-pushed
the
wp_nonce_url-unit-tests
branch
2 times, most recently
from
3b0f573 to
e99d99c
Compare
costdev
force-pushed
the
wp_nonce_url-unit-tests
branch
2 times, most recently
from
6620c85 to
c949408
Compare
costdev
force-pushed
the
wp_nonce_url-unit-tests
branch
from
c949408 to
5be5a43
Compare
| * @param string $actionurl URL to add nonce action. | ||
| */ | ||
| public function test_should_handle_existing_query_args( $actionurl ) { | ||
| $actionurl = preg_replace( '/&(?!amp;)/', '&', $actionurl ); |
There was a problem hiding this comment.
This looks a little like it is testing the code by running a copy-paste of the code.
I think i'd prefer an $expted to be passed so it's clear what is been tested.
There was a problem hiding this comment.
Not quite a copy-paste. It's taking a realistic URL from the data provider, and ensuring that it has &. In doing so, this verifies that str_replace() is called in wp_nonce_url() to convert & to &.
I'm happy to change the variable on Line 108 from $actionurl to $expected. The parameter however is named $actionurl to increase familiarity between the source function and the test method/datasets for easier testing, so I'd like to keep the parameter name as-is. Does that work for you?
There was a problem hiding this comment.
I was thinking more of using two params, $actionulr, $expected, to avoid surprises
There was a problem hiding this comment.
Hmm... Looking back over this test method and datasets, this PR needs more work to properly cover wp_nonce_url.
I'll ping you when I have this revision work done.
costdev
force-pushed
the
wp_nonce_url-unit-tests
branch
from
5be5a43 to
da9f3a4
Compare
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
LGTM, added a suggestion about a more precise assertion because I am the worst.
Literally testing how long a string is. Thanks, Peter.
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
Dandy 🌻
Thank you!
|
Committed in https://core.trac.wordpress.org/changeset/54407 / 426d20a |
4 participants
This PR revises the datasets from PR 2201 for accurate testing.
Trac ticket: https://core.trac.wordpress.org/ticket/54870