Skip to content

Commit dae25d5

Browse files
LaurensBosscherKent C. Dodds
authored and
Kent C. Dodds
committed
fix: ensure timing things are not mocked (#305)
* feature/use-original-setTimeout-when-used-in-a-test * also utilize the original clearTimeout, added tests for getSetTimeout and getClearTimeout * added extra check to getSetTimeout and getClearTimeout to make sure that environments withouth window will work as expected * simplified getSetTimeout and getClearTimeout * updated tests for the Node environment, removed dependency injection for window * updated test name * Update src/helpers.js Co-Authored-By: Kent C. Dodds <[email protected]> * Update src/helpers.js Co-Authored-By: Kent C. Dodds <[email protected]> * updated with feedback from Kent * do some fancy magic stuff * no longer needed * fix coverage Co-authored-by: Kent C. Dodds <[email protected]>
1 parent 170978b commit dae25d5

10 files changed

+152
-114
lines changed

‎src/__tests__/fake-timers.js‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import {render} from './helpers/test-utils'
2+
3+
// Because we're using fake timers here and I don't want these tests to run
4+
// for the actual length of the test (because it's waiting for a timeout error)
5+
// we'll mock the setTimeout, clearTimeout, and setImmediate to be the ones
6+
// that jest will mock for us.
7+
jest.mock('../helpers', () => {
8+
const actualHelpers = jest.requireActual('../helpers')
9+
return {
10+
...actualHelpers,
11+
setTimeout,
12+
clearTimeout,
13+
setImmediate,
14+
}
15+
})
16+
17+
jest.useFakeTimers()
18+
19+
// Because of the way jest mocking works here's the order of things (and no, the order of the code above doesn't make a difference):
20+
// 1. Just mocks '../helpers' and setTimeout/clearTimeout/setImmediate are set to their "correct" values
21+
// 2. We tell Jest to use fake timers
22+
// 3. We reset the modules and we mock '../helpers' again so now setTimeout/clearTimeout/setImmediate are set to their mocked values
23+
// We're only doing this because want to mock those values so this test doesn't take 4501ms to run.
24+
jest.resetModules()
25+
26+
const {
27+
waitForElement,
28+
waitForDomChange,
29+
waitForElementToBeRemoved,
30+
} = require('../')
31+
32+
test('waitForElementToBeRemoved: times out after 4500ms by default', () => {
33+
const {container} = render(`<div></div>`)
34+
const promise = expect(
35+
waitForElementToBeRemoved(() => container),
36+
).rejects.toThrowErrorMatchingInlineSnapshot(
37+
`"Timed out in waitForElementToBeRemoved."`,
38+
)
39+
jest.advanceTimersByTime(4501)
40+
return promise
41+
})
42+
43+
test('waitForElement: can time out', async () => {
44+
const promise = waitForElement(() => {})
45+
jest.advanceTimersByTime(4600)
46+
await expect(promise).rejects.toThrow(/timed out/i)
47+
})
48+
49+
test('waitForElement: can specify our own timeout time', async () => {
50+
const promise = waitForElement(() => {}, {timeout: 4700})
51+
const handler = jest.fn()
52+
promise.then(handler, handler)
53+
// advance beyond the default
54+
jest.advanceTimersByTime(4600)
55+
// promise was neither rejected nor resolved
56+
expect(handler).toHaveBeenCalledTimes(0)
57+
58+
// advance beyond our specified timeout
59+
jest.advanceTimersByTime(150)
60+
61+
// timed out
62+
await expect(promise).rejects.toThrow(/timed out/i)
63+
})
64+
65+
test('waitForDomChange: can time out', async () => {
66+
const promise = waitForDomChange()
67+
jest.advanceTimersByTime(4600)
68+
await expect(promise).rejects.toThrow(/timed out/i)
69+
})
70+
71+
test('waitForDomChange: can specify our own timeout time', async () => {
72+
const promise = waitForDomChange({timeout: 4700})
73+
const handler = jest.fn()
74+
promise.then(handler, handler)
75+
// advance beyond the default
76+
jest.advanceTimersByTime(4600)
77+
// promise was neither rejected nor resolved
78+
expect(handler).toHaveBeenCalledTimes(0)
79+
80+
// advance beyond our specified timeout
81+
jest.advanceTimersByTime(150)
82+
83+
// timed out
84+
await expect(promise).rejects.toThrow(/timed out/i)
85+
})

‎src/__tests__/wait-for-dom-change.js‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,29 +50,3 @@ Array [
5050
]
5151
`)
5252
})
53-
54-
test('can time out', async () => {
55-
jest.useFakeTimers()
56-
const promise = waitForDomChange()
57-
jest.advanceTimersByTime(4600)
58-
await expect(promise).rejects.toThrow(/timed out/i)
59-
jest.useRealTimers()
60-
})
61-
62-
test('can specify our own timeout time', async () => {
63-
jest.useFakeTimers()
64-
const promise = waitForDomChange({timeout: 4700})
65-
const handler = jest.fn()
66-
promise.then(handler, handler)
67-
// advance beyond the default
68-
jest.advanceTimersByTime(4600)
69-
// promise was neither rejected nor resolved
70-
expect(handler).toHaveBeenCalledTimes(0)
71-
72-
// advance beyond our specified timeout
73-
jest.advanceTimersByTime(150)
74-
75-
// timed out
76-
await expect(promise).rejects.toThrow(/timed out/i)
77-
jest.useRealTimers()
78-
})

‎src/__tests__/wait-for-element-to-be-removed.fake-timers.js‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

‎src/__tests__/wait-for-element-to-be-removed.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,27 @@ test('resolves on mutation if callback throws an error', async () => {
3333
})
3434
await waitForElementToBeRemoved(() => getByTestId('div'), {timeout: 100})
3535
})
36+
37+
test('requires a function as the first parameter', () => {
38+
return expect(
39+
waitForElementToBeRemoved(),
40+
).rejects.toThrowErrorMatchingInlineSnapshot(
41+
`"waitForElementToBeRemoved requires a function as the first parameter"`,
42+
)
43+
})
44+
45+
test('requires an element to exist first', () => {
46+
return expect(
47+
waitForElementToBeRemoved(() => null),
48+
).rejects.toThrowErrorMatchingInlineSnapshot(
49+
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`,
50+
)
51+
})
52+
53+
test('requires an unempty array of elements to exist first', () => {
54+
return expect(
55+
waitForElementToBeRemoved(() => []),
56+
).rejects.toThrowErrorMatchingInlineSnapshot(
57+
`"The callback function which was passed did not return an element or non-empty array of elements. waitForElementToBeRemoved requires that the element(s) exist before waiting for removal."`,
58+
)
59+
})

‎src/__tests__/wait-for-element.js‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,6 @@ test('waits for element to appear in a specified container', async () => {
1919
expect(element).toBeTruthy()
2020
})
2121

22-
test('can time out', async () => {
23-
jest.useFakeTimers()
24-
const promise = waitForElement(() => {})
25-
jest.advanceTimersByTime(4600)
26-
await expect(promise).rejects.toThrow(/timed out/i)
27-
jest.useRealTimers()
28-
})
29-
30-
test('can specify our own timeout time', async () => {
31-
jest.useFakeTimers()
32-
const promise = waitForElement(() => {}, {timeout: 4700})
33-
const handler = jest.fn()
34-
promise.then(handler, handler)
35-
// advance beyond the default
36-
jest.advanceTimersByTime(4600)
37-
// promise was neither rejected nor resolved
38-
expect(handler).toHaveBeenCalledTimes(0)
39-
40-
// advance beyond our specified timeout
41-
jest.advanceTimersByTime(150)
42-
43-
// timed out
44-
await expect(promise).rejects.toThrow(/timed out/i)
45-
jest.useRealTimers()
46-
})
47-
4822
test('throws last thrown error', async () => {
4923
const {rerender, container} = render('<div />')
5024
let error

‎src/helpers.js‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,20 @@
11
import MutationObserver from '@sheerun/mutationobserver-shim'
22

3+
const globalObj = typeof window === 'undefined' ? global : window
4+
5+
// we only run our tests in node, and setImmediate is supported in node.
6+
// istanbul ignore next
7+
function setImmediatePolyfill(fn) {
8+
return globalObj.setTimeout(fn, 0)
9+
}
10+
11+
// istanbul ignore next
12+
const {
13+
setTimeout,
14+
clearTimeout,
15+
setImmediate = setImmediatePolyfill,
16+
} = globalObj
17+
318
function newMutationObserver(onMutation) {
419
const MutationObserverConstructor =
520
typeof window !== 'undefined' &&
@@ -18,20 +33,10 @@ function getDocument() {
1833
return window.document
1934
}
2035

21-
/*
22-
* There are browsers for which `setImmediate` is not available. This
23-
* serves as a polyfill of sorts, adopting `setTimeout` as the closest
24-
* equivalent
25-
*/
26-
function getSetImmediate() {
27-
/* istanbul ignore else */
28-
if (typeof setImmediate === 'function') {
29-
return setImmediate
30-
} else {
31-
return function setImmediate(fn) {
32-
return setTimeout(fn, 0)
33-
}
34-
}
36+
export {
37+
getDocument,
38+
newMutationObserver,
39+
setImmediate,
40+
setTimeout,
41+
clearTimeout,
3542
}
36-
37-
export {getDocument, newMutationObserver, getSetImmediate}

‎src/wait-for-dom-change.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import {newMutationObserver, getDocument, getSetImmediate} from './helpers'
1+
import {
2+
newMutationObserver,
3+
getDocument,
4+
setImmediate,
5+
setTimeout,
6+
clearTimeout,
7+
} from './helpers'
28
import {getConfig} from './config'
39

410
function waitForDomChange({
@@ -12,7 +18,6 @@ function waitForDomChange({
1218
},
1319
} = {}) {
1420
return new Promise((resolve, reject) => {
15-
const setImmediate = getSetImmediate()
1621
const timer = setTimeout(onTimeout, timeout)
1722
const observer = newMutationObserver(onMutation)
1823
observer.observe(container, mutationObserverOptions)

‎src/wait-for-element-to-be-removed.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import {getDocument, getSetImmediate, newMutationObserver} from './helpers'
1+
import {
2+
getDocument,
3+
newMutationObserver,
4+
setImmediate,
5+
setTimeout,
6+
clearTimeout,
7+
} from './helpers'
28
import {getConfig} from './config'
39

410
function waitForElementToBeRemoved(
@@ -44,7 +50,6 @@ function waitForElementToBeRemoved(
4450
}
4551

4652
function onDone(error, result) {
47-
const setImmediate = getSetImmediate()
4853
clearTimeout(timer)
4954
setImmediate(() => observer.disconnect())
5055
if (error) {

‎src/wait-for-element.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import {newMutationObserver, getDocument, getSetImmediate} from './helpers'
1+
import {
2+
newMutationObserver,
3+
getDocument,
4+
setImmediate,
5+
setTimeout,
6+
clearTimeout,
7+
} from './helpers'
28
import {getConfig} from './config'
39

410
function waitForElement(
@@ -23,10 +29,10 @@ function waitForElement(
2329
}
2430
let lastError
2531
const timer = setTimeout(onTimeout, timeout)
32+
2633
const observer = newMutationObserver(onMutation)
2734
observer.observe(container, mutationObserverOptions)
2835
function onDone(error, result) {
29-
const setImmediate = getSetImmediate()
3036
clearTimeout(timer)
3137
setImmediate(() => observer.disconnect())
3238
if (error) {

‎tests/jest.config.node.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const baseConfig = require('kcd-scripts/jest')
44
module.exports = {
55
...baseConfig,
66
rootDir: path.join(__dirname, '..'),
7-
setupFilesAfterEnv: undefined,
87
displayName: 'node',
98
testEnvironment: 'jest-environment-node',
109
testMatch: ['**/__node_tests__/**.js'],

0 commit comments

Comments
 (0)