Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#57342 closed defect (bug) (fixed)

Tests_Query_TaxQuery::test_tax_terms_should_limit_query PHPUnit test is not testing the correct SQL query

Reported by: davidbinda's profile david.binda Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

I have stumbled upon an issue with the Tests_Query_TaxQuery::test_tax_terms_should_limit_query PHPUnit test, which is not testing the correct SQL query.

The MockAction's filter is added to terms_pre_query filter hook on the very begining of the method, prior the test posts are created and terms assigned to those.

The terms_pre_query hook is being triggered during the post creation from various functions, especially from the term_exists since the r52921

Here's a list of debug backtrace summary of the MockAction::filter function being called durging the whole execution of the Tests_Query_TaxQuery::test_tax_terms_should_limit_query method:

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Term->create_object, wp_insert_term, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Term->create_object, wp_insert_term, wp_unique_term_slug, term_exists, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Term->create_object, wp_insert_term, wp_unique_term_slug, term_exists, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_set_post_categories, wp_set_post_terms, wp_set_object_terms, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_set_post_categories, wp_set_post_terms, wp_set_object_terms, term_exists, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_transition_post_status, do_action('transition_post_status'), WP_Hook->do_action, WP_Hook->apply_filters, _update_term_count_on_transition_post_status, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_transition_post_status, do_action('transition_post_status'), WP_Hook->do_action, WP_Hook->apply_filters, _update_term_count_on_transition_post_status, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_transition_post_status, do_action('transition_post_status'), WP_Hook->do_action, WP_Hook->apply_filters, _update_term_count_on_transition_post_status, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_UnitTest_Factory_For_Thing->create, WP_UnitTest_Factory_For_Post->create_object, wp_insert_post, wp_transition_post_status, do_action('transition_post_status'), WP_Hook->do_action, WP_Hook->apply_filters, _update_term_count_on_transition_post_status, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, wp_set_object_terms, wp_get_object_terms, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, wp_set_object_terms, term_exists, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

Tests_Query_TaxQuery->test_tax_terms_should_limit_query, WP_Query->__construct, WP_Query->query, WP_Query->get_posts, WP_Tax_Query->get_sql, WP_Tax_Query->get_sql_clauses, WP_Tax_Query->get_sql_for_query, WP_Tax_Query->get_sql_for_clause, WP_Tax_Query->clean_query, WP_Tax_Query->transform_query, WP_Term_Query->query, WP_Term_Query->get_terms, apply_filters_ref_array('terms_pre_query'), WP_Hook->apply_filters, MockAction->filter

As it can be seen from the above list, the second trigger of the terms_pre_query filter is initiated from the wp_insert_post, rather than from the WP_Tax_Query which is the subject of the test ( per $filter_args[1][1]->request; ).

The same goes for the Tests_Query_TaxQuery::test_tax_terms_should_limit_query_to_one and Tests_Query_TaxQuery::test_hierarchical_taxonomies_do_not_limit_query

IMHO, the mocked filter should be hooked later in the method, so the desired SQL query is the first one in the events list.

Attachments (1)

57342.diff (2.7 KB) - added by david.binda 2 years ago.

Download all attachments as: .zip

Change History (5)

@david.binda
2 years ago

#1 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.2

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#3 @costdev
2 years ago

  • Keywords has-patch added

#4 @SergeyBiryukov
2 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 55460:

Tests: Make sure the correct query is tested for term limits in taxonomy queries.

By hooking into terms_pre_query after the fixture posts and terms are created but before the actual taxonomy query runs, we ensure that the correct SQL query from WP_Term_Query::get_terms() is tested for requested term limits, rather than the one initiated from wp_insert_post() or wp_insert_term() via term_exists().

Follow-up to [52921], [53037].

Props david.binda.
Fixes #57342.

Note: See TracTickets for help on using tickets.