WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 5 days ago

#49631 assigned task (blessed)

Test coverage for media_sideload_image() _source_url meta

Reported by: antpb Owned by: killua99
Milestone: Future Release Priority: normal
Severity: normal Version: 5.4
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Since #48164 when an image is sideloaded we store the original attachment URL in _source_url. There should be test coverage across this feature that validates that the meta is being properly stored.

@pbiron mentioned in the Media meeting where this was discussed that there are currently no existing tests around the media_sideload_image() function. This would be a great start to getting that covered.

@joemcgill also mentioned that "You may want to consider writing the test in a way that sideloads an image that is already available on the test site."

Link to media meeting where this took place: https://wordpress.slack.com/archives/C02SX62S6/p1584022312111000

Change History (15)

#1 @antpb
8 months ago

  • Summary changed from Test coverage for to Test coverage for media_sideload_image() _source_url meta

#2 @joemcgill
8 months ago

  • Owner set to killua99
  • Status changed from new to assigned

Assigning to @killua99 for a first pass.

Note that you can write a test using WP_TESTS_DOMAIN to sideload from the site set up during testing, rather than an external source.

This ticket was mentioned in Slack in #core by killua99. View the logs.


7 months ago

#4 @killua99
7 months ago

  • Keywords has-patch added; needs-patch removed

I wrote the first pass.

The source is not coming from the site setup since is a mockup with http://example.org/ is hard to sideload even creating the attachment upload, the request to download fails and is impossible to test it in that way. I found something that could be useful for now. Perhaps add some TODO notes on the test to improve the real sideload from a mockup site that could be accessible on travis.

This ticket was mentioned in Slack in #core-media by killua99. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-restapi by killua99. View the logs.


7 months ago

#9 @killua99
7 months ago

Ok now the latest patch has a full mockup to the HTTP request. No real external assets is been use rather than the DIR_TESTDATA and that's ok. Hope this test cover the scope (plus some extras)

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


5 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


4 months ago

#12 @antpb
4 months ago

  • Milestone changed from 5.5 to 5.6

Hello! This was discussed in the recent Media Component meeting. We are currently in Beta 1 for 5.5 and might need to move this to the 5.6 milestone. If anyone wants to finish this up, I think it's fair to include in the next beta cycle but we probably don't want to go too far into the future beta releases.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 days ago

#14 @antpb
5 days ago

  • Milestone changed from 5.6 to Future Release

Hello @killua99 ! Thanks for the work on these tests! In the recent Media meeting, the team agreed we should mark this for Future Release. If you think this can be finished before Beta 2 of 5.6 please feel free to let us know and we'll move it back into the milestone. :)

#15 @antpb
5 days ago

  • Type changed from defect (bug) to task (blessed)
Note: See TracTickets for help on using tickets.