Skip to content

Tests: Add test coverage for media_sideload_image() - #13397

Draft
mukeshpanchal27 wants to merge 2 commits into
WordPress:trunkfrom
mukeshpanchal27:tests/49631-media-sideload-image
Draft

Tests: Add test coverage for media_sideload_image()#13397
mukeshpanchal27 wants to merge 2 commits into
WordPress:trunkfrom
mukeshpanchal27:tests/49631-media-sideload-image

Conversation

@mukeshpanchal27

Copy link
Copy Markdown
Member

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

Adds unit test coverage for media_sideload_image(), which has none today. This picks up where #190 left off and addresses the review feedback left there.

What is covered

The tests live in the existing Tests_Admin_IncludesMedia test case (tests/phpunit/tests/admin/includesMedia.php) rather than a new file, since that is where the rest of the wp-admin/includes/media.php coverage already sits.

  • _source_url post meta (the ticket's original focus, added in [47251] / #48164), including that the meta stores the URL as passed when it carries a query string, while the file name has the query string stripped.
  • Every $return_type, via a data provider: the default image tag, 'html', 'src', 'id', and an unrecognized value falling back to the image tag.
  • $desc being used as escaped alt text on the returned tag.
  • Attachment parentage — the attachment is attached to the given post.
  • Invalid URLs — a disallowed extension, an executable extension, no extension, and a partial extension (.jpgx, exercising the \b in the regex) each return an image_sideload_failed / "Invalid image URL." error before any HTTP request is made. An empty $file is covered separately.
  • The image_sideload_extensions filter — adding an extension (bmp) lets a matching URL through and still records _source_url, removing jpg/jpeg/jpe rejects one, and the filter receives the URL being sideloaded as its second argument.
  • Download failures — a transport WP_Error and a non-200 response are both returned to the caller, and no attachment is created.

Addressing the feedback on #190

  • "This teardown is not needed"wpTearDownAfterClass() is gone; posts are created per test with the factory and rolled back automatically. tear_down() now only calls remove_added_uploads(), which is still required (WP_UnitTestCase snapshots the uploads directory in set_up() but does not clear it), and matches Tests_Post_Attachments, Tests_Image_SiteIcon and others. The remove_filter() call is dropped — hooks are restored between tests.
  • "This would need to return more than just a response code" — the pre_http_request mock now returns a complete response array (headers, body, response, cookies, filename) so download_url()'s Content-Disposition and response-message handling operate on a realistic shape rather than a bare status code.
  • "Break this up into several test methods or use a data provider" — the single test_media_sideload_image() is now 12 focused test methods, three of which use data providers.
  • "Expected value should be passed as the first parameter" — all assertions are assertSame( $expected, $actual, $message ) with a failure message.

Also modernised along the way: set_up()/tear_down() instead of setUp()/tearDown(), assertSame() instead of assertEquals(), no assertRegExp()/assertInternalType() (removed in PHPUnit 9), array syntax per WPCS, and @covers ::media_sideload_image annotations.

Testing instructions

npm run test:php -- --filter Tests_Admin_IncludesMedia

42 tests, 129 assertions pass. Commenting out add_post_meta( $id, '_source_url', $file ); in media_sideload_image() fails 3 of the new tests, confirming they exercise the behaviour the ticket is about. The full --group media suite (876 tests) also passes.


🤖 Generated with Claude Code

Adds unit tests for `media_sideload_image()` in the existing
`Tests_Admin_IncludesMedia` test case, covering:

* The `_source_url` post meta introduced in [47251], including URLs with
  a query string.
* Each `$return_type`: the default image tag, `src`, `id`, and an
  unrecognized value falling back to the image tag.
* The description being used as escaped alt text.
* The attachment being attached to the given post.
* URLs without an allowed extension being rejected before any HTTP
  request is made, plus an empty URL.
* The `image_sideload_extensions` filter adding and removing extensions,
  and receiving the URL being sideloaded.
* Download failures (transport errors and non-200 responses) being
  returned to the caller without creating an attachment.

The HTTP request made by `download_url()` is short-circuited through
`pre_http_request`, which returns a complete response array and writes a
local fixture into the temporary file the request would have streamed to.

Props killua99, joemcgill, mukesh27.
See #49631, #48164.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@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.

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.

1 participant