Skip to content

Commit

Permalink
Fix native test timeouts caused by combining fake timers and setImmed…
Browse files Browse the repository at this point in the history
…iate (#37715)

* Upgrade to @testing-library/[email protected]

* Clean up fake timer usage after tests

This may be unnecessary, but avoid potential issues where fake timers
are unexpectedly used and cause breakages in other tests.

* Enable combination of modern fake timers and waitFor

Previously, `waitFor` would timeout when `jest.useFakeTimers('modern')`
was enabled. The 'modern' version is now the default in Jest 27.

The Jest preset from `@testing-library/react-native` provides a
workaround for a larger issue in React Native and Jest that mutates the
global `Promise` object. https://git.io/JSDZI

* Remove global enabling of fake timers

Enabling fake timers can have negative consequences with `waitFor`, e.g.
causing unexpected timeouts. Enabling it globally is far-reaching and
this should likely be enabled within individual tests as needed.

* Replace jest-jasmine2 with jest-circus

The latter is considered the successor to the former. We seemingly do
not depend on anything explicitly provided by `jest-jasmine2` and should
likely move on from it.

* Switch testing environment from jsdom to node

Improves speed of test environment setup and fixes a timeout issue when
combining `waitFor` and `jest.useFakeTimers('modern')`. It is not yet
exactly pinpointed as to _why_ this fixes the timeout issue. It appears
to related to `setImmediate` and `setTimeout`.

* Remove setImmediate global for testing environment

This may be unnecessary if `testEnvironment: 'node'` is retained.
However, most tests are currently broken due to missing DOM APIs.

* Polyfill required DOM APIs for testing environment

Now that `testEnvironment: 'node'` is utilized for the testing
environment, we must mirror the app runtime and polyfill the necessary
DOM APIs used in the source.

The Enzyme configuration removed conflicts with the switch from
`testEnvironment: 'jsdom'` to `testEnvironment: 'node'`. Enzyme depends
upon `react-dom`, which introduces far more dependencies upon DOM APIs.
Currently, all Enzyme-related tests fail and need to be replaced with
`@testing-library/react-native`.

* Avoid import of react-dom within native file

Importing `react-dom` introduces additional dependencies upon the DOM
API and is incompatible with `testEnvironment: 'node'`. The `act`
utility is available from `@testing-library/react-native`.

* Explicitly toggle fake timers in tests

This may not be necessary, but may help avoid unexpected issues from
lingering fake timers, e.g. timeout errors while using `waitFor`.

* Reinstate legacy Jest timers

The Jest preset from `@testing-library/react-native` that fixed support
for "modern" timers by modifying the polyfilled the global `Promise`
resulted in new failures from within React Native itself. Specifically,
core React Native components rely upon `.done` from the `promise`
package. `.done` is a non-standard method that does not exist on the
global `Promise` used by `@testing-library/react-native`'s preset.

* Disable erroneously failing test

This previously passing test now fails after upgrading
`@testing-library/react-native` due to changes in the library. Setting
`pointerEvent` to "box-none" or "none" currently erroneously prevents
triggering other events unrelated to pressing on the element, e.g.
`onTouch*`, `onLayout`.

https://git.io/JSHZt

* Reinstate jest-jasmine2 to support done callback

The Jest team create jest-circus as the predecessor for jest-jasmine2.
The former does not support the `done` callback with async/await, and
would appear to have no plans to do so.

It would likely benefit us to refactor the one current test using
`done` away from it, and embrace `jest-circus` to maintain alignment
with Jest core.

* Refactor native unit tests away from done callback

The Jest team created `jest-circus` as the predecessor for
`jest-jasmine2`. The former does not support the `done` callback with
async/await, and would appear to have no plans to do so.

In order to embrace `jest-circus` and maintain alignment with Jest core,
the one test using `done` was refactored to avoid it

- https://git.io/JSHWU
- https://git.io/JSHWk

* Remove Enzyme from Editor tests

* Remove Enzyme from Paragraph tests

* Remove Enzyme from BlockMover tests

* Remove Enzyme from BlockEdit test

* Remove Enzyme from LinksUI test

* Remove Enzyme from ListEdit tests

* Remove Enzyme from Platform tests

* Remove Enzyme from BlockTypesTab tests

* Remove Enzyme from HTMLTextInput tests

* Remove Enzyme from ReusableBlockTab tests

* Remove Enzyme from MediaUpload tests

* Remove Enzyme from BlockMediaUpdateProgress tests

* Remove Enzyme from MediaUploadProgress tests

* Fix ReferencEerror in Inserter test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Fix ReferencEerror in Verse test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Fix ReferencEerror in Audio test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Fix ReferencEerror in File test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Fix ReferencEerror in Search test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Fix ReferencEerror in Missing test

Usage of `react-test-renderer` caused the following error. Leveraging
`@testing-library/react-native` instead resolved it for an unknown
reason.

```
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From packages/block-editor/src/components/inserter/test/index.native.js.

      468 |                     style: listStyle,
      469 |                     safeAreaBottomInset,
    > 470 |                     scrollEnabled,
          |                                             ^
      471 |                     automaticallyAdjustContentInsets: false,
      472 |             };
      473 |

      at Object.get PanResponder [as PanResponder] (node_modules/react-native/index.js:251:12)
      at BottomSheet.render (packages/components/src/mobile/bottom-sheet/index.native.js:470:39)
      at finishClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8459:31)
      at updateClassComponent (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8409:24)
      at beginWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:9990:16)
```

* Upgrade to @testing-library/[email protected]

* Add assertions to block type tab tests

Improve test clarity with explicit expect assertions.

Co-authored-by: Carlos Garcia <[email protected]>

* Remove unnecessary abstraction

The switch away from Enzyme reduced this abstraction to a single line,
so it provides less value now.

* Consistently assert media block update progress spinner removal

This increases consistency amongst the tests, as most already include
this assertion.

* Update MediaUpload test to select 'Choose from device' option

This selection better aligns with the test description.

* Remove duplicative matchMedia global definition

The test environment now imports the globals setup file used by the app
runtime. That file includes a `matchMedia` global definition as well.

* Add assertions to LinkSettings tests

Improve test clarity with explicit expect assertions.

* Removed shallow renderer usage in tests

Shallow rendering components is generally considered a non-optimal
approach to testing React components by the React community. This
replaces `shallow` with `render` to further test integration of the
subject components.

* Remove unused import

The `React` import was utilized by the now removed `shallow` render
implementation.

* Remove unnecessary top-level beforeAll usage

Jest runs each test file independently, so top-level code will not
impact other test files. Since the `(before|after)All` usage in code
changed is all top-level, and not within a `describe`, it is
superfluous.

Co-authored-by: Carlos Garcia <[email protected]>
  • Loading branch information
2 people authored and gziolo committed Jan 14, 2022
1 parent 245f679 commit 0420fce
Show file tree
Hide file tree
Showing 39 changed files with 838 additions and 804 deletions.
77 changes: 18 additions & 59 deletions package-lock.json

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
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
"@storybook/react": "6.4.9",
"@testing-library/jest-dom": "5.16.1",
"@testing-library/react": "11.2.2",
"@testing-library/react-native": "7.1.0",
"@testing-library/react-native": "9.0.0",
"@types/classnames": "2.3.1",
"@types/eslint": "7.28.0",
"@types/estree": "0.0.50",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* External dependencies
*/
import { shallow, mount } from 'enzyme';
import { render } from 'test/helpers';
import { Text } from 'react-native';

/**
* WordPress dependencies
Expand All @@ -26,39 +27,39 @@ describe( 'Edit', () => {
} );

it( 'should return null if block type not defined', () => {
const wrapper = shallow( <Edit name="core/test-block" /> );
const screen = render( <Edit name="core/test-block" /> );

expect( wrapper.type() ).toBe( null );
expect( screen.toJSON() ).toBe( null );
} );

it( 'should use edit implementation of block', () => {
const edit = () => <div />;
const edit = () => <Text>core/test-block</Text>;
registerBlockType( 'core/test-block', {
category: 'text',
title: 'block title',
edit,
} );

const wrapper = shallow( <Edit name="core/test-block" /> );
const screen = render( <Edit name="core/test-block" /> );

expect( wrapper.exists( edit ) ).toBe( true );
expect( screen.getByText( 'core/test-block' ) ).toBeDefined();
} );

it( 'should assign context', () => {
const edit = ( { context } ) => context.value;
const edit = ( { context } ) => <Text>{ context.value }</Text>;
registerBlockType( 'core/test-block', {
category: 'text',
title: 'block title',
usesContext: [ 'value' ],
edit,
} );

const wrapper = mount(
const screen = render(
<BlockContextProvider value={ { value: 'Ok' } }>
<Edit name="core/test-block" />
</BlockContextProvider>
);

expect( wrapper.html() ).toBe( 'Ok' );
expect( screen.getByText( 'Ok' ) ).toBeDefined();
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export class BlockMediaUpdateProgress extends Component {
<View style={ styles.mediaUploadProgress } pointerEvents="box-none">
{ showSpinner && (
<View style={ styles.progressBar }>
<Spinner progress={ progress } />
<Spinner progress={ progress } testID="spinner" />
</View>
) }
{ renderContent( {
Expand Down
Loading

0 comments on commit 0420fce

Please sign in to comment.