10up / wp_mock

WordPress API Mocking Framework
https://wp-mock.gitbook.io
Other
674 stars 70 forks source link

WP_Mock::expectHookAdded() does not support calling add_action() with a closure #89

Closed acobster closed 1 year ago

acobster commented 7 years ago

TL;DR: when I call add_action() with a closure callback, expectHookAdded() does not recognize the closure as having been called. Adding a call to $responder->react() inside expectHookAdded() fixes this.

Since the real add_action() is designed to take any callable, I would expect to be able to call expectHookAdded with any valid callable, including a closure.

For example, I am trying to unit test a trait that encapsulates adding an admin filter on the Posts listing screen. Specifically, I'm testing the trait's addAdminFilter() method. I would expect to be able to do something like this:

<?php

require_once 'vendor/autoload.php';

use PHPUnit\Framework\TestCase;

trait HasCustomAdminFilters {
  public function addAdminFilter( $name, $options, $postType, callable $queryModifier ) {
    // allow user to define how they want the admin query modified
    add_action('pre_get_posts', function(\WP_Query $query) use($name, $postType, $queryModifier) {
      // call the user-defined query modifying code
      $queryModifier($query);
    });
  }
}

class HasCustomAdminFiltersTest extends TestCase {
  public function setUp() {
    WP_Mock::setUp();
  }

  public function tearDown() {
    WP_Mock::tearDown();
  }

  public function testAddAdminFilter() {
    $callback = function() {};

    WP_Mock::expectActionAdded('pre_get_posts', WP_Mock\Functions::type('callable'));

    // register the action handlers
    $this->getObjectForTrait('HasCustomAdminFilters')->addAdminFilter(
      'custom_filter',
      ['dropdown' => 'options'],
      'my_post_type',
      $callback
    );
  }
}

This throws:

Mockery\Exception\InvalidCountException: Method intercepted() from Mockery_0__intercept should be called at least 1 times but called 0 times.

Digging into the WP_Mock class, I see that the expectHookAdded() code creates an internal mock object and tells it to be intercepted; then it "performs" the added hook.

This works for the basic use-case of declaring a global function, e.g. foo(), and then calling add_action('pre_get_posts', 'foo'). But in my use-case, I want to encapsulate my callback as a closure, not expose it as a global function. Adding a call to $responder->react() at the end of expectHookAdded() fixes this issue.

I'm not sure if such a change would have other implications. Is it an appropriate solution? Should expectAction() also get the same treatment?

ericmann commented 7 years ago

@acobster Would you be willing to submit a PR making that change? This will make it easier for us to see what's going on and evaluate the potential impact there ...

acobster commented 7 years ago

Absolutely, I'll get one in soon!

On May 26, 2017 10:04 PM, "Eric Mann" notifications@github.com wrote:

@acobster https://github.com/acobster Would you be willing to submit a PR making that change? This will make it easier for us to see what's going on and evaluate the potential impact there ...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/10up/wp_mock/issues/89#issuecomment-304428044, or mute the thread https://github.com/notifications/unsubscribe-auth/AATksKDUIVSUcjnMYUjaj4i5uGueB-tVks5r967RgaJpZM4Madz- .