Skip to content

Build/Test Tools: Fail tests that make external HTTP requests outside the external-http group. - #13407

Draft
adimoldovan wants to merge 1 commit into
WordPress:trunkfrom
adimoldovan:63083-tests-making-external-requests
Draft

Build/Test Tools: Fail tests that make external HTTP requests outside the external-http group.#13407
adimoldovan wants to merge 1 commit into
WordPress:trunkfrom
adimoldovan:63083-tests-making-external-requests

Conversation

@adimoldovan

@adimoldovan adimoldovan commented Sep 4, 2026

Copy link
Copy Markdown

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

Fails any test that makes an external HTTP request without @group external-http.

WP_UnitTestCase_Base::set_up() adds a pre_http_request filter to every test outside the group. The filter runs at PHP_INT_MAX, so a mock that the test adds itself answers first. A request that gets that far is recorded and blocked with a WP_Error, and assert_post_conditions() then fails the test:

This test made an external HTTP request but is not in the `external-http` group.
Add `@group external-http` to it, or mock the request with the `pre_http_request` filter.
- http://example.com/embed/foo

The check runs only when WP_RUN_CORE_TESTS is set, so plugin and theme suites that build on the core test suite keep working.

Blocking the request and failing in assert_post_conditions() are both needed. Blocking alone would silently satisfy a test that asserts only is_wp_error(). Failing from inside the filter instead is not safe: core has 193 PHP try blocks with about 85 catch ( Exception ) or catch ( Throwable ) clauses, several of them on HTTP paths (oEmbed discovery, SimplePie, the AI client's PSR-18 wrapper), and AssertionFailedError descends from RuntimeException.

Tests_oEmbed_WpEmbed::test_run_shortcode_url_only gets the missing annotation. It requests http://example.com/embed/foo, and it is the only test in trunk that the check catches. Its own assertions pass either way, because run_shortcode() falls back to maybe_make_link() and produces the same link.

Left out of this patch:

  • The second check in the ticket, that a test in external-http does make a request. Four tests fail it today. Three carry a wrong annotation: Tests_HTTP_Functions::test_get_cookie_host_only, Tests_oEmbed_WpEmbed::test_shortcode_should_return_empty_string_for_missing_url, and Tests_oEmbed_WpEmbed::test_autoembed_should_return_modified_content. The fourth, PluralFormsTest::test_locales_file_not_empty, is a false positive, because data_locales() downloads only when GP_Locales is absent and an earlier test in the class already loaded it. That check needs an opt-out, so it belongs in its own patch.
  • Tests_Admin_IncludesTheme::test_get_theme_featured_list_api under multisite. get_theme_feature_list() returns the hardcoded list early when the user cannot install_themes, which a plain administrator cannot do on multisite, so the test asserts nothing about the API and still passes. The same second check found it.
  • Requests from data providers, which run before set_up(). See #64963.
  • Requests that do not use WP_Http, such as file_get_contents() or fsockopen().

getGroups() works on every PHPUnit version the supported branches use, but PHPUnit 10 removes it. That migration will have to touch this one condition.

Testing Instructions

  1. Start the environment: npm run env:start && npm run env:install.

  2. Run the suite: npm run test:php. Expect no failures (31,050 tests).

  3. Run the group: npm run test:php -- --group external-http. Expect no failures (86 tests).

  4. Run the AJAX group: npm run test:php -- --group ajax. Expect no failures (190 tests).

  5. Run multisite: npm run test:php -- -c tests/phpunit/multisite.xml. Expect no failures (31,885 tests).

  6. Run multisite external HTTP: npm run test:php -- -c tests/phpunit/multisite.xml --group external-http. Expect no failures (87 tests).

  7. Confirm the check catches a new offender. Save this as tests/phpunit/tests/guardDemo.php:

    <?php
    class Tests_Guard_Demo extends WP_UnitTestCase {
        public function test_external_request() {
            wp_remote_get( 'http://example.com/' );
            $this->assertTrue( true );
        }
    }

    Run npm run test:php -- --filter Tests_Guard_Demo. Expect one failure that names the URL and tells you to add the group. Add @group external-http to the test and run it again: the default run now excludes it. Delete the file.

  8. Confirm the annotation in this patch is what keeps the suite green. Remove @group external-http from Tests_oEmbed_WpEmbed::test_run_shortcode_url_only and run npm run test:php -- --filter Tests_oEmbed_WpEmbed::test_run_shortcode_url_only. Expect the same failure. Restore the annotation.

  9. Check the coding standards: composer lint tests/phpunit/includes/abstract-testcase.php tests/phpunit/tests/oembed/WpEmbed.php.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: The report-only audit of the test suite, the implementation, and this description.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

… the `external-http` group.

Add a `pre_http_request` filter in `WP_UnitTestCase_Base` that blocks an external HTTP
request from a test outside the `external-http` group, then fails that test. The filter
runs at `PHP_INT_MAX`, so a mock added by the test answers first. The check runs only
when `WP_RUN_CORE_TESTS` is set.

Add `@group external-http` to `Tests_oEmbed_WpEmbed::test_run_shortcode_url_only`, the
only test in trunk that the check catches.
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@westonruter westonruter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good.

*
* @var string[]
*/
protected $blocked_http_requests = array();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected $blocked_http_requests = array();
protected array $blocked_http_requests = array();

/**
* URLs of blocked external HTTP requests made during the current test.
*
* @var string[]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @var string[]
* @var list<non-falsy-string>

* @param string $url The request URL.
* @return array|WP_Error The preemptive response, or an error for a blocked request.
*/
public function block_external_http_request( $response, $args, $url ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function block_external_http_request( $response, $args, $url ) {
public function block_external_http_request( $response, array $args, string $url ) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants