Build/Test Tools: Fail tests that make external HTTP requests outside the external-http group. - #13407
Draft
adimoldovan wants to merge 1 commit into
Draft
Conversation
… 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.
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
westonruter
reviewed
Sep 4, 2026
westonruter
left a comment
Member
There was a problem hiding this comment.
This is looking really good.
| * | ||
| * @var string[] | ||
| */ | ||
| protected $blocked_http_requests = array(); |
Member
There was a problem hiding this comment.
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[] |
Member
There was a problem hiding this comment.
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 ) { |
Member
There was a problem hiding this comment.
Suggested change
| public function block_external_http_request( $response, $args, $url ) { | |
| public function block_external_http_request( $response, array $args, string $url ) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 apre_http_requestfilter to every test outside the group. The filter runs atPHP_INT_MAX, so a mock that the test adds itself answers first. A request that gets that far is recorded and blocked with aWP_Error, andassert_post_conditions()then fails the test:The check runs only when
WP_RUN_CORE_TESTSis 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 onlyis_wp_error(). Failing from inside the filter instead is not safe: core has 193 PHPtryblocks with about 85catch ( Exception )orcatch ( Throwable )clauses, several of them on HTTP paths (oEmbed discovery, SimplePie, the AI client's PSR-18 wrapper), andAssertionFailedErrordescends fromRuntimeException.Tests_oEmbed_WpEmbed::test_run_shortcode_url_onlygets the missing annotation. It requestshttp://example.com/embed/foo, and it is the only test in trunk that the check catches. Its own assertions pass either way, becauserun_shortcode()falls back tomaybe_make_link()and produces the same link.Left out of this patch:
external-httpdoes 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, andTests_oEmbed_WpEmbed::test_autoembed_should_return_modified_content. The fourth,PluralFormsTest::test_locales_file_not_empty, is a false positive, becausedata_locales()downloads only whenGP_Localesis 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_apiunder multisite.get_theme_feature_list()returns the hardcoded list early when the user cannotinstall_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.set_up(). See #64963.WP_Http, such asfile_get_contents()orfsockopen().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
Start the environment:
npm run env:start && npm run env:install.Run the suite:
npm run test:php. Expect no failures (31,050 tests).Run the group:
npm run test:php -- --group external-http. Expect no failures (86 tests).Run the AJAX group:
npm run test:php -- --group ajax. Expect no failures (190 tests).Run multisite:
npm run test:php -- -c tests/phpunit/multisite.xml. Expect no failures (31,885 tests).Run multisite external HTTP:
npm run test:php -- -c tests/phpunit/multisite.xml --group external-http. Expect no failures (87 tests).Confirm the check catches a new offender. Save this as
tests/phpunit/tests/guardDemo.php: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-httpto the test and run it again: the default run now excludes it. Delete the file.Confirm the annotation in this patch is what keeps the suite green. Remove
@group external-httpfromTests_oEmbed_WpEmbed::test_run_shortcode_url_onlyand runnpm run test:php -- --filter Tests_oEmbed_WpEmbed::test_run_shortcode_url_only. Expect the same failure. Restore the annotation.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.