Brain-WP/BrainMonkey

There should be an expectRemoved function for Filters and Actions

mundschenk-at opened this issue · 4 comments

mundschenk-at commented

I've noticed that there currently is no Filters\expectRemoved function as a counterpart to Filters\expectAdded. Is there a technical reason or is this simply an oversight? I think providing such functions (for filters and actions) would make the API more complete.

gmazzap commented

Hi @mundschenk-at

Honestly I rarely use mocking removal of hooks...this missing goes back to v1 of Brain Monkey and I never missed it, and you are the first reporting this in 3 years, so I guess is not that common for people to desire moking of hooks removal.

That said, I can understand that is a legitimate request, and I have nothing against that. Unfortunately is not a super simple change, as it needs change in 2 or 3 classes, plus I would need tests before merging.

If you want to take the time to write a PR I can guide you futher, otherwise I will leave this open hoping for someone to step in, or I'll do myself as soon as I can, but I can already say I can't work on this in the immediate future.

mundschenk-at commented

Hi @gmazzap, if you can point me to the relevant classes/files, I'll look into providing a PR.

gmazzap commented

Hi @mundschenk-at thanks!

  1. First you need to add "expectation target" types here https://github.com/Brain-WP/BrainMonkey/blob/master/src/Expectation/ExpectationTarget.php#L30-L43
  2. Then you need to add factory methods in the ExpectationFactory, taking as example https://github.com/Brain-WP/BrainMonkey/blob/master/src/Expectation/ExpectationFactory.php#L66-L71
  3. Then you need to add execute metods in the HookExpectationExecutor taking as example this https://github.com/Brain-WP/BrainMonkey/blob/master/src/Hook/HookExpectationExecutor.php#L52-L55
  4. After that, you need to edit the mocked functions (https://github.com/Brain-WP/BrainMonkey/blob/master/inc/wp-hook-functions.php#L98-L114) to also execute the expectation, (see how that is done for add_action https://github.com/Brain-WP/BrainMonkey/blob/master/inc/wp-hook-functions.php#L53). However, while mocked add_action returns void, you need to to return true or false based on the fact that the action was there or not. You can use Monkey\Actions\has / Monkey\Filters\has for the scope.
  5. You should also prevent that whenHappen (https://github.com/Brain-WP/BrainMonkey/blob/master/src/Expectation/Expectation.php#L221-L229) is used with remove extetation types.
  6. Finally there should be some tests, see how other API functions are tested https://github.com/Brain-WP/BrainMonkey/tree/master/tests/src/Api and also changed classes like ExpectationTarget, ExpectationFactory, HookExpectationExecutor, and Expectation, should have some more added tests.

It is not a very trivial work... but I hope you don't get discouraged.

gmazzap commented

The two expectations functions were added in v2.3.0

  • 🎉1