feat: call "cleanup" automatically (#238)

aleclarson · thymikee · mdjastrzebski · web-flow · commit 175b9b9fa2e1 · 2020-05-20T13:33:28.000+02:00
* feat: add "cleanup" function * docs: add "cleanup" section * feat: automatically call "cleanup" after each test ...if an "afterEach" global function is defined, and process.env.RTL_SKIP_AUTO_CLEANUP is falsy. Taken from: https://github.com/testing-library/react-testing-library/blob/14670debd45236d2c5d0a61a83dadc72af1bef7c/src/index.js * docs: mention automatic cleanup * add within * fix lint * Updated tests, added auto-cleanup test * Added ways to prevent auto cleanup * Small fix * Code review changes * Update website/docs/API.md Co-authored-by: Michał Pierzchała <thymikee@gmail.com> * More changes * Removed afterEach(cleanup) calls in examples * Code cleanup Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Maciej Jastrzebski <mdjastrzebski@gmail.com>
diff --git a/dont-cleanup-after-each.js b/dont-cleanup-after-each.js
@@ -0,0 +1 @@
+process.env.RNTL_SKIP_AUTO_CLEANUP = true;
diff --git a/examples/reactnavigation/src/__tests__/AppNavigator.test.js b/examples/reactnavigation/src/__tests__/AppNavigator.test.js
@@ -1,15 +1,13 @@
 import React from 'react';
 import { NavigationContainer } from '@react-navigation/native';
-import { render, fireEvent, cleanup } from 'react-native-testing-library';
+import { render, fireEvent } from 'react-native-testing-library';
 
 import AppNavigator from '../AppNavigator';
 
 // Silence the warning https://github.com/facebook/react-native/issues/11094#issuecomment-263240420
 jest.mock('react-native/Libraries/Animated/src/NativeAnimatedHelper');
 
 describe('Testing react navigation', () => {
-  afterEach(cleanup);
-
   test('page contains the header and 10 items', () => {
     const component = (
       <NavigationContainer>
diff --git a/examples/redux/components/AddTodo.test.js b/examples/redux/components/AddTodo.test.js
@@ -1,12 +1,10 @@
 import React from 'react';
 import { Provider } from 'react-redux';
-import { cleanup, fireEvent, render } from 'react-native-testing-library';
+import { fireEvent, render } from 'react-native-testing-library';
 import configureStore from '../store';
 import AddTodo from './AddTodo';
 
 describe('Application test', () => {
-  afterEach(cleanup);
-
   test('adds a new test when entry has been included', () => {
     const store = configureStore();
 
diff --git a/examples/redux/components/TodoList.test.js b/examples/redux/components/TodoList.test.js
@@ -1,12 +1,10 @@
 import React from 'react';
 import { Provider } from 'react-redux';
-import { cleanup, fireEvent, render } from 'react-native-testing-library';
+import { fireEvent, render } from 'react-native-testing-library';
 import configureStore from '../store';
 import TodoList from './TodoList';
 
 describe('Application test', () => {
-  afterEach(cleanup);
-
   test('it should execute with a store with 4 elements', () => {
     const initialState = {
       todos: [
diff --git a/examples/redux/reducers/todoReducer.js b/examples/redux/reducers/todoReducer.js
@@ -6,7 +6,6 @@ export default function todoReducer(state = [], action) {
       return state.concat(action.payload);
 
     case actions.REMOVE:
-      console.log(action);
       return state.filter((todo) => todo.id !== action.payload.id);
 
     case actions.MODIFY:
diff --git a/pure.js b/pure.js
@@ -0,0 +1,2 @@
+// makes it so people can import from 'react-native-testing-library/pure'
+module.exports = require('./build/pure');
diff --git a/src/__tests__/auto-cleanup-skip.js b/src/__tests__/auto-cleanup-skip.js
@@ -0,0 +1,39 @@
+import React from 'react';
+import { View } from 'react-native';
+
+let render;
+beforeAll(() => {
+  process.env.RNTL_SKIP_AUTO_CLEANUP = 'true';
+  const rtl = require('../');
+  render = rtl.render;
+});
+
+let isMounted = false;
+
+class Test extends React.Component<*> {
+  componentDidMount() {
+    isMounted = true;
+  }
+
+  componentWillUnmount() {
+    isMounted = false;
+    if (this.props.onUnmount) {
+      this.props.onUnmount();
+    }
+  }
+  render() {
+    return <View />;
+  }
+}
+
+// This just verifies that by importing RNTL in pure mode in an environment which supports
+// afterEach (like jest) we won't get automatic cleanup between tests.
+test('component is mounted, but not umounted before test ends', () => {
+  const fn = jest.fn();
+  render(<Test onUnmount={fn} />);
+  expect(fn).not.toHaveBeenCalled();
+});
+
+test('component is NOT automatically umounted after first test ends', () => {
+  expect(isMounted).toEqual(true);
+});
diff --git a/src/__tests__/auto-cleanup.js b/src/__tests__/auto-cleanup.js
@@ -0,0 +1,33 @@
+import React from 'react';
+import { View } from 'react-native';
+import { render } from '..';
+
+let isMounted = false;
+
+class Test extends React.Component<*> {
+  componentDidMount() {
+    isMounted = true;
+  }
+
+  componentWillUnmount() {
+    isMounted = false;
+    if (this.props.onUnmount) {
+      this.props.onUnmount();
+    }
+  }
+  render() {
+    return <View />;
+  }
+}
+
+// This just verifies that by importing RNTL in an environment which supports afterEach (like jest)
+// we'll get automatic cleanup between tests.
+test('component is mounted, but not umounted before test ends', () => {
+  const fn = jest.fn();
+  render(<Test onUnmount={fn} />);
+  expect(fn).not.toHaveBeenCalled();
+});
+
+test('component is automatically umounted after first test ends', () => {
+  expect(isMounted).toEqual(false);
+});
diff --git a/src/__tests__/cleanup.test.js b/src/__tests__/cleanup.test.js
@@ -2,7 +2,7 @@
 /* eslint-disable react/no-multi-comp */
 import React from 'react';
 import { View } from 'react-native';
-import { cleanup, render } from '..';
+import { cleanup, render } from '../pure';
 
 class Test extends React.Component<*> {
   componentWillUnmount() {
diff --git a/src/__tests__/waitFor.test.js b/src/__tests__/waitFor.test.js
@@ -55,6 +55,10 @@ test('waits for element until timeout is met', async () => {
   await expect(
     waitFor(() => getByText('Fresh'), { timeout: 100 })
   ).rejects.toThrow();
+
+  // Async action ends after 300ms and we only waited 100ms, so we need to wait
+  // for the remaining async actions to finish
+  await waitFor(() => getByText('Fresh'));
 });
 
 test('waits for element with custom interval', async () => {
diff --git a/src/index.js b/src/index.js
@@ -1,18 +1,18 @@
 // @flow
-import act from './act';
-import cleanup from './cleanup';
-import fireEvent from './fireEvent';
 import flushMicrotasksQueue from './flushMicrotasksQueue';
-import render from './render';
-import shallow from './shallow';
-import waitFor, { waitForElement } from './waitFor';
-import within from './within';
+import { cleanup } from './pure';
 
-export { act };
-export { cleanup };
-export { fireEvent };
-export { flushMicrotasksQueue };
-export { render };
-export { shallow };
-export { waitFor, waitForElement };
-export { within };
+// If we're running in a test runner that supports afterEach
+// then we'll automatically run cleanup afterEach test
+// this ensures that tests run in isolation from each other
+// if you don't like this then either import the `pure` module
+// or set the RNTL_SKIP_AUTO_CLEANUP env variable to 'true'.
+if (typeof afterEach === 'function' && !process.env.RNTL_SKIP_AUTO_CLEANUP) {
+  // eslint-disable-next-line no-undef
+  afterEach(async () => {
+    await flushMicrotasksQueue();
+    cleanup();
+  });
+}
+
+export * from './pure';
diff --git a/src/pure.js b/src/pure.js
@@ -0,0 +1,18 @@
+// @flow
+import act from './act';
+import cleanup from './cleanup';
+import fireEvent from './fireEvent';
+import flushMicrotasksQueue from './flushMicrotasksQueue';
+import render from './render';
+import shallow from './shallow';
+import waitFor, { waitForElement } from './waitFor';
+import within from './within';
+
+export { act };
+export { cleanup };
+export { fireEvent };
+export { flushMicrotasksQueue };
+export { render };
+export { shallow };
+export { waitFor, waitForElement };
+export { within };
diff --git a/website/docs/API.md b/website/docs/API.md
@@ -74,7 +74,11 @@ Re-render the in-memory tree with a new root element. This simulates a React upd
 unmount(): void
 ```
 
-Unmount the in-memory tree, triggering the appropriate lifecycle events
+Unmount the in-memory tree, triggering the appropriate lifecycle events.
+
+:::note
+Usually you should not need to call `unmount` as it is done automatically if your test runner supports `afterEach` hook (like Jest, mocha, Jasmine).
+:::
 
 ### `debug`
 
@@ -123,10 +127,14 @@ const cleanup: () => void;
 
 Unmounts React trees that were mounted with `render`.
 
+:::info
+Please note that this is done automatically if the testing framework you're using supports the `afterEach` global (like mocha, Jest, and Jasmine). If not, you will need to do manual cleanups after each test.
+:::
+
 For example, if you're using the `jest` testing framework, then you would need to use the `afterEach` hook like so:
 
 ```jsx
-import { cleanup, render } from 'react-native-testing-library';
+import { cleanup, render } from 'react-native-testing-library/pure';
 import { View } from 'react-native';
 
 afterEach(cleanup);
diff --git a/website/docs/Migration20.md b/website/docs/Migration20.md
@@ -9,6 +9,30 @@ This guides describes major steps involved in migrating your testing code from u
 
 Node 8 reached its EOL more than 5 months ago, so it's about time to target the library to Node 10. If you used lower version, you'll have to upgrade to v10, but we suggest using the latest LTS version.
 
+## Auto Cleanup
+
+`cleanup()` function is now called automatically after every test, if your testing framework supports `afterEach` hook (like Jest, mocha, and Jasmine).
+
+You should be able to safely remove all `afterEach(cleanup)` calls in your code.
+
+This change might break your code, if you tests are not isolated, i.e. you call `render` outside `test` block. Generally, you should [keep your tests isolated](https://kentcdodds.com/blog/test-isolation-with-react), but if you can't or don't want to do this right away you can prevent this behavior using any of the foloowing ways:
+
+1. by importing `'react-native-testing-library/pure'` instead of `'react-native-testing-library'`
+
+2. by importing `'react-native-testing-library/dont-cleanup-after-each'` before importing `'react-native-testing-library'`. You can do it in a global way by using Jest's `setupFiles` like this:
+
+```js
+{
+  setupFiles: ['react-native-testing-library/dont-cleanup-after-each'];
+}
+```
+
+3. by setting `RTNL_SKIP_AUTO_CLEANUP` env variable to `true`. You can do this with `cross-evn` like this:
+
+```sh
+cross-env RNTL_SKIP_AUTO_CLEANUP=true jest
+```
+
 ## WaitFor API changes
 
 `waitForElement` function has been renamed to `waitFor` for consistency with React Testing Library. Additionally the signature has slightly changed from:
diff --git a/website/docs/ReactNavigation.md b/website/docs/ReactNavigation.md
@@ -54,7 +54,7 @@ export default function HomeScreen({ navigation }) {
     new Array(20).fill(null).map((_, idx) => idx + 1)
   );
 
-  const onOpacityPress = item => navigation.navigate('Details', item);
+  const onOpacityPress = (item) => navigation.navigate('Details', item);
 
   return (
     <View>
@@ -162,8 +162,6 @@ import AppNavigator from '../AppNavigator';
 jest.mock('react-native/Libraries/Animated/src/NativeAnimatedHelper');
 
 describe('Testing react navigation', () => {
-  afterEach(cleanup);
-
   test('page contains the header and 10 items', () => {
     const component = (
       <NavigationContainer>

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+process.env.RNTL_SKIP_AUTO_CLEANUP = true;`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+// makes it so people can import from 'react-native-testing-library/pure'`
	`2`	`+module.exports = require('./build/pure');`