-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add convenience ::expectFilterRemoved()
::expectActionRemoved()
Fix #157
#246
base: trunk
Are you sure you want to change the base?
Add convenience ::expectFilterRemoved()
::expectActionRemoved()
Fix #157
#246
Conversation
`\WP_Mock\Functions::type( 'int' )`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @BrianHenryIE for this nice PR, I have a couple of suggestions below, which I added for one method, but apply for all of the other new methods as well.
* @return void | ||
* @throws InvalidArgumentException | ||
*/ | ||
public static function expectActionRemoved(string $action, $callback, $priority = null) : void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about returning the expectation once set? incase it was needed to for later. As well as adding nullable-int data type for the $priority
argument
* @return void | |
* @throws InvalidArgumentException | |
*/ | |
public static function expectActionRemoved(string $action, $callback, $priority = null) : void | |
* @return Mockery\Expectation | |
* @throws InvalidArgumentException | |
*/ | |
public static function expectActionRemoved(string $action, $callback, ?int $priority = null) : Mockery\Expectation |
self::userFunction( | ||
'remove_action', | ||
array( | ||
'args' => array_filter(func_get_args()), | ||
'times' => 1, | ||
) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better performance, it would be ideal to be more implicit about the aguments instead of reaching for the func_get_args
, that way the interpreter won't have to do much assumption work with the data types.
Also, we are trying to move away from the classic array syntax towards.
self::userFunction( | |
'remove_action', | |
array( | |
'args' => array_filter(func_get_args()), | |
'times' => 1, | |
) | |
); | |
$args = [$action, $callback]; | |
if ($priority) { | |
$args[] = $priority; | |
} | |
return self::userFunction('remove_action', [ | |
'args' => $args, | |
'times' => 1, | |
]); |
Summary
As said in #157:
See:
remove_filter()
.Closes: #157
Details
It's really just a convenience function on top of
WP_Mock::userFunction()
.We're using
array_filter()
there to handle the optional use of$priority
inremove_filter()
.::expectFilterNotRemoved()
is the same with'times' => 0
.Contributor checklist
Testing
Reviewer checklist