Skip to content
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

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from
23 changes: 22 additions & 1 deletion docs/usage/mocking-wp-action-and-filter-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,25 @@ final class MyClassTest extends TestCase
$this->assertEquals('Default', (new MyClass())->filterContent());
}
}
```
```

## Asserting that actions and filters have been removed

Similarly, we can test that actions and filters are removed when expected, e.g.removing another plugin's admin notice. This is done using `WP_Mock::expectActionRemoved()` and `WP_Mock::expectFilterRemoved()`. Or conversely, we can confirm that they have _not_ been removed using `WP_Mock::expectActionNotRemoved()` and `WP_Mock::expectFilterNotRemoved()`. The latter functions are useful where a function being tested returns early in some scenarios, and we want to ensure that the hooks are not removed in that case.

```php
use MyPlugin\MyClass;
use WP_Mock\Tools\TestCase as TestCase;

final class MyClassTest extends TestCase
{
public function testRemoveAction() : void
{
$classInstance = new MyClass();

WP_Mock::expectActionRemoved('admin_notices', 'invasive_admin_notice');

$classInstance->removeInvasiveAdminNotice();
}
}
```
84 changes: 84 additions & 0 deletions php/WP_Mock.php
Original file line number Diff line number Diff line change
Expand Up @@ -546,4 +546,88 @@ public static function getDeprecatedMethodListener(): DeprecatedMethodListener
{
return static::$deprecatedMethodListener;
}

/**
* Adds an expectation that an action should be removed.
*
* @param string $action the action hook name
* @param string|callable-string|callable|Type $callback the callable to be removed
* @param int|Type|null $priority the priority it should be registered at
*
* @return void
* @throws InvalidArgumentException
*/
public static function expectActionRemoved(string $action, $callback, $priority = null) : void
Comment on lines +557 to +560
Copy link
Contributor

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

Suggested change
* @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,
)
);
Comment on lines +562 to +568
Copy link
Contributor

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.

Suggested change
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,
]);

}

/**
* Adds an expectation that an action should not be removed.
*
* @param string $action the action hook name
* @param null|string|callable-string|callable|Type $callback the callable to be removed
* @param int|Type|null $priority optional priority for the registered callback that is being removed
*
* @return void
* @throws InvalidArgumentException
*/
public static function expectActionNotRemoved(string $action, $callback, $priority = null) : void
{
self::userFunction(
'remove_action',
array(
'args' => array_filter(func_get_args()),
'times' => 0,
)
);
}

/**
* Adds an expectation that a filter should be removed.
*
* @param string $filter the filter name
* @param string|callable-string|callable|Type $callback the callable to be removed
* @param int|Type|null $priority the registered priority
*
* @return void
* @throws InvalidArgumentException
*/
public static function expectFilterRemoved(string $filter, $callback, $priority = null) : void
{
self::userFunction(
'remove_filter',
array(
'args' => array_filter(func_get_args()),
'times' => 1,
)
);
}

/**
* Adds an expectation that a filter should not be removed.
*
* @param string $filter the filter name
* @param null|string|callable-string|callable|Type $callback the callable to be removed
* @param int|Type|null $priority optional priority for the registered callback that is being removed
*
* @return void
* @throws InvalidArgumentException
*/
public static function expectFilterNotRemoved(string $filter, $callback, $priority = null) : void
{
self::userFunction(
'remove_filter',
array(
'args' => array_filter(func_get_args()),
'times' => 0,
)
);
}
}
56 changes: 56 additions & 0 deletions tests/Integration/WP_MockTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,60 @@ public function testCanExpectHooksNotAdded() : void

$this->assertConditionsMet();
}

/**
* @covers \WP_Mock::expectActionRemoved()
* @covers \WP_Mock::expectFilterRemoved()
*
* @return void
* @throws Exception
*/
public function testCanExpectHooksRemoved() : void
{
WP_Mock::expectActionRemoved('wpMockTestActionWithCallbackAndPriority', 'wpMockTestFunction', 10);
WP_Mock::expectFilterRemoved('wpMockTestFilterWithCallbackAndPriority', 'wpMockTestFunction', 10);

WP_Mock::expectActionRemoved(
'wpMockTestActionWithCallbackAndAnyPriority',
'wpMockTestFunction',
\WP_Mock\Functions::type('int')
);
WP_Mock::expectFilterRemoved(
'wpMockTestFilterWithCallbackAndAnyPriority',
'wpMockTestFunction',
\WP_Mock\Functions::type('int')
);

WP_Mock::expectActionRemoved('wpMockTestActionWithCallbackAndDefaultPriority', 'wpMockTestFunction');
WP_Mock::expectFilterRemoved('wpMockTestFilterWithCallbackAndDefaultPriority', 'wpMockTestFunction');

remove_action('wpMockTestActionWithCallbackAndPriority', 'wpMockTestFunction', 10);
remove_filter('wpMockTestFilterWithCallbackAndPriority', 'wpMockTestFunction', 10);

remove_action('wpMockTestActionWithCallbackAndAnyPriority', 'wpMockTestFunction', rand(1,100));
remove_filter('wpMockTestFilterWithCallbackAndAnyPriority', 'wpMockTestFunction', rand(1,100));

remove_action('wpMockTestActionWithCallbackAndDefaultPriority', 'wpMockTestFunction');
remove_filter('wpMockTestFilterWithCallbackAndDefaultPriority', 'wpMockTestFunction');

$this->assertConditionsMet();
}

/**
* @covers \WP_Mock::expectActionNotRemoved()
* @covers \WP_Mock::expectFilterNotRemoved()
*
* @return void
* @throws Exception
*/
public function testCanExpectHooksNotRemoved() : void
{
WP_Mock::expectActionNotRemoved('wpMockTestActionWithCallbackAndPriority', 'wpMockTestFunction', 10);
WP_Mock::expectFilterNotRemoved('wpMockTestFilterWithCallbackAndPriority', 'wpMockTestFunction', 10);

WP_Mock::expectActionNotRemoved('wpMockTestActionWithCallbackAndDefaultPriority', 'wpMockTestFunction');
WP_Mock::expectFilterNotRemoved('wpMockTestFilterWithCallbackAndDefaultPriority', 'wpMockTestFunction');

$this->assertConditionsMet();
}
}
Loading