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 AnyInstance matcher. Fix #160 #161

Merged
merged 14 commits into from
Oct 27, 2022

Conversation

BrianHenryIE
Copy link
Contributor

@BrianHenryIE BrianHenryIE commented Jul 7, 2021

Description of the Change

Adds a new matcher, AnyInstance to allow using expectActionAdded and expectFilterAdded without having a reference to the object that will be added to the hook. e.g.

\WP_Mock::expectFilterAdded( 'the_content', array( new \WP_Mock\Matcher\AnyInstance( Special::class ), 'the_content' ) );

Alternate Designs, Benefits, Possible Drawbacks

Discussed in #160, opened in January 2021, which references six further related issues.

Maybe this should be part of Functions::type( $type ).

Verification Process

  • I've been using this in my own projects for about six months now, it works as desired. e.g. tests
  • I've written unit tests against the new class itself: AnyInstanceTest
  • How did you verify that the change has not introduced any regressions?

The existing tests were failing, so I'm not certain.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.
  • There is no phpcs.xml in the project.
  • The existing tests were already failing.

Applicable Issues

#160, #8, #80, #93, #127, #143, #12.

Changelog Entry

### Added
- AnyInstance matcher

Included in the PR at: CHANGELOG.md

Maybe can be merged into FuzzyObject.
Previously it would only work both classes were subclasses of a common parent. Now it matches if there is a sub-class/parent-class relationship between the two classes.
@BrianHenryIE BrianHenryIE changed the title Any instance Fix #160 Add AnyInstance matcher. Fix #160 Jul 7, 2021
@BrianHenryIE
Copy link
Contributor Author

The changes to FuzzyObject need particular attention. It's a change to FuzzyObject::haveCommonAncestor() which changes the behaviour to "is the expected class a subclass of the actual class".

Maybe that should be moved into AnyInstance and not interfere with FuzzyObject.

@jeffpaul jeffpaul requested a review from ericmann July 7, 2021 18:28
@jeffpaul jeffpaul added this to the 0.4.3 milestone Jul 7, 2021
@coveralls
Copy link

coveralls commented Sep 22, 2022

Coverage Status

Coverage increased (+2.0%) to 12.166% when pulling bc9770d on BrianHenryIE:any-instance into b647631 on 10up:trunk.

@BrianHenryIE
Copy link
Contributor Author

hey @unfulvio-godaddy , is there anything I can do to help get this merged? I've been using it extensively since before the PR and it's been smooth.

@unfulvio-godaddy
Copy link
Member

this looks good, thank you @BrianHenryIE

probably in the future we are going to introduce some PHPCS standards to improve on code consistency, but right now I'd like to have this merged since it addressed the problem outlined in some issues and seems fairly straightforward without breaking changes --- thanks

@unfulvio-godaddy unfulvio-godaddy merged commit d058417 into 10up:trunk Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnyInstance matcher for expectHookAdded How to expectFilterAdded in constructor
4 participants