fix: ensure timing things are not mocked (testing-library#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]>

src/__tests__/fake-timers.js

@@ -0,0 +1,85 @@
+import {render} from './helpers/test-utils'
+
+// Because we're using fake timers here and I don't want these tests to run
+// for the actual length of the test (because it's waiting for a timeout error)
+// we'll mock the setTimeout, clearTimeout, and setImmediate to be the ones
+// that jest will mock for us.
+jest.mock('../helpers', () => {
+  const actualHelpers = jest.requireActual('../helpers')
+  return {
+    ...actualHelpers,
+    setTimeout,
+    clearTimeout,
+    setImmediate,
+  }
+})
+
+jest.useFakeTimers()
+
+// 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):
+// 1. Just mocks '../helpers' and setTimeout/clearTimeout/setImmediate are set to their "correct" values
+// 2. We tell Jest to use fake timers
+// 3. We reset the modules and we mock '../helpers' again so now setTimeout/clearTimeout/setImmediate are set to their mocked values
+// We're only doing this because want to mock those values so this test doesn't take 4501ms to run.
+jest.resetModules()
+
+const {
+  waitForElement,
+  waitForDomChange,
+  waitForElementToBeRemoved,
+} = require('../')
+
+test('waitForElementToBeRemoved: times out after 4500ms by default', () => {
+  const {container} = render(`<div></div>`)
+  const promise = expect(
+    waitForElementToBeRemoved(() => container),
+  ).rejects.toThrowErrorMatchingInlineSnapshot(
+    `"Timed out in waitForElementToBeRemoved."`,
+  )
+  jest.advanceTimersByTime(4501)
+  return promise
+})
+
+test('waitForElement: can time out', async () => {
+  const promise = waitForElement(() => {})
+  jest.advanceTimersByTime(4600)
+  await expect(promise).rejects.toThrow(/timed out/i)
+})
+
+test('waitForElement: can specify our own timeout time', async () => {
+  const promise = waitForElement(() => {}, {timeout: 4700})
+  const handler = jest.fn()
+  promise.then(handler, handler)
+  // advance beyond the default
+  jest.advanceTimersByTime(4600)
+  // promise was neither rejected nor resolved
+  expect(handler).toHaveBeenCalledTimes(0)
+
+  // advance beyond our specified timeout
+  jest.advanceTimersByTime(150)
+
+  // timed out
+  await expect(promise).rejects.toThrow(/timed out/i)
+})
+
+test('waitForDomChange: can time out', async () => {
+  const promise = waitForDomChange()
+  jest.advanceTimersByTime(4600)
+  await expect(promise).rejects.toThrow(/timed out/i)
+})
+
+test('waitForDomChange: can specify our own timeout time', async () => {
+  const promise = waitForDomChange({timeout: 4700})
+  const handler = jest.fn()
+  promise.then(handler, handler)
+  // advance beyond the default
+  jest.advanceTimersByTime(4600)
+  // promise was neither rejected nor resolved
+  expect(handler).toHaveBeenCalledTimes(0)
+
+  // advance beyond our specified timeout
+  jest.advanceTimersByTime(150)
+
+  // timed out
+  await expect(promise).rejects.toThrow(/timed out/i)
+})

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

@@ -50,29 +50,3 @@ Array [
 ]
 `)
 })
-
-test('can time out', async () => {
-  jest.useFakeTimers()
-  const promise = waitForDomChange()
-  jest.advanceTimersByTime(4600)
-  await expect(promise).rejects.toThrow(/timed out/i)
-  jest.useRealTimers()
-})
-
-test('can specify our own timeout time', async () => {
-  jest.useFakeTimers()
-  const promise = waitForDomChange({timeout: 4700})
-  const handler = jest.fn()
-  promise.then(handler, handler)
-  // advance beyond the default
-  jest.advanceTimersByTime(4600)
-  // promise was neither rejected nor resolved
-  expect(handler).toHaveBeenCalledTimes(0)
-
-  // advance beyond our specified timeout
-  jest.advanceTimersByTime(150)
-
-  // timed out
-  await expect(promise).rejects.toThrow(/timed out/i)
-  jest.useRealTimers()
-})

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

@@ -33,3 +33,27 @@ test('resolves on mutation if callback throws an error', async () => {
   })
   await waitForElementToBeRemoved(() => getByTestId('div'), {timeout: 100})
 })
+
+test('requires a function as the first parameter', () => {
+  return expect(
+    waitForElementToBeRemoved(),
+  ).rejects.toThrowErrorMatchingInlineSnapshot(
+    `"waitForElementToBeRemoved requires a function as the first parameter"`,
+  )
+})
+
+test('requires an element to exist first', () => {
+  return expect(
+    waitForElementToBeRemoved(() => null),
+  ).rejects.toThrowErrorMatchingInlineSnapshot(
+    `"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."`,
+  )
+})
+
+test('requires an unempty array of elements to exist first', () => {
+  return expect(
+    waitForElementToBeRemoved(() => []),
+  ).rejects.toThrowErrorMatchingInlineSnapshot(
+    `"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."`,
+  )
+})

src/__tests__/wait-for-element.js

@@ -19,32 +19,6 @@ test('waits for element to appear in a specified container', async () => {
   expect(element).toBeTruthy()
 })
 
-test('can time out', async () => {
-  jest.useFakeTimers()
-  const promise = waitForElement(() => {})
-  jest.advanceTimersByTime(4600)
-  await expect(promise).rejects.toThrow(/timed out/i)
-  jest.useRealTimers()
-})
-
-test('can specify our own timeout time', async () => {
-  jest.useFakeTimers()
-  const promise = waitForElement(() => {}, {timeout: 4700})
-  const handler = jest.fn()
-  promise.then(handler, handler)
-  // advance beyond the default
-  jest.advanceTimersByTime(4600)
-  // promise was neither rejected nor resolved
-  expect(handler).toHaveBeenCalledTimes(0)
-
-  // advance beyond our specified timeout
-  jest.advanceTimersByTime(150)
-
-  // timed out
-  await expect(promise).rejects.toThrow(/timed out/i)
-  jest.useRealTimers()
-})
-
 test('throws last thrown error', async () => {
   const {rerender, container} = render('<div />')
   let error

src/helpers.js

@@ -1,5 +1,20 @@
 import MutationObserver from '@sheerun/mutationobserver-shim'
 
+const globalObj = typeof window === 'undefined' ? global : window
+
+// we only run our tests in node, and setImmediate is supported in node.
+// istanbul ignore next
+function setImmediatePolyfill(fn) {
+  return globalObj.setTimeout(fn, 0)
+}
+
+// istanbul ignore next
+const {
+  setTimeout,
+  clearTimeout,
+  setImmediate = setImmediatePolyfill,
+} = globalObj
+
 function newMutationObserver(onMutation) {
   const MutationObserverConstructor =
     typeof window !== 'undefined' &&
@@ -18,20 +33,10 @@ function getDocument() {
   return window.document
 }
 
-/*
- * There are browsers for which `setImmediate` is not available. This
- * serves as a polyfill of sorts, adopting `setTimeout` as the closest
- * equivalent
- */
-function getSetImmediate() {
-  /* istanbul ignore else */
-  if (typeof setImmediate === 'function') {
-    return setImmediate
-  } else {
-    return function setImmediate(fn) {
-      return setTimeout(fn, 0)
-    }
-  }
+export {
+  getDocument,
+  newMutationObserver,
+  setImmediate,
+  setTimeout,
+  clearTimeout,
 }
-
-export {getDocument, newMutationObserver, getSetImmediate}

src/wait-for-dom-change.js

@@ -1,4 +1,10 @@
-import {newMutationObserver, getDocument, getSetImmediate} from './helpers'
+import {
+  newMutationObserver,
+  getDocument,
+  setImmediate,
+  setTimeout,
+  clearTimeout,
+} from './helpers'
 import {getConfig} from './config'
 
 function waitForDomChange({
@@ -12,7 +18,6 @@ function waitForDomChange({
   },
 } = {}) {
   return new Promise((resolve, reject) => {
-    const setImmediate = getSetImmediate()
     const timer = setTimeout(onTimeout, timeout)
     const observer = newMutationObserver(onMutation)
     observer.observe(container, mutationObserverOptions)

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

@@ -1,4 +1,10 @@
-import {getDocument, getSetImmediate, newMutationObserver} from './helpers'
+import {
+  getDocument,
+  newMutationObserver,
+  setImmediate,
+  setTimeout,
+  clearTimeout,
+} from './helpers'
 import {getConfig} from './config'
 
 function waitForElementToBeRemoved(
@@ -44,7 +50,6 @@ function waitForElementToBeRemoved(
     }
 
     function onDone(error, result) {
-      const setImmediate = getSetImmediate()
       clearTimeout(timer)
       setImmediate(() => observer.disconnect())
       if (error) {

src/wait-for-element.js

@@ -1,4 +1,10 @@
-import {newMutationObserver, getDocument, getSetImmediate} from './helpers'
+import {
+  newMutationObserver,
+  getDocument,
+  setImmediate,
+  setTimeout,
+  clearTimeout,
+} from './helpers'
 import {getConfig} from './config'
 
 function waitForElement(
@@ -23,10 +29,10 @@ function waitForElement(
     }
     let lastError
     const timer = setTimeout(onTimeout, timeout)
+
     const observer = newMutationObserver(onMutation)
     observer.observe(container, mutationObserverOptions)
     function onDone(error, result) {
-      const setImmediate = getSetImmediate()
       clearTimeout(timer)
       setImmediate(() => observer.disconnect())
       if (error) {

tests/jest.config.node.js

@@ -4,7 +4,6 @@ const baseConfig = require('kcd-scripts/jest')
 module.exports = {
   ...baseConfig,
   rootDir: path.join(__dirname, '..'),
-  setupFilesAfterEnv: undefined,
   displayName: 'node',
   testEnvironment: 'jest-environment-node',
   testMatch: ['**/__node_tests__/**.js'],