Mobile unit tests: remove custom waitFor implementation (WordPress#46735

) * useNestedSettingsUpdate: don't rerender if blockListSettings value changes * Mobile unit tests: remove custom waitFor implementation * Upgrade @testing-library/react-native to 12.1.2, fixes effect flushing bugs * Remove references to legacy screen.container, prefer snapshots * Spacer block: use waitForModalVisible * Image test: remove unneeded workarounds * Queries exclude elements hidden from accessibility, find them explicitly
pewresearch · Jul 13, 2023 · 081e092 · 081e092
1 parent c78498b
commit 081e092
Show file tree

Hide file tree

Showing 32 changed files with 842 additions and 397 deletions.
diff --git a/docs/contributors/code/react-native/integration-test-guide.md b/docs/contributors/code/react-native/integration-test-guide.md
@@ -121,42 +121,32 @@ const radiusSlider = getByTestId( 'Slider Border Radius' );
 
 Note that either a plain string or a regular expression can be passed into these queries. A regular expression is best for querying part of a string (e.g. any element whose accessibility label contains `Unsupported Block. Row 1`). Note that special characters such as `.` need to be escaped.
 
-### Use of `waitFor`
+### Use of `find` queries
 
-After rendering the components or firing an event, side effects might happen due to potential state updates so the element we’re looking for might not be yet rendered. In this case, we would need to wait for the element to be available and for this purpose, we can use the `waitFor` function, which periodically executes the provided callback to determine whether the element appeared or not.
+After rendering the components or firing an event, side effects might happen due to potential state updates so the element we’re looking for might not be yet rendered. In this case, we would need to wait for the element to be available and for this purpose, we can use the `find*` versions of query functions, which internally use `waitFor` and periodically check whether the element appeared or not.
 
 Here are some examples:
 
 ```js
-const mediaLibraryButton = await waitFor( () =>
-	getByText( 'WordPress Media Library' )
-);
+const mediaLibraryButton = await findByText( 'WordPress Media Library' );
 ```
 
 ```js
-const missingBlock = await waitFor( () =>
-	getByLabelText( /Unsupported Block\. Row 1/ )
-);
+const missingBlock = await findByLabelText( /Unsupported Block\. Row 1/ );
 ```
 
 ```js
-const radiusSlider = await waitFor( () =>
-	getByTestId( 'Slider Border Radius' )
-);
+const radiusSlider = await findByTestId( 'Slider Border Radius' );
 ```
 
-In most cases we’ll use the `waitFor` function, but it’s important to note that it should be restricted to those queries that actually require waiting for the element to be available.
-
-NOTE: The `react-native-testing-library` package provides the `query*` and `find*` functions for this purpose too, but we should avoid using them for now because there’s a [known issue](https://github.com/callstack/react-native-testing-library/issues/379) that would make the test fail.
+In most cases we’ll use the `find*` functions, but it’s important to note that it should be restricted to those queries that actually require waiting for the element to be available.
 
 ### `within` queries
 
 It’s also possible to query elements contained in other elements via the `within` function, here is an example:
 
 ```js
-const missingBlock = await waitFor( () =>
-	getByLabelText( /Unsupported Block\. Row 1/ )
-);
+const missingBlock = await findByLabelText( /Unsupported Block\. Row 1/ );
 const translatedTableTitle = within( missingBlock ).getByText( 'Tabla' );
 ```
 
@@ -236,7 +226,7 @@ Here is an example of how to insert a Paragraph block:
 
 ```js
 // Open the inserter menu
-fireEvent.press( await waitFor( () => getByLabelText( 'Add block' ) ) );
+fireEvent.press( await findByLabelText( 'Add block' ) );
 
 const blockList = getByTestId( 'InserterUI-Blocks' );
 // onScroll event used to force the FlatList to render all items
@@ -249,7 +239,7 @@ fireEvent.scroll( blockList, {
 } );
 
 // Insert a Paragraph block
-fireEvent.press( await waitFor( () => getByText( `Paragraph` ) ) );
+fireEvent.press( await findByText( `Paragraph` ) );
 ```
 
 ### Open block settings
@@ -259,7 +249,7 @@ The block settings can be accessed by tapping the "Open Settings" button after s
 ```js
 fireEvent.press( block );
 
-const settingsButton = await waitFor( () => getByLabelText( 'Open Settings' ) );
+const settingsButton = await findByLabelText( 'Open Settings' );
 fireEvent.press( settingsButton );
 ```
 
@@ -301,9 +291,7 @@ fireEvent.scroll( blockList, {
 Sliders found in bottom sheets should be queried using their `testID`:
 
 ```js
-const radiusSlider = await waitFor( () =>
-	getByTestId( 'Slider Border Radius' )
-);
+const radiusSlider = await findByTestId( 'Slider Border Radius' );
 fireEvent( radiusSlider, 'valueChange', '30' );
 ```
 
@@ -314,8 +302,8 @@ Note that a slider’s `testID` is "Slider " + label. So for a slider with a lab
 One caveat when adding blocks is that if they contain inner blocks, these inner blocks are not rendered. The following example shows how we can make a Buttons block render its inner Button blocks (assumes we’ve already obtained a reference to the Buttons block as `buttonsBlock`):
 
 ```js
-const innerBlockListWrapper = await waitFor( () =>
-	within( buttonsBlock ).getByTestId( 'block-list-wrapper' )
+const innerBlockListWrapper = await within( buttonsBlock ).findByTestId(
+	'block-list-wrapper'
 );
 fireEvent( innerBlockListWrapper, 'layout', {
 	nativeEvent: {
@@ -325,8 +313,8 @@ fireEvent( innerBlockListWrapper, 'layout', {
 	},
 } );
 
-const buttonInnerBlock = await waitFor( () =>
-	within( buttonsBlock ).getByLabelText( /Button Block\. Row 1/ )
+const buttonInnerBlock = await within( buttonsBlock ).findByLabelText(
+	/Button Block\. Row 1/
 );
 fireEvent.press( buttonInnerBlock );
 ```

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -115,7 +115,7 @@
 		"@storybook/react": "6.5.7",
 		"@testing-library/jest-dom": "5.16.5",
 		"@testing-library/react": "13.4.0",
-		"@testing-library/react-native": "11.3.0",
+		"@testing-library/react-native": "12.1.2",
 		"@testing-library/user-event": "14.4.3",
 		"@types/classnames": "2.3.1",
 		"@types/eslint": "7.28.0",

diff --git a/packages/block-editor/src/components/block-mover/test/__snapshots__/index.native.js.snap b/packages/block-editor/src/components/block-mover/test/__snapshots__/index.native.js.snap
@@ -42,7 +42,7 @@ exports[`Block Mover Picker moving blocks moves blocks up and down 1`] = `
 <!-- /wp:spacer -->"
 `;
 
-exports[`Block Mover Picker should match snapshot 1`] = `
+exports[`Block Mover Picker should render without crashing and match snapshot 1`] = `
 [
   <View>
     <View
@@ -76,7 +76,7 @@ exports[`Block Mover Picker should match snapshot 1`] = `
     >
       <View
         collapsable={false}
-        handlerTag={3}
+        handlerTag={1}
         handlerType="LongPressGestureHandler"
         onGestureHandlerEvent={[Function]}
         onGestureHandlerStateChange={[Function]}
@@ -144,7 +144,7 @@ exports[`Block Mover Picker should match snapshot 1`] = `
     >
       <View
         collapsable={false}
-        handlerTag={4}
+        handlerTag={2}
         handlerType="LongPressGestureHandler"
         onGestureHandlerEvent={[Function]}
         onGestureHandlerStateChange={[Function]}

diff --git a/packages/block-editor/src/components/block-mover/test/index.native.js b/packages/block-editor/src/components/block-mover/test/index.native.js
@@ -24,26 +24,7 @@ import { registerCoreBlocks } from '@wordpress/block-library';
 import { BlockMover } from '../index';
 
 describe( 'Block Mover Picker', () => {
-	it( 'renders without crashing', () => {
-		const props = {
-			isFirst: false,
-			isLast: true,
-			canMove: true,
-			numberOfBlocks: 2,
-			firstIndex: 1,
-
-			onMoveDown: jest.fn(),
-			onMoveUp: jest.fn(),
-			onLongPress: jest.fn(),
-
-			rootClientId: '',
-			isStackedHorizontally: true,
-		};
-		const screen = render( <BlockMover { ...props } /> );
-		expect( screen.container ).toBeTruthy();
-	} );
-
-	it( 'should match snapshot', () => {
+	it( 'should render without crashing and match snapshot', () => {
 		const props = {
 			isFirst: false,
 			isLast: true,

diff --git a/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js b/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js
@@ -3,7 +3,6 @@
  */
 import { useLayoutEffect, useMemo } from '@wordpress/element';
 import { useSelect, useDispatch, useRegistry } from '@wordpress/data';
-import isShallowEqual from '@wordpress/is-shallow-equal';
 
 /**
  * Internal dependencies
@@ -52,13 +51,11 @@ export default function useNestedSettingsUpdate(
 	const { updateBlockListSettings } = useDispatch( blockEditorStore );
 	const registry = useRegistry();
 
-	const { blockListSettings, parentLock } = useSelect(
+	const { parentLock } = useSelect(
 		( select ) => {
 			const rootClientId =
 				select( blockEditorStore ).getBlockRootClientId( clientId );
 			return {
-				blockListSettings:
-					select( blockEditorStore ).getBlockListSettings( clientId ),
 				parentLock:
 					select( blockEditorStore ).getTemplateLock( rootClientId ),
 			};
@@ -83,14 +80,16 @@ export default function useNestedSettingsUpdate(
 		prioritizedInserterBlocks
 	);
 
+	const _templateLock =
+		templateLock === undefined || parentLock === 'contentOnly'
+			? parentLock
+			: templateLock;
+
 	useLayoutEffect( () => {
 		const newSettings = {
 			allowedBlocks: _allowedBlocks,
 			prioritizedInserterBlocks: _prioritizedInserterBlocks,
-			templateLock:
-				templateLock === undefined || parentLock === 'contentOnly'
-					? parentLock
-					: templateLock,
+			templateLock: _templateLock,
 		};
 
 		// These values are not defined for RN, so only include them if they
@@ -116,41 +115,37 @@ export default function useNestedSettingsUpdate(
 			newSettings.__experimentalDirectInsert = __experimentalDirectInsert;
 		}
 
-		if ( ! isShallowEqual( blockListSettings, newSettings ) ) {
-			// Batch updates to block list settings to avoid triggering cascading renders
-			// for each container block included in a tree and optimize initial render.
-			// To avoid triggering updateBlockListSettings for each container block
-			// causing X re-renderings for X container blocks,
-			// we batch all the updatedBlockListSettings in a single "data" batch
-			// which results in a single re-render.
-			if ( ! pendingSettingsUpdates.get( registry ) ) {
-				pendingSettingsUpdates.set( registry, [] );
-			}
-			pendingSettingsUpdates
-				.get( registry )
-				.push( [ clientId, newSettings ] );
-			window.queueMicrotask( () => {
-				if ( pendingSettingsUpdates.get( registry )?.length ) {
-					registry.batch( () => {
-						pendingSettingsUpdates
-							.get( registry )
-							.forEach( ( args ) => {
-								updateBlockListSettings( ...args );
-							} );
-						pendingSettingsUpdates.set( registry, [] );
-					} );
-				}
-			} );
+		// Batch updates to block list settings to avoid triggering cascading renders
+		// for each container block included in a tree and optimize initial render.
+		// To avoid triggering updateBlockListSettings for each container block
+		// causing X re-renderings for X container blocks,
+		// we batch all the updatedBlockListSettings in a single "data" batch
+		// which results in a single re-render.
+		if ( ! pendingSettingsUpdates.get( registry ) ) {
+			pendingSettingsUpdates.set( registry, [] );
 		}
+		pendingSettingsUpdates
+			.get( registry )
+			.push( [ clientId, newSettings ] );
+		window.queueMicrotask( () => {
+			if ( pendingSettingsUpdates.get( registry )?.length ) {
+				registry.batch( () => {
+					pendingSettingsUpdates
+						.get( registry )
+						.forEach( ( args ) => {
+							updateBlockListSettings( ...args );
+						} );
+					pendingSettingsUpdates.set( registry, [] );
+				} );
+			}
+		} );
 	}, [
 		clientId,
-		blockListSettings,
 		_allowedBlocks,
 		_prioritizedInserterBlocks,
+		_templateLock,
 		__experimentalDefaultBlock,
 		__experimentalDirectInsert,
-		templateLock,
-		parentLock,
 		captureToolbars,
 		orientation,
 		updateBlockListSettings,

diff --git a/packages/block-library/src/buttons/test/edit.native.js b/packages/block-library/src/buttons/test/edit.native.js
@@ -81,7 +81,8 @@ describe( 'Buttons block', () => {
 			);
 
 			const incrementButton = await within( radiusStepper ).findByTestId(
-				'Increment'
+				'Increment',
+				{ hidden: true }
 			);
 			fireEvent( incrementButton, 'onPressIn' );
 
@@ -181,7 +182,7 @@ describe( 'Buttons block', () => {
 			expect( addBlockHerePlaceholders.length ).toBe( 0 );
 
 			// Add a new Button block
-			fireEvent.press( await screen.findByText( 'Button' ) );
+			fireEvent.press( within( blockList ).getByText( 'Button' ) );
 
 			// Get new button
 			const secondButtonBlock = await getBlock( screen, 'Button', {