External Media: Add API unit tests #15988
Conversation
obenland
added
[Feature] WPCOM API
[Feature] WP REST API
[Extension] External Media
This is an automated check which relies on E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-15988 |
|
@jeherve Attempting to make #15717 somewhat easier to review, I split out the API endpoint and added unit test (to make it easier to test without a client). I've not had much luck creating a test for the image sideload endpoint. Not sure how to best mock an external image. Do you happen to have any advice or pointers there? Or is it fine to skip for now? How can I tell Codeclimate about the added test coverage? |
obenland
added
[Status] Needs Review
obenland
force-pushed
the
add/external-media-endpoint
branch
from
10ace2e
to
8589b5d
Compare
|
Closing in favor of merging #15717. |
obenland
force-pushed
the
add/external-media-endpoint
branch
from
8589b5d
to
f91b759
Compare
|
Caution: This PR has changes that must be merged to WordPress.com |
obenland
changed the title
obenland
added
[Status] Needs Team Review
and removed
[Status] Needs Review
|
@jeherve How do you feel about introducing skipped tests? |
obenland
force-pushed
the
feature/external-media
branch
from
ed6f1e9
to
14ab20d
Compare
obenland
force-pushed
the
add/external-media-endpoint
branch
from
f91b759
to
32765ff
Compare
I think that'd be fine for now, but maybe add a comment in there explaining the things you've tried and how that turned out, or maybe a link to a comment here with more details? This may help the next person who looks at this, and save them some time in their investigation? |
|
I was able to make the image copy test work with bunch of filter modifications. Feels fragile, but maybe that's just how it needs to be when we're trying to test an endpoint for a remote image with a local file. 🤷♂️ All tests are 🟢 now, though! |
tests/php/core-api/wpcom-endpoints/test_class-wpcom-rest-api-v2-endpoint-external-media.php.php
Outdated
Show resolved
Hide resolved
* [not verified] Add External media endpoint * [not verified] Add unit tests * [not verified] Add class coverage info * Simplify schema * Fix image copy test * Use local image * Remove duplicate file ending Props @frontdevde.
* External Media: Render Media after authentication (#16044) See #15717 (review) * [not verified] Handle 401 responses (#16041) See #15717 (review) * External Media: Better reflect accepted media types (#16071) * External Media: Account for multiple images (#16077) See #16071 (comment) * External Media: Fix Modal focus loss on arrow keypress (#16055) Co-authored-by: Marek Hrabe <marekhrabe@me.com> * External Media: Add API unit tests (#15988) * [not verified] Add External media endpoint * [not verified] Add unit tests * [not verified] Add class coverage info * Simplify schema * Fix image copy test * Use local image * Remove duplicate file ending Props @frontdevde. * External Media: Improve Insert Flow (#16081) * [not verified] # This is a combination of 5 commits. External Media: Improve Insert Flow See #15867 Remove unused import Lock pointer events when copying This prevents a UX issue where user can still interact with the modal content while the "Inserting..." request is pending. Announce Inserting action along with alt text of selected images. Update modal headers to reflect status Props @jancavan. * External Media: Fix mobile modal size (#16098) * External Media: Improve Insert Flow See #15867 * Remove unused import * Lock pointer events when copying This prevents a UX issue where user can still interact with the modal content while the "Inserting..." request is pending. * Announce Inserting action along with alt text of selected images. * Fix mobile mobile width to take up the full available modal space * Change is-copying to modifier instead of element to follow BEM syntax Co-authored-by: Konstantin Obenland <obenland@gmx.de> Co-authored-by: Bart <bartlomiej.kalisz@gmail.com> * External Media: Loading and disabled states for the copying state (#16103) * disable pagination while loading * fade out all images when copying * disable event handlers and mark as disabled when copying * disable pexels search when copying * disabled google filters when copying * add text to copying indicator * disable google photos view selector when copying * [not verified] remove debugging condition * Pass functions not booleans to event emitters Co-authored-by: Konstantin Obenland <obenland@gmx.de> * Removed :not() copying css state since they are the same modal now * Reset focus to the modal after clicking Insert. Otherwise focus is reset to the body and focus needs to stay trapped within the modal. * Cosmetically rearranging code order * External Media: Screen Reader accessible states for Insert flow (#16113) * Add visually hidden description for the external media modal to give improved screen reader instructions and communicate the copying state * [not verified] Remove title attribute from grid images * [not verified] Move Inserting Image... copy to before the image so it is announced first. * Separate-out description and element id Helps with keeping describedby id in sync Co-authored-by: Konstantin Obenland <obenland@gmx.de> Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com> Co-authored-by: Bart <bartlomiej.kalisz@gmail.com> Co-authored-by: Marek Hrabe <marekhrabe@me.com> * External Media: Restore focus on modal close (#16102) * External Media: Disable for existing galleries (#16176) * External Media: Disable for existing galleries * Update comment * External Media: Improve upload permission check (#16185) * Use `value` to disable for existing galleries `addToGallery` is not used by all gallery-type blocks, like tiled-gallery or slideshow. Props @jeherve. Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com> Co-authored-by: Marek Hrabe <marekhrabe@me.com> Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
Adds unit tests for external media API endpoints.
Changes proposed in this Pull Request:
Is this a new feature or does it add/remove features to an existing part of Jetpack?
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
To run unit tests:
/copyendpoint test is being skipped at the moment.phpunit --filter WP_Test_WPCOM_REST_API_V2_Endpoint_External_Media.Proposed changelog entry for your changes:
Not necessary