_test_list_hierarchical_page() - improve clean-up

[birgire]

The _test_list_hierarchical_page() helper method modifies $_REQUEST and $GLOBALS, but does not unset it afterwards.

It affected a new test I was writing in a strange way, see #40188.

There might be other test methods, that modify globals, without a clean-up afterwards.

[birgire]

Clean up for $_REQUEST and $GLOBALS set in Tests_Admin_includesListTable::_test_list_hierarchical_page()

[swissspidy]

Thanks @birgire, good catch!

The only problem with 42044.patch​ is that this won't run when the assertion above fails. Execution is stopped there. Putting this after $output = ob_get_clean(); instead would prevent this.

[birgire]

Thank you @swissspidy, good point

I updated the patch in 42044.2.patch​

[birgire]

We have backupGlobals="false" by default.

I played with the ​@backupGlobals annotation on the test class:

/**
 * @group admin
 * @backupGlobals enabled
 */
class Tests_Admin_includesListTable extends WP_UnitTestCase {
        protected $backupGlobalsBlacklist = array( 'wpdb' );

where we blacklist the $wpdb object, as it seems it can't survive the serialization.

The blacklisting is explained ​here.

I ran the

tests/phpunit/tests/admin/includesListTable.php

three times, without the annotation:

Time: 1.43 seconds, Memory: 34.00MB
OK (14 tests, 32 assertions)

Time: 1.51 seconds, Memory: 34.00MB
OK (14 tests, 32 assertions)

Time: 1.39 seconds, Memory: 34.00MB
OK (14 tests, 32 assertions)

using the 42044.2.patch​ ] and three times, only with the @backupGlobals:

Time: 1.63 seconds, Memory: 38.00MB
OK (14 tests, 32 assertions)

Time: 1.59 seconds, Memory: 38.00MB
OK (14 tests, 32 assertions)

Time: 1.56 seconds, Memory: 38.00MB
OK (14 tests, 32 assertions)

So there's some overhead involved with it.

It didn't seem to have any effect to only enabled it on the problematic method:

/**
* Helper function to test the output of a page which uses `WP_Posts_List_Table`.
*
* @backupGlobals enabled
* @param array $args         Query args for the list of pages.
* @param array $expected_ids Expected IDs of pages returned.
*/
protected function _test_list_hierarchical_page( array $args, array $expected_ids ) {

with

FAILURES!
Tests: 14, Assertions: 31, Failures: 2.

[birgire]

ps: I just searched the trac and found 4 tickets where backupGlobals are discussed:

https://core.trac.wordpress.org/search?q=backupGlobals

in e.g. #28535 it was mentioned as worth investigating, but had previously caused issues and that using it would probably be to heavy for the whole project.

Not sure how the status is today, but I can imagine, from the above testing, that it would at least increase the memory usage and the run-time. That might overshadow the positive things it could have ;-)

ps: I found two files where the ​preserveGlobalState annotation is used:

tests/phpunit/tests/oembed/headers.php: * @preserveGlobalState disabled
tests/phpunit/tests/ajax/Response.php:   * @preserveGlobalState disabled

[SergeyBiryukov]

In 41929:

Build/Test Tools: Clean up $_REQUEST and $GLOBALS after modifying them in _test_list_hierarchical_page().

Props birgire.
Fixes #42044.