Fixes a bug that causes a responder not to run the intercepted method while testing an hook that uses a closure + other minor fixes #224
Conversation
…nstance of \Mockery\Mathcer\Type Fixed a bug in Hook::safe_offset that caused \Mockery\Mathcer\Type::(\Closure::class) objects to be treated as other objects. Now they correctly return __CLOSURE__, in order to test that hooks that use closures are added. Added a provider entry to the relative unit test to ensure the correct behavior. Fixed a bug in HookedCallback::react that caused all the hooks added via \WP_Mock::addHook to be added as filters. Now the correct type is passed.
There was a problem hiding this comment.
hey @Talpx1 thanks for the contribution! this looks great! I think we can merge it -- can I just ask you to update the objects references like in phpdocs to avoid using fully qualified namespaces - just to keep things short (IDEs and static analyzers should parse those anyway) -- thank you!!
Lastly, sorry for my English and if I'm doing something wrong, it's my absolute first pull request :)
no worries at all, pull request perfetta, ciao e grazie!
removed the use of fully qualified namespaces in comment and type check as suggested by @unfulvio-godaddy Co-authored-by: Fulvio Notarstefano <unfulvio@godaddy.com>
better naming for Mockery matcher type in data provider Co-authored-by: Fulvio Notarstefano <unfulvio@godaddy.com>
|
Hey @unfulvio-godaddy, thanks for reviewing my PR. I removed the fully qualified namespaces and committed your suggestions. Grazie a te! :) |
|
hey @Talpx1 thanks for those updates looks like you may have missed a namespace reference in Hook.php which is currently having the tests failing -- if you add |
|
Oops! Sorry about that :( |
|
all good now @Talpx1 thanks again for your contribution! |
|
@unfulvio-godaddy Happy to help! :) |
Summary
WP_Mock::expect* functions now are correctly annotated to accept an Instance of \Mockery\Mathcer\Type
Fixed a bug in Hook::safe_offset that caused \Mockery\Matcher\Type::(\Closure::class) objects to be treated as other objects. Now they correctly return CLOSURE, in order to test that hooks that use closures are added. Added a provider entry to the relative unit test to ensure the correct behavior. Fixed a bug in HookedCallback::react that caused all the hooks added via \WP_Mock::addHook to be added as filters. Now the correct type is passed.
Closes: Does not close any open issue
Details
I was testing an action that uses a closure. Even if I used the same line as the documentation, I was getting an error on a Mockery instance that I never created. I found out it was the one created inside WP_Mock::expectActionAdded, that checks if the interceptor method has been executed. Well, it was not getting executed because the processor array key for this Mockery\Matcher\Type instance was the hash of the object itself and not the string CALLABLE as it is expected. I added an additional check in the Hook::safe_offset in order to prevent this behavior, the $callback variable you will see is used by Mockery\Matcher\Type to ensure that the callable is actually a closure, it's the only way I found since the parameter is passed by reference. While working on it, I noticed that in HookedCallback::react the call to \WP_Mock::addHook was not passing the type as a parameter, causing all the hooks to be registered as a filter (which is the default value), so I added the type parameter. Lastly I added to the WP_Mock::expect* functions the appropriate PHPDoc to hint the IDEs that these methods can also accept a \Mockery\Matcher\Type instance. I also updated the Hook test data provider to test the case I described above.
Lastly, sorry for my English and if I'm doing something wrong, it's my absolute first pull request :)
PS: I would like, if possible, to contribute further to make this package compatible with PHPUnit 10.x
Contributor checklist
Testing
Reviewer checklist