Skip to content

Commit

Permalink
Fix act() tests by properly unmounting the container
Browse files Browse the repository at this point in the history
While playing around with the new `act()` API I discovered a test that
was unexpectedly failing. After some digging I noticed that the test is
passing when run in isolation and that the test suite can be fixe by
properly calling `ReactDOM.unmountComponentAtNode()` to clean up after
every test run.

You can reproduce the issue by simply commenting out the call to
`unmountComponentAtNode` in the `afterEach` callback. The newly added
test will fail but pass in isolation.

I thought it would be a good idea to fix this upstream so nobody runs
into this issue again.
  • Loading branch information
philipp-spiess committed Feb 28, 2019
1 parent 3ada82b commit b5e91d5
Showing 1 changed file with 55 additions and 35 deletions.
90 changes: 55 additions & 35 deletions packages/react-dom/src/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
let act;
let container;

function getTestDocument(markup) {
const doc = document.implementation.createHTMLDocument('');
Expand All @@ -35,6 +36,13 @@ describe('ReactTestUtils', () => {
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
act = ReactTestUtils.act;

container = document.createElement('div');
document.body.appendChild(container);
});
afterEach(() => {
ReactDOM.unmountComponentAtNode(container);
document.body.removeChild(container);
});

it('Simulate should have locally attached media events', () => {
Expand Down Expand Up @@ -67,7 +75,6 @@ describe('ReactTestUtils', () => {
// De-duplication check
ReactTestUtils.mockComponent(MockedComponent);

const container = document.createElement('div');
ReactDOM.render(<MockedComponent>Hello</MockedComponent>, container);
expect(container.textContent).toBe('Hello');
});
Expand Down Expand Up @@ -186,7 +193,6 @@ describe('ReactTestUtils', () => {
}
}

const container = document.createElement('div');
ReactDOM.render(
<Wrapper>
{null}
Expand Down Expand Up @@ -347,7 +353,6 @@ describe('ReactTestUtils', () => {
},
};
spyOnDevAndProd(obj, 'handler').and.callThrough();
const container = document.createElement('div');
const node = ReactDOM.render(
<input type="text" onChange={obj.handler} />,
container,
Expand Down Expand Up @@ -382,7 +387,6 @@ describe('ReactTestUtils', () => {
},
};
spyOnDevAndProd(obj, 'handler').and.callThrough();
const container = document.createElement('div');
const instance = ReactDOM.render(
<SomeComponent handleChange={obj.handler} />,
container,
Expand Down Expand Up @@ -426,7 +430,6 @@ describe('ReactTestUtils', () => {
}

const handler = jest.fn().mockName('spy');
const container = document.createElement('div');
const instance = ReactDOM.render(
<SomeComponent handleClick={handler} />,
container,
Expand Down Expand Up @@ -466,7 +469,6 @@ describe('ReactTestUtils', () => {
event = e;
});

const container = document.createElement('div');
const instance = ReactDOM.render(<div onKeyDown={stub} />, container);
const node = ReactDOM.findDOMNode(instance);

Expand Down Expand Up @@ -523,26 +525,20 @@ describe('ReactTestUtils', () => {
React.useEffect(props.callback);
return null;
}
const container = document.createElement('div');
document.body.appendChild(container);

try {
let called = false;
act(() => {
ReactDOM.render(
<App
callback={() => {
called = true;
}}
/>,
container,
);
});
let called = false;
act(() => {
ReactDOM.render(
<App
callback={() => {
called = true;
}}
/>,
container,
);
});

expect(called).toBe(true);
} finally {
document.body.removeChild(container);
}
expect(called).toBe(true);
});

it('flushes effects on every call', () => {
Expand All @@ -558,8 +554,6 @@ describe('ReactTestUtils', () => {
);
}

const container = document.createElement('div');
document.body.appendChild(container);
let calledCtr = 0;
act(() => {
ReactDOM.render(
Expand All @@ -586,8 +580,6 @@ describe('ReactTestUtils', () => {
expect(calledCtr).toBe(4);
act(click);
expect(calledCtr).toBe(5);

document.body.removeChild(container);
});

it('can use act to batch effects on updates too', () => {
Expand All @@ -599,8 +591,7 @@ describe('ReactTestUtils', () => {
</button>
);
}
const container = document.createElement('div');
document.body.appendChild(container);

let button;
act(() => {
ReactDOM.render(<App />, container);
Expand All @@ -611,7 +602,6 @@ describe('ReactTestUtils', () => {
button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
});
expect(button.innerHTML).toBe('1');
document.body.removeChild(container);
});

it('detects setState being called outside of act(...)', () => {
Expand All @@ -625,8 +615,7 @@ describe('ReactTestUtils', () => {
</button>
);
}
const container = document.createElement('div');
document.body.appendChild(container);

let button;
act(() => {
ReactDOM.render(<App />, container);
Expand All @@ -637,7 +626,6 @@ describe('ReactTestUtils', () => {
expect(() => setValueRef(1)).toWarnDev([
'An update to App inside a test was not wrapped in act(...).',
]);
document.body.removeChild(container);
});

it('lets a ticker update', () => {
Expand All @@ -651,7 +639,6 @@ describe('ReactTestUtils', () => {
});
return toggle;
}
const container = document.createElement('div');

act(() => {
act(() => {
Expand All @@ -678,6 +665,39 @@ describe('ReactTestUtils', () => {
);
});

it('flushes immediate re-renders with act', () => {
jest.useFakeTimers();

function App() {
let [ctr, setCtr] = React.useState(0);

React.useEffect(() => {
if (ctr === 0) {
setCtr(1);
}

const timeout = setTimeout(() => setCtr(2), 1000);
return () => clearTimeout(timeout);
});

return ctr;
}

act(() => {
ReactDOM.render(<App />, container);
// Since the effects won't be flushed yet, this does not advance the timer
jest.runAllTimers();
});

expect(container.innerHTML).toBe('1');

act(() => {
jest.runAllTimers();
});

expect(container.innerHTML).toBe('2');
});

it('warns if you try to await an .act call', () => {
expect(act(() => {}).then).toWarnDev(
[
Expand Down

0 comments on commit b5e91d5

Please sign in to comment.