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 support for types in expectFilter mocks #254

Conversation

badasswp
Copy link
Contributor

@badasswp badasswp commented Feb 1, 2025

Summary

This PR introduces support for type inclusion in the expectFilter mock expressions. In this way it is possible to assert if an object has been passed in as an argument like so:

\WP_Mock::expectFilter( 'my_filter', \WP_Mock\Functions::type( MyClass::class ) );

Closes: #253

Details

At the moment, when we pass in an object or instance in our apply_filters and proceed to mock it using types, it incorrectly compares two different hashes, because our \WP_Mock\Functions::type( MyClass::class ) returns a Mockery Type object whose hash is different from a new MyClass() object. See here.

$key = $this->safe_offset($arg);
if (! is_array($processors) || ! isset($processors[ $key ])) {
    $this->strict_check();


    return $arg;
}

I have introduced the use of a static array in the Hook abstraction called $objects which basically acts as a hash map to store key/value pairs of the object's class name and the unique hash value of the Mockery Type associated with that object when the WP_Mock\Functions::type() is called in the expectFilter.

/** @var array<mixed> collection of objects mapped to their Type hashes */
    public static array $objects = [];

AND

{
    $type = Mockery::type($expected);
    Filter::$objects[ $expected ] = spl_object_hash($type);

    return $type;
}

In this way, I am able to cross-check if the key exists in the is_object call of the safe_offset method when an object or instance is passed in and then effectively return the correct hash for that object like so:

if (is_object($value)){
    if (! $value instanceof Type) {
        $class = get_class($value);

        if (isset(static::$objects[ $class ])) {
            return static::$objects[ $class ];
        }
    }

    return spl_object_hash($value);
}
unit-tests

Contributor checklist

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly
  • I have added tests to cover changes introduced by this pull request
  • All new and existing tests pass

Testing

Create classes called MyClass & NewClass like so:

namespace MyPlugin;

class MyClass
{
    public function filterContent() : NewClass
    {
        return apply_filters('custom_content_filter', new NewClass());
    }
}

class NewClass
{
    public function __construct()
    {
        echo 'New Class';
    }
}

Create Test like so:

use WP_Mock;
use WP_Mock\Tools\TestCase as TestCase;

use MyPlugin\MyClass;
use MyPlugin\NewClass;

final class MyClassTest extends TestCase
{
    public function testAnonymousObject() : void 
    {
        WP_Mock::expectFilter('custom_content_filter', WP_Mock\Functions::type(NewClass::class));

        $this->assertInstanceOf(NewClass::class, (new MyClass())->filterContent());
        $this->assertConditionsMet();
    }
}

Run test, it should pass successfully:

composer run test

Reviewer checklist

  • Code changes review
  • Documentation changes review
  • Unit tests pass
  • Static analysis passes

Sorry, something went wrong.

@badasswp badasswp force-pushed the feat/add-support-for-types-in-expect-filter-mocks branch from 3fed0cb to 76fdc61 Compare February 1, 2025 13:48
@nmolham-godaddy nmolham-godaddy self-requested a review February 3, 2025 01:29
@coveralls
Copy link

Coverage Status

coverage: 66.446% (+0.1%) from 66.312%
when pulling cb4f051 on badasswp:feat/add-support-for-types-in-expect-filter-mocks
into 010d556 on 10up:trunk.

Copy link
Contributor

@nmolham-godaddy nmolham-godaddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful work @badasswp, I just added few comments and suggestions below

badasswp and others added 3 commits February 3, 2025 09:28
Fix typo in doc block.

Co-authored-by: Nabeel Molham <[email protected]>
chore: ensure only strings are returned

Co-authored-by: Nabeel Molham <[email protected]>
@badasswp
Copy link
Contributor Author

badasswp commented Feb 7, 2025

Hi @nmolham-godaddy If you've got some spare time later today, could you pls help me review this PR at your earliest convenience. Thanks.

@badasswp
Copy link
Contributor Author

@nmolham-godaddy Gentle reminder. If you have got some time, pls could you take a look and review this PR. I've made modifications based on your comments & suggestions from the last time. @agibson-godaddy @rneudorf-godaddy If you could assist, that would be great. Thanks.

Copy link
Contributor

@agibson-godaddy agibson-godaddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for the delay @badasswp . Thanks for your contribution! This looks good to me. :)

@agibson-godaddy agibson-godaddy dismissed nmolham-godaddy’s stale review March 6, 2025 12:30

Changes have been addressed

@agibson-godaddy agibson-godaddy merged commit 7e34b7e into 10up:trunk Mar 6, 2025
@badasswp
Copy link
Contributor Author

badasswp commented Mar 6, 2025

Thank you @agibson-godaddy I appreciate.

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.

Support for Types Inclusion in expectFilter
4 participants