Skip to content

Commit

Permalink
Mobile unit tests: remove custom waitFor implementation (#46735)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@jsnajdr
jsnajdr committed Jun 21, 2023
1 parent 40d1c7d commit 71d2dc5
Show file tree
Hide file tree
Showing 32 changed files with 842 additions and 397 deletions.
42 changes: 15 additions & 27 deletions docs/contributors/code/react-native/integration-test-guide.md
View file
Expand Up @@ -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' );
```

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 );
```

Expand Down Expand Up @@ -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' );
```

Expand All @@ -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: {
Expand All @@ -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 );
```
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions packages/block-editor/src/components/block-mover/test/__snapshots__/index.native.js.snap
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down
21 changes: 1 addition & 20 deletions packages/block-editor/src/components/block-mover/test/index.native.js
View file
Expand Up @@ -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,
Expand Down
67 changes: 31 additions & 36 deletions packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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 ),
};
Expand All @@ -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
Expand All @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions packages/block-library/src/buttons/test/edit.native.js
View file
Expand Up @@ -81,7 +81,8 @@ describe( 'Buttons block', () => {
);

const incrementButton = await within( radiusStepper ).findByTestId(
'Increment'
'Increment',
{ hidden: true }
);
fireEvent( incrementButton, 'onPressIn' );

Expand Down Expand Up @@ -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', {
Expand Down

1 comment on commit 71d2dc5

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 71d2dc5.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5336566861
📝 Reported issues:

Please sign in to comment.