Make WordPress Core

Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#58776 closed defect (bug) (fixed)

Main query global not reset after tests

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

Several core tests added in the past few releases of WordPress (mostly related to lazy-loading) have been making use of the main query global ($wp_the_query, different from $wp_query) and modifying it in tests.

It was assumed that that global was being reset between tests similar to other query loop-related globals, but in https://github.com/WordPress/wordpress-develop/pull/4799#issuecomment-1629114720 it was discovered that the $wp_the_query global is not reset between tests, which is unexpected behavior and caused problems with the tests in that PR.

This should be fixed so that the global is reset similar to the other related globals like $wp_query and $wp. Similar to the regular WordPress bootstrap process in wp-settings.php, the test suite should ensure that $wp_query and $wp_the_query point to the same WP_Query instance.

Change History (7)

This ticket was mentioned in PR #4823 on WordPress/wordpress-develop by @flixos90.


12 months ago
#1

  • Keywords has-unit-tests added

This PR fixes a problem in the WordPress PHPUnit test suite that was initially spotted in https://github.com/WordPress/wordpress-develop/pull/4799#issuecomment-1629114720: The test base class has not been resetting the main query global $wp_the_query, which results in unexpected test behavior where some tests can pollute the global scope for subsequent tests. Since the WordPress PHPUnit test suite generally resets crucial globals related to the post loop, core developers should be able to expect that the $wp_the_query global is covered by that mechanism as well.

Similar to how wp-settings.php sets them up for the actual WordPress bootstrap process, the $wp_the_query and $wp_query global should have the same values / point to the same WP_Query instance before each test run.

Trac ticket: https://core.trac.wordpress.org/ticket/58776

#2 @flixos90
12 months ago

https://github.com/WordPress/wordpress-develop/pull/4823 addresses this bug in a simple way. All existing tests still pass without any modifications, so this should be a straightforward fix.

With that fix, the new tests that are being added in https://github.com/WordPress/wordpress-develop/pull/4799 should also pass as expected. cc @joemcgill

@flixos90 commented on PR #4823:


12 months ago
#3

After a broader search of the WP core unit test suite, I noticed that there was in fact already one workaround that was only needed because of this problem, in the tests/phpunit/tests/query/isTerm.php file's set_up() method. I removed this in https://github.com/WordPress/wordpress-develop/pull/4823/commits/3c314c9edc69b618a540944aa159fc8aa2422a95, as it is unnecessary with this more holistic fix. cc @joemcgill

#4 @joemcgill
12 months ago

  • Keywords commit added

This PR looks good to me. Would be nice to get it committed prior to the final beta release.

#5 @joemcgill
12 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56212:

Build/Test Tools: Reset main query object after each test.

This resets the main query variable, $wp_the_query during the WP_UnitTestCase_Base::tear_down method that runs after each PHPUnit test. This ensures any changes to the main query object is reset after each test to avoid cross contamination between tests, similar to how other globals are reset.

Props flixos90, spacedmonkey, joemcgill.
Fixes #58776.

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


12 months ago

Note: See TracTickets for help on using tickets.