Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RNMobile] Add API fetch test helper #50391

Merged
merged 2 commits into from
May 8, 2023
Merged

[RNMobile] Add API fetch test helper #50391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c5e0bf
Add setup api fetch test helper
fluiddot May 5, 2023
8379426
Use setup api fetch helper in Image block test cases
fluiddot May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions packages/block-library/src/image/test/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getEditorHtml,
render,
waitFor,
setupApiFetch,
} from 'test/helpers';
import { Image } from 'react-native';
import Clipboard from '@react-native-clipboard/clipboard';
Expand All @@ -22,9 +23,10 @@ import {
sendMediaUpload,
subscribeMediaUpload,
} from '@wordpress/react-native-bridge';
import { select } from '@wordpress/data';
import { select, dispatch } from '@wordpress/data';
import { store as editorStore } from '@wordpress/editor';
import { store as coreStore } from '@wordpress/core-data';
import apiFetch from '@wordpress/api-fetch';
import '@wordpress/jest-console';

/**
Expand All @@ -45,7 +47,15 @@ function mockGetMedia( media ) {
jest.spyOn( select( coreStore ), 'getMedia' ).mockReturnValue( media );
}

const apiFetchPromise = Promise.resolve( {} );
const FETCH_MEDIA = {
request: {
path: `/wp/v2/media/1?context=edit`,
},
response: {
source_url: 'https://cldup.com/cXyG__fTLN.jpg',
id: 1,
},
};

const clipboardPromise = Promise.resolve( '' );
Clipboard.getString.mockImplementation( () => clipboardPromise );
Expand All @@ -58,6 +68,18 @@ beforeAll( () => {
getSizeSpy.mockImplementation( ( _url, callback ) => callback( 300, 200 ) );
} );

beforeEach( () => {
// Mock media fetch requests
setupApiFetch( [ FETCH_MEDIA ] );

// Invalidate `getMedia` resolutions to allow requesting to the API the same media id
dispatch( coreStore ).invalidateResolutionForStoreSelector( 'getMedia' );
} );

afterEach( () => {
apiFetch.mockReset();
} );

afterAll( () => {
getBlockTypes().forEach( ( { name } ) => {
unregisterBlockType( name );
Expand All @@ -78,8 +100,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand All @@ -105,8 +127,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand All @@ -132,8 +154,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand Down Expand Up @@ -169,8 +191,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand Down Expand Up @@ -211,8 +233,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is not fetched via `getMedia` due to the presence of query parameters in the URL.
expect( apiFetch ).not.toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand All @@ -236,8 +258,8 @@ describe( 'Image Block', () => {
<figcaption class="wp-element-caption">Mountain</figcaption></figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand Down Expand Up @@ -267,8 +289,8 @@ describe( 'Image Block', () => {
</figure>
<!-- /wp:image -->`;
const screen = await initializeEditor( { initialHtml } );
// We must await the image fetch via `getMedia`
await act( () => apiFetchPromise );
// Check that image is fetched via `getMedia`
expect( apiFetch ).toHaveBeenCalledWith( FETCH_MEDIA.request );

const [ imageBlock ] = screen.getAllByLabelText( /Image Block/ );
fireEvent.press( imageBlock );
Expand Down
4 changes: 4 additions & 0 deletions test/native/integration-test-helpers/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ Changes the text of a RichText component.

Paste content into a RichText component.

### [`setupApiFetch`](https://github.com/WordPress/gutenberg/blob/HEAD/test/native/integration-test-helpers/setup-api-fetch.js)

Sets up the `apiFetch` library for testing by mocking request responses.

### [`setupCoreBlocks`](https://github.com/WordPress/gutenberg/blob/HEAD/test/native/integration-test-helpers/setup-core-blocks.js)

Registers all core blocks or a specific list of blocks before running tests, once the tests are run, all registered blocks are unregistered.
Expand Down
1 change: 1 addition & 0 deletions test/native/integration-test-helpers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export { openBlockSettings } from './open-block-settings';
export { selectRangeInRichText } from './rich-text-select-range';
export { typeInRichText } from './rich-text-type';
export { pasteIntoRichText } from './rich-text-paste';
export { setupApiFetch } from './setup-api-fetch';
export { setupCoreBlocks } from './setup-core-blocks';
export { setupMediaPicker } from './setup-media-picker';
export { setupMediaUpload } from './setup-media-upload';
Expand Down
50 changes: 50 additions & 0 deletions test/native/integration-test-helpers/setup-api-fetch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* WordPress dependencies
*/
import apiFetch from '@wordpress/api-fetch';

/**
* Sets up the `apiFetch` library for testing by mocking request responses.
*
* Example:
*
* const responses = [
* {
* request: {
* path: `/wp/v2/media/1?context=edit`,
* },
* response: {
* source_url: 'https://image-1.jpg',
* id: 1,
* },
* },
* {
* request: {
* path: `/wp/v2/media/2?context=edit`,
* },
* response: {
* source_url: 'https://image-2.jpg',
* id: 2,
* },
* },
* ];
* setupApiFetch( responses );
* ...
* expect( apiFetch ).toHaveBeenCalledWith( responses[1].request );
*
* @param {object[]} responses Array with the potential responses to return upon requests.
*
*/
export function setupApiFetch( responses ) {
apiFetch.mockImplementation( async ( options ) => {
const matchedResponse = responses.find(
( { request: { path, method } } ) => {
return (
path === options.path &&
( ! options.method || method === options.method )
);
}
);
return matchedResponse?.response;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be a benefit to logging an error here if there is no matchedResponse?

		if ( ! matchedResponse ) {
			throw new Error( `No response for path: ${ path }` );
		}

		return matchedResponse.response;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I originally added this condition but noticed the editor makes a set of requests always upon its initialization. This made me think that probably makes more sense to delegate checking what requests should be made or shouldn't to the test logic, instead here. In any case, we could add an option to make this function more restrictive and fail the test if an unexpected request is made. Maybe it's useful in some cases 🤔 .

For reference, the following requests are made when the editor is initialized:

  • /wp/v2/settings
  • /wp/v2/media
  • /wp/v2/pages
  • /wp/v2/block-patterns/patterns
  • /wp/v2/block-patterns/categories
  • /wp/v2/posts/post-id-<POST_GUID>

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for sharing the logic behind that decision, that makes sense to me.

In any case, we could add an option to make this function more restrictive and fail the test if an unexpected request is made. Maybe it's useful in some cases 🤔 .

I trust your judgement, but have a sense that the value would be relatively low. The current code looks good to me :) thanks again for expanding on the logic here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I trust your judgement, but have a sense that the value would be relatively low. The current code looks good to me :) thanks again for expanding on the logic here.

Ok, I'll merge the PR as-is, thanks!

} );
}