Tests: Mock REST API remote requests

[hellofromTonya]

Recent timeout failures (​see GitHub failed job) of specific REST API tests show some tests are not mocking the remote requests. These requests can be mocked with a callback hooked into 'pre_http_request' filter.

Each test should be reviewed to determine:

• If a remote request to a live API endpoint is being made
• If those specific requests should/can be mocked
• and then mocks created for each in its context of the condition under test

Ideally a mocking strategy could be created to abstract the heavy lifting and make it easier for these tests to mock the requests.

Recent timeout failures (​see this job) are due to remote requests not being mocked. This patch mocks the specific failing tests, though each test will need to be investigated separately for mocking strategy.

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

@TimothyBJacobs This PR mocks the 2 failing tests we saw ​https://wordpress.slack.com/archives/C02RQBWTW/p1636603357442100

❗ A run of the PHPUnit Tests workflow has failed for the trunk branch.

REST API: Add /wp/v2/block-navigation-areas endpoint

More details: https://github.com/WordPress/wordpress-develop/actions/runs/1447262178.

The ticket I opened takes the slow road to fix each of these as found.

Can you do a confidence check of the mocks?

[hellofromTonya]

In 52137:

Build/Test Tools: Mock remote request for WP_REST_Block_Directory_Controller::get_items().

Instead of hitting the live API, this commit mocks the remote request.

Follow-up to [48242].

Props hellofromTonya, noisysocks, sergeybiryukov, TimothyBlynJacobs.
Fixes #54420.

[hellofromTonya]

Whoopsie, didn't mean to close it. Commit should have been See not Fixes.

[hellofromTonya]

In 52138:

Build/Test Tools: Mock remote request for unknown plugin in WP_REST_Plugins_Controller::create_item().

Instead of hitting the live API, this commit mocks the remote request when testing creating an item that's an unknown plugin.

Follow-up to [48242].

Props hellofromTonya, noisysocks, sergeybiryukov, TimothyBlynJacobs.
See #54420.

Committed via changesets https://core.trac.wordpress.org/changeset/52137 and https://core.trac.wordpress.org/changeset/52138.

[hellofromTonya]

Updating keywords as ready patches were committed.

[SergeyBiryukov]

Thanks for the ticket and the fixes!

Just noting that [52137] addresses the failure in test_get_items(), but not in test_get_items_no_results(), which also ​failed in the same job and apparently needs a similar fix.

Should we move the pre_http_request filter into a helper method to be reused there?

[hellofromTonya]

Replying to SergeyBiryukov:

Thanks for the ticket and the fixes!

Just noting that [52137] addresses the failure in test_get_items(), but not in test_get_items_no_results(), which also ​failed in the same job and apparently needs a similar fix.

Should we move the pre_http_request filter into a helper method to be reused there?

Whoops, missed that one. Thanks for the ping @SergeyBiryukov. To keep it simply, I agree with adding a helper method. Doing it now.

Mocks remote request in WP_REST_Block_Directory_Controller_Test:: test_get_items_no_results().

To reduce redundant code, moves the mock to a helper method that accepts expected request with those specific args to override.

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

[SergeyBiryukov]

Carrying this over from comment:37:ticket:51669:

Got this again when running multisite tests:

There was 1 error:

1) WP_REST_Plugins_Controller_Test::test_create_item
An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)

/var/www/build/wp-admin/includes/plugin-install.php:183
/var/www/build/wp-includes/rest-api/endpoints/class-wp-rest-plugins-controller.php:288
/var/www/build/wp-includes/rest-api/class-wp-rest-server.php:1139
/var/www/build/wp-includes/rest-api/class-wp-rest-server.php:985
/var/www/tests/phpunit/includes/spy-rest-server.php:57
/var/www/build/wp-includes/rest-api.php:479
/var/www/tests/phpunit/tests/rest-api/rest-plugins-controller.php:379

Apparently [49913] and [49951] didn't cover the external HTTP request in plugins_api(). Looks like the plugins_api() call in these tests is by design, so they should probably use the external-http group and the skipTestOnTimeout() method, or mock the response.

[SergeyBiryukov]

Replying to SergeyBiryukov:

Carrying this over from comment:37:ticket:51669:

On second thought, it looks like [52138] may have already addressed that :)

[hellofromTonya]

In 52146:

Build/Test Tools: Mock no results remote request in WP_REST_Block_Directory_Controller:: get_items().

• Refactors the mock logic to a helper function for reuse in multiple tests.
• Mocks the remote request in WP_REST_Block_Directory_Controller_Test:: test_get_items_no_results() using the mock helper.

Follow-up to [48242], [52137].

Props hellofromTonya, sergeybiryukov.
See #54420.

Committed via changeset https://core.trac.wordpress.org/changeset/52146.

[hellofromTonya]

Tests in Tests_Admin_IncludesTheme::test_get_theme_featured_list_api() timed out in this ​CI Job, i.e. Run external HTTP tests. Noting it here as another test for mocking the HTTP remote request.

[SergeyBiryukov]

Replying to hellofromTonya:

Tests in Tests_Admin_IncludesTheme::test_get_theme_featured_list_api() timed out in this ​CI Job, i.e. Run external HTTP tests. Noting it here as another test for mocking the HTTP remote request.

This was added in [39906] / #28121 to make sure that the list of theme features pulled from the WordPress.org API returns the expected data structure. Wouldn't mocking this response defeat the purpose of the test?

This raises an interesting point: when mocking the response, we should consider the intent of the test, and whether it was specifically written to protect from unexpected changes in the WordPress.org API response.

This may very well also apply to the tests changed in [52137] and [52138]. Note that the latter previously had a comment: // This will hit the live API. Seems worth investigating whether that live request was intentional. Perhaps a better option would be to add them to the external-http group and ignore any timeouts? See the WP_UnitTestCase_Base::skipTestOnTimeout() method and the tests using it for example.

[SergeyBiryukov]

In 52382:

Tests: Mock the HTTP request response in download_url() tests.

This aims to speed up the tests and minimize unrelated failures by avoiding an unnecessary external HTTP request, while still performing the intended functionality checks.

Update similar helpers in some other tests to use more consistent terminology.

Follow-up to [37907], [46175], [51626].

See #54420, #53363.

[hellofromTonya]

With RC1 tomorrow, moving to 6.1.

[audrasjb]

As per today's bug scrub:
Unfortunately, this ticket wasn’t updated since it was moved from milestone 6.0 to 6.1. Since WP 6.1 RC1 is coming next Tuesday, let’s move it to Future Release.

Feel free tom move it to the next milestone (6.2).

[hellofromTonya]

Moving to Future Release until there's consensus on dev feedback. Also changing the type to defect instead of blessed task.