Make WordPress Core

Opened 8 years ago

Last modified 3 years ago

#36786 new defect (bug)

Can't pass filter names to `MockAction::get_call_count()`

Reported by: dlh's profile dlh Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-testing needs-refresh
Focuses: Cc:

Description (last modified by netweb)

The first of these tests works as expected, but the second generates an "Undefined index" notice.

<?php
function test_get_call_count_action() {
        $action = rand_str();
        $ma = new MockAction();

        add_action( $action, array( $ma, 'action' ) );
        do_action( $action );

        $this->assertSame( 1, $ma->get_call_count( 'action' ) );
}

function test_get_call_count_filter() {
        $filter = rand_str();
        $ma = new MockAction();

        add_filter( $filter, array( $ma, 'filter' ) );
        apply_filters( $filter, rand_str() );

        $this->assertSame( 1, $ma->get_call_count( 'filter' ) );
}

The attached patch attempts to fix the notice and allow passing filter names to get_call_count().

Attachments (2)

36786.patch (777 bytes) - added by dlh 8 years ago.
1655.patch (4.2 KB) - added by gaambo 3 years ago.
#36786 refresh patch

Download all attachments as: .zip

Change History (7)

@dlh
8 years ago

#1 @netweb
8 years ago

  • Description modified (diff)
  • Keywords has-patch added

#2 @johnbillion
7 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#3 @isabel_brison
3 years ago

  • Keywords needs-refresh added

The patch no longer applies cleanly.

This ticket was mentioned in PR #1655 on WordPress/wordpress-develop by gaambo.


3 years ago
#4

  • Keywords has-unit-tests added; needs-refresh removed

Fixes MockAction::get_call_count for filters (Undefined index notice) and fix REST API tests to use function correctly

Trac ticket: 36786

#5 @gaambo
3 years ago

  • Keywords needs-refresh added; has-unit-tests removed

Actually, it's not the action and filter key that need to be compared (that was my first thought too), but the tag key. Naming of action/filter/tag in the MockAction class is kind of confusing on first look.
The events array stores all called hooks in an array, where the key action or filter is the callback function and tag is the action-/filter-name (as the first parameter in do_action/apply_filters), that's kind of confusing, but I didn't want to change all of the naming in this class, because that might be a bigger change and the event property is also public (and may be used in other ways I can't find). If I should do the renaming, just let me know.

I also had to fix some rest-api tests because they were using this function in a wrong way. If I understand the function comment correctly, $tag is the hook name (action/filter as the first parameter in do_action and apply_filters). But the tests were passing the callback function as parameter (to get the call count of this specific function). I removed the attribute from the function callings in the rest-api test and all tests are running fine.
I don't see any other place in which get_call_count is called in this way. But if needed, we could add a get_function_call_count function to check for the call count of the function.

Here is the refreshed patch.

@gaambo
3 years ago

#36786 refresh patch

Note: See TracTickets for help on using tickets.