From d76ca27ca92b6cd5960832a8558897fc319675e1 Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Thu, 4 Jul 2019 12:58:43 +0200 Subject: [PATCH 01/12] feature/use-original-setTimeout-when-used-in-a-test --- src/helpers.js | 28 ++++++++++++++++++++++++++- src/wait-for-dom-change.js | 9 ++++++++- src/wait-for-element-to-be-removed.js | 8 +++++++- src/wait-for-element.js | 9 ++++++++- tests/setup-env.js | 4 ++++ 5 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/helpers.js b/src/helpers.js index 77f68b8c..9fb90eec 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -34,4 +34,30 @@ function getSetImmediate() { } } -export {getDocument, newMutationObserver, getSetImmediate} +let originalSetTimeout = global.setTimeout + +if (typeof window !== 'undefined') { + originalSetTimeout = window.setTimeout +} + +/* + * Return the original setTimeout function so that all functions work as expected by a user + * if he/she utilizes dom-testing-library in a test where jest.fakeTimers() is used. + * + * global.useFakeTimers is used by us to make sure that we can still utilize fakeTimers in our tests. + * + * see: https://github.com/testing-library/dom-testing-library/issues/300 + */ +function getSetTimeout() { + if (global.useFakeTimers) { + if (typeof window !== 'undefined') { + return window.setTimeout + } + + return global.setTimeout + } + + return originalSetTimeout +} + +export {getDocument, newMutationObserver, getSetImmediate, getSetTimeout} diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index ba9be95f..ac506f69 100644 --- a/src/wait-for-dom-change.js +++ b/src/wait-for-dom-change.js @@ -1,4 +1,9 @@ -import {newMutationObserver, getDocument, getSetImmediate} from './helpers' +import { + newMutationObserver, + getDocument, + getSetImmediate, + getSetTimeout, +} from './helpers' import {getConfig} from './config' function waitForDomChange({ @@ -13,6 +18,8 @@ function waitForDomChange({ } = {}) { return new Promise((resolve, reject) => { const setImmediate = getSetImmediate() + const setTimeout = getSetTimeout() + const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) observer.observe(container, mutationObserverOptions) diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 5cb82f4c..8202925b 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/src/wait-for-element-to-be-removed.js @@ -1,4 +1,9 @@ -import {getDocument, getSetImmediate, newMutationObserver} from './helpers' +import { + getDocument, + getSetImmediate, + newMutationObserver, + getSetTimeout, +} from './helpers' import {getConfig} from './config' function waitForElementToBeRemoved( @@ -22,6 +27,7 @@ function waitForElementToBeRemoved( ), ) } + const setTimeout = getSetTimeout() const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element.js b/src/wait-for-element.js index 6bcb9dde..13f25c62 100644 --- a/src/wait-for-element.js +++ b/src/wait-for-element.js @@ -1,4 +1,9 @@ -import {newMutationObserver, getDocument, getSetImmediate} from './helpers' +import { + newMutationObserver, + getDocument, + getSetImmediate, + getSetTimeout, +} from './helpers' import {getConfig} from './config' function waitForElement( @@ -22,7 +27,9 @@ function waitForElement( return } let lastError + const setTimeout = getSetTimeout() const timer = setTimeout(onTimeout, timeout) + const observer = newMutationObserver(onMutation) observer.observe(container, mutationObserverOptions) function onDone(error, result) { diff --git a/tests/setup-env.js b/tests/setup-env.js index 4435317a..eec63f76 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1 +1,5 @@ import 'jest-dom/extend-expect' + +// Make sure that our tests do the use fakeTimer mock: +// https://github.com/testing-library/dom-testing-library/issues/300 +global.useFakeTimers = true From 22f915c914fef060d86d60e87f576f1afdf49439 Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Thu, 4 Jul 2019 14:10:33 +0200 Subject: [PATCH 02/12] also utilize the original clearTimeout, added tests for getSetTimeout and getClearTimeout --- src/__tests__/helpers.js | 57 ++++++++++++++++++++++++++- src/helpers.js | 32 +++++++++++++-- src/wait-for-dom-change.js | 6 ++- src/wait-for-element-to-be-removed.js | 6 ++- src/wait-for-element.js | 7 +++- 5 files changed, 100 insertions(+), 8 deletions(-) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index cbf6e628..f69024d1 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -1,9 +1,64 @@ -import {getDocument, newMutationObserver} from '../helpers' +import { + getDocument, + newMutationObserver, + getSetTimeout, + getClearTimeout, +} from '../helpers' test('returns global document if exists', () => { expect(getDocument()).toBe(document) }) +describe('getSetTimeout', () => { + it('returns mocked setTimeout if global.useFakeTimers is set and jest.useFakeTimers is used', () => { + // global.useFakeTimers is set to true for all tests in tests/setup-env.js + jest.useFakeTimers() + + const setTimeout = getSetTimeout(window) + + expect(setTimeout._isMockFunction).toBe(true) + + jest.useRealTimers() + }) + it('returns original getSetTimeout from window', () => { + const setTimeout = getSetTimeout(window) + + expect(setTimeout).toBe(window.setTimeout) + expect(setTimeout._isMockFunction).toBe(undefined) + }) + it('returns original getSetTimeout from global if window is undefined', () => { + const setTimeout = getSetTimeout(undefined) + + expect(setTimeout).toBe(global.setTimeout) + expect(setTimeout._isMockFunction).toBe(undefined) + }) +}) + +describe('getClearTimeout', () => { + it('returns mocked clearTimeout if global.useFakeTimers is set and jest.useFakeTimers is used', () => { + // global.useFakeTimers is set to true for all tests in tests/setup-env.js + jest.useFakeTimers() + + const clearTimeout = getClearTimeout(window) + + expect(clearTimeout._isMockFunction).toBe(true) + + jest.useRealTimers() + }) + it('returns original clearTimeout from window', () => { + const clearTimeout = getClearTimeout(window) + + expect(clearTimeout).toBe(window.clearTimeout) + expect(clearTimeout._isMockFunction).toBe(undefined) + }) + it('returns original clearTimeout from global if window is undefined', () => { + const clearTimeout = getClearTimeout(undefined) + + expect(clearTimeout).toBe(global.clearTimeout) + expect(clearTimeout._isMockFunction).toBe(undefined) + }) +}) + class DummyClass { constructor(args) { this.args = args diff --git a/src/helpers.js b/src/helpers.js index 9fb90eec..c2a1c01d 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -40,6 +40,12 @@ if (typeof window !== 'undefined') { originalSetTimeout = window.setTimeout } +let originalClearTimeout = global.clearTimeout + +if (typeof window !== 'undefined') { + originalClearTimeout = window.clearTimeout +} + /* * Return the original setTimeout function so that all functions work as expected by a user * if he/she utilizes dom-testing-library in a test where jest.fakeTimers() is used. @@ -48,10 +54,10 @@ if (typeof window !== 'undefined') { * * see: https://github.com/testing-library/dom-testing-library/issues/300 */ -function getSetTimeout() { +function getSetTimeout(windowObject) { if (global.useFakeTimers) { - if (typeof window !== 'undefined') { - return window.setTimeout + if (typeof windowObject !== 'undefined') { + return windowObject.setTimeout } return global.setTimeout @@ -60,4 +66,22 @@ function getSetTimeout() { return originalSetTimeout } -export {getDocument, newMutationObserver, getSetImmediate, getSetTimeout} +function getClearTimeout(windowObject) { + if (global.useFakeTimers) { + if (typeof windowObject !== 'undefined') { + return windowObject.clearTimeout + } + + return global.clearTimeout + } + + return originalClearTimeout +} + +export { + getDocument, + newMutationObserver, + getSetImmediate, + getSetTimeout, + getClearTimeout, +} diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index ac506f69..778b0b96 100644 --- a/src/wait-for-dom-change.js +++ b/src/wait-for-dom-change.js @@ -3,6 +3,7 @@ import { getDocument, getSetImmediate, getSetTimeout, + getClearTimeout, } from './helpers' import {getConfig} from './config' @@ -18,7 +19,10 @@ function waitForDomChange({ } = {}) { return new Promise((resolve, reject) => { const setImmediate = getSetImmediate() - const setTimeout = getSetTimeout() + const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) + const clearTimeout = getClearTimeout( + typeof window !== 'undefined' && window, + ) const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 8202925b..9091ba03 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/src/wait-for-element-to-be-removed.js @@ -3,6 +3,7 @@ import { getSetImmediate, newMutationObserver, getSetTimeout, + getClearTimeout, } from './helpers' import {getConfig} from './config' @@ -27,7 +28,10 @@ function waitForElementToBeRemoved( ), ) } - const setTimeout = getSetTimeout() + const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) + const clearTimeout = getClearTimeout( + typeof window !== 'undefined' && window, + ) const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element.js b/src/wait-for-element.js index 13f25c62..ce15775f 100644 --- a/src/wait-for-element.js +++ b/src/wait-for-element.js @@ -3,6 +3,7 @@ import { getDocument, getSetImmediate, getSetTimeout, + getClearTimeout, } from './helpers' import {getConfig} from './config' @@ -27,7 +28,11 @@ function waitForElement( return } let lastError - const setTimeout = getSetTimeout() + const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) + const clearTimeout = getClearTimeout( + typeof window !== 'undefined' && window, + ) + const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) From cf2fb0309d7f19d6ca4d35046fa878ef04ea41f1 Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Thu, 4 Jul 2019 14:15:35 +0200 Subject: [PATCH 03/12] added extra check to getSetTimeout and getClearTimeout to make sure that environments withouth window will work as expected --- src/helpers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helpers.js b/src/helpers.js index c2a1c01d..3f2cbb4c 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -56,7 +56,7 @@ if (typeof window !== 'undefined') { */ function getSetTimeout(windowObject) { if (global.useFakeTimers) { - if (typeof windowObject !== 'undefined') { + if (typeof windowObject !== 'undefined' && windowObject !== false) { return windowObject.setTimeout } @@ -68,7 +68,7 @@ function getSetTimeout(windowObject) { function getClearTimeout(windowObject) { if (global.useFakeTimers) { - if (typeof windowObject !== 'undefined') { + if (typeof windowObject !== 'undefined' && windowObject !== false) { return windowObject.clearTimeout } From ce83f8f7e2fef8ea14915dee1df7832310caec38 Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Tue, 9 Jul 2019 10:28:06 +0200 Subject: [PATCH 04/12] simplified getSetTimeout and getClearTimeout --- src/helpers.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/helpers.js b/src/helpers.js index 3f2cbb4c..fcad8ba8 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -56,11 +56,7 @@ if (typeof window !== 'undefined') { */ function getSetTimeout(windowObject) { if (global.useFakeTimers) { - if (typeof windowObject !== 'undefined' && windowObject !== false) { - return windowObject.setTimeout - } - - return global.setTimeout + return windowObject ? window.setTimeout : global.setTimeout } return originalSetTimeout @@ -68,11 +64,7 @@ function getSetTimeout(windowObject) { function getClearTimeout(windowObject) { if (global.useFakeTimers) { - if (typeof windowObject !== 'undefined' && windowObject !== false) { - return windowObject.clearTimeout - } - - return global.clearTimeout + return windowObject ? window.clearTimeout : global.clearTimeout } return originalClearTimeout From 060eafd3d24e845501e13304675b0df19495e3cb Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Wed, 10 Jul 2019 10:21:16 +0200 Subject: [PATCH 05/12] updated tests for the Node environment, removed dependency injection for window --- src/__node_tests__/index.js | 47 +++++++++++++++++++++++++++ src/__tests__/helpers.js | 21 +++--------- src/helpers.js | 12 ++++--- src/wait-for-dom-change.js | 6 ++-- src/wait-for-element-to-be-removed.js | 6 ++-- src/wait-for-element.js | 6 ++-- tests/jest.config.node.js | 2 +- tests/setup-env-node.js | 3 ++ 8 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 tests/setup-env-node.js diff --git a/src/__node_tests__/index.js b/src/__node_tests__/index.js index fabe3534..cb19e14c 100644 --- a/src/__node_tests__/index.js +++ b/src/__node_tests__/index.js @@ -76,3 +76,50 @@ test('works without a browser context on a dom node (JSDOM Fragment)', () => { /> `) }) + +describe('getClearTimeout', () => { + beforeEach(jest.resetModules) + + test('returns original clearTimeout from global if window is undefined', () => { + global.useFakeTimers = false + const {getClearTimeout} = require('../helpers') + const clearTimeout = getClearTimeout() + + expect(clearTimeout).toBe(global.clearTimeout) + expect(clearTimeout._isMockFunction).toBe(undefined) + }) + test('returns mocked clearTimeout if jest.useFaketimers is used', () => { + jest.useFakeTimers() + const {getClearTimeout} = require('../helpers') + + const clearTimeout = getClearTimeout() + + expect(clearTimeout).toBe(global.clearTimeout) + expect(clearTimeout._isMockFunction).toBe(true) + + jest.useRealTimers() + }) +}) + +describe('getSetTimeout', () => { + beforeEach(jest.resetModules) + + test('returns original getSetTimeout from global if window is undefined', () => { + global.useFakeTimers = false + const {getSetTimeout} = require('../helpers') + const setTimeout = getSetTimeout() + + expect(setTimeout).toBe(global.setTimeout) + expect(setTimeout._isMockFunction).toBe(undefined) + }) + test('returns mocked getSetTimeout from global jest.useFaketimers is used', () => { + jest.useFakeTimers() + const {getSetTimeout} = require('../helpers') + const setTimeout = getSetTimeout() + + expect(setTimeout).toBe(global.setTimeout) + expect(setTimeout._isMockFunction).toBe(true) + + jest.useRealTimers() + }) +}) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index f69024d1..a22baaa0 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -14,24 +14,19 @@ describe('getSetTimeout', () => { // global.useFakeTimers is set to true for all tests in tests/setup-env.js jest.useFakeTimers() - const setTimeout = getSetTimeout(window) + const setTimeout = getSetTimeout() + expect(setTimeout).toBe(window.setTimeout) expect(setTimeout._isMockFunction).toBe(true) jest.useRealTimers() }) it('returns original getSetTimeout from window', () => { - const setTimeout = getSetTimeout(window) + const setTimeout = getSetTimeout() expect(setTimeout).toBe(window.setTimeout) expect(setTimeout._isMockFunction).toBe(undefined) }) - it('returns original getSetTimeout from global if window is undefined', () => { - const setTimeout = getSetTimeout(undefined) - - expect(setTimeout).toBe(global.setTimeout) - expect(setTimeout._isMockFunction).toBe(undefined) - }) }) describe('getClearTimeout', () => { @@ -39,24 +34,18 @@ describe('getClearTimeout', () => { // global.useFakeTimers is set to true for all tests in tests/setup-env.js jest.useFakeTimers() - const clearTimeout = getClearTimeout(window) + const clearTimeout = getClearTimeout() expect(clearTimeout._isMockFunction).toBe(true) jest.useRealTimers() }) it('returns original clearTimeout from window', () => { - const clearTimeout = getClearTimeout(window) + const clearTimeout = getClearTimeout() expect(clearTimeout).toBe(window.clearTimeout) expect(clearTimeout._isMockFunction).toBe(undefined) }) - it('returns original clearTimeout from global if window is undefined', () => { - const clearTimeout = getClearTimeout(undefined) - - expect(clearTimeout).toBe(global.clearTimeout) - expect(clearTimeout._isMockFunction).toBe(undefined) - }) }) class DummyClass { diff --git a/src/helpers.js b/src/helpers.js index fcad8ba8..ea3061a1 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -54,17 +54,21 @@ if (typeof window !== 'undefined') { * * see: https://github.com/testing-library/dom-testing-library/issues/300 */ -function getSetTimeout(windowObject) { +function getSetTimeout() { if (global.useFakeTimers) { - return windowObject ? window.setTimeout : global.setTimeout + // eslint-disable-next-line no-negated-condition + return typeof window !== 'undefined' ? window.setTimeout : global.setTimeout } return originalSetTimeout } -function getClearTimeout(windowObject) { +function getClearTimeout() { if (global.useFakeTimers) { - return windowObject ? window.clearTimeout : global.clearTimeout + // eslint-disable-next-line no-negated-condition + return typeof window !== 'undefined' + ? window.clearTimeout + : global.clearTimeout } return originalClearTimeout diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index 778b0b96..47c7a714 100644 --- a/src/wait-for-dom-change.js +++ b/src/wait-for-dom-change.js @@ -19,10 +19,8 @@ function waitForDomChange({ } = {}) { return new Promise((resolve, reject) => { const setImmediate = getSetImmediate() - const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) - const clearTimeout = getClearTimeout( - typeof window !== 'undefined' && window, - ) + const setTimeout = getSetTimeout() + const clearTimeout = getClearTimeout() const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 9091ba03..999bd411 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/src/wait-for-element-to-be-removed.js @@ -28,10 +28,8 @@ function waitForElementToBeRemoved( ), ) } - const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) - const clearTimeout = getClearTimeout( - typeof window !== 'undefined' && window, - ) + const setTimeout = getSetTimeout() + const clearTimeout = getClearTimeout() const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) diff --git a/src/wait-for-element.js b/src/wait-for-element.js index ce15775f..178df703 100644 --- a/src/wait-for-element.js +++ b/src/wait-for-element.js @@ -28,10 +28,8 @@ function waitForElement( return } let lastError - const setTimeout = getSetTimeout(typeof window !== 'undefined' && window) - const clearTimeout = getClearTimeout( - typeof window !== 'undefined' && window, - ) + const setTimeout = getSetTimeout() + const clearTimeout = getClearTimeout() const timer = setTimeout(onTimeout, timeout) diff --git a/tests/jest.config.node.js b/tests/jest.config.node.js index 26741820..7a0a6547 100644 --- a/tests/jest.config.node.js +++ b/tests/jest.config.node.js @@ -4,7 +4,7 @@ const baseConfig = require('kcd-scripts/jest') module.exports = { ...baseConfig, rootDir: path.join(__dirname, '..'), - setupFilesAfterEnv: undefined, + setupFilesAfterEnv: ['/tests/setup-env-node.js'], displayName: 'node', testEnvironment: 'jest-environment-node', testMatch: ['**/__node_tests__/**.js'], diff --git a/tests/setup-env-node.js b/tests/setup-env-node.js new file mode 100644 index 00000000..f42d9184 --- /dev/null +++ b/tests/setup-env-node.js @@ -0,0 +1,3 @@ +// Make sure that our tests do the use fakeTimer mock: +// https://github.com/testing-library/dom-testing-library/issues/300 +global.useFakeTimers = true From 9d7f0878c6311780c11f5582b1c1208c10fad3cb Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Wed, 10 Jul 2019 10:22:11 +0200 Subject: [PATCH 06/12] updated test name --- src/__node_tests__/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__node_tests__/index.js b/src/__node_tests__/index.js index cb19e14c..7a5ae2dd 100644 --- a/src/__node_tests__/index.js +++ b/src/__node_tests__/index.js @@ -88,7 +88,7 @@ describe('getClearTimeout', () => { expect(clearTimeout).toBe(global.clearTimeout) expect(clearTimeout._isMockFunction).toBe(undefined) }) - test('returns mocked clearTimeout if jest.useFaketimers is used', () => { + test('returns mocked clearTimeout from global if jest.useFaketimers is used', () => { jest.useFakeTimers() const {getClearTimeout} = require('../helpers') @@ -112,7 +112,7 @@ describe('getSetTimeout', () => { expect(setTimeout).toBe(global.setTimeout) expect(setTimeout._isMockFunction).toBe(undefined) }) - test('returns mocked getSetTimeout from global jest.useFaketimers is used', () => { + test('returns mocked getSetTimeout from global if jest.useFaketimers is used', () => { jest.useFakeTimers() const {getSetTimeout} = require('../helpers') const setTimeout = getSetTimeout() From a13396adf9feac9b15ca759465a72cc1e472a94e Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Wed, 10 Jul 2019 17:09:17 +0200 Subject: [PATCH 07/12] Update src/helpers.js Co-Authored-By: Kent C. Dodds --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index ea3061a1..177c3d2e 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -48,7 +48,7 @@ if (typeof window !== 'undefined') { /* * Return the original setTimeout function so that all functions work as expected by a user - * if he/she utilizes dom-testing-library in a test where jest.fakeTimers() is used. + * if they utilize dom-testing-library in a test where jest.fakeTimers() is used. * * global.useFakeTimers is used by us to make sure that we can still utilize fakeTimers in our tests. * From 8eccbb4cd87eae9cf2b794a47001cdf59d96e4ab Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Thu, 11 Jul 2019 17:18:05 +0200 Subject: [PATCH 08/12] Update src/helpers.js Co-Authored-By: Kent C. Dodds --- src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers.js b/src/helpers.js index 177c3d2e..ef99f15d 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -57,7 +57,7 @@ if (typeof window !== 'undefined') { function getSetTimeout() { if (global.useFakeTimers) { // eslint-disable-next-line no-negated-condition - return typeof window !== 'undefined' ? window.setTimeout : global.setTimeout + return typeof window === 'undefined' ? global.setTimeout : window.setTimeout } return originalSetTimeout From 0293c3797c590aec7ab3b52e076042547105888d Mon Sep 17 00:00:00 2001 From: Laurens Bosscher Date: Thu, 11 Jul 2019 17:31:34 +0200 Subject: [PATCH 09/12] updated with feedback from Kent --- src/helpers.js | 8 +++----- tests/setup-env.js | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/helpers.js b/src/helpers.js index ef99f15d..f12c50b4 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -56,7 +56,6 @@ if (typeof window !== 'undefined') { */ function getSetTimeout() { if (global.useFakeTimers) { - // eslint-disable-next-line no-negated-condition return typeof window === 'undefined' ? global.setTimeout : window.setTimeout } @@ -65,10 +64,9 @@ function getSetTimeout() { function getClearTimeout() { if (global.useFakeTimers) { - // eslint-disable-next-line no-negated-condition - return typeof window !== 'undefined' - ? window.clearTimeout - : global.clearTimeout + return typeof window === 'undefined' + ? global.clearTimeout + : window.clearTimeout } return originalClearTimeout diff --git a/tests/setup-env.js b/tests/setup-env.js index 303b55f4..73cc0eba 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,8 +1,8 @@ import 'jest-dom/extend-expect' import jestSerializerAnsi from 'jest-serializer-ansi' -expect.addSnapshotSerializer(jestSerializerAnsi) - // Make sure that our tests do the use fakeTimer mock: // https://github.com/testing-library/dom-testing-library/issues/300 -global.useFakeTimers = true \ No newline at end of file +global.useFakeTimers = true + +expect.addSnapshotSerializer(jestSerializerAnsi) From 39357b63a936770dfdbb27301540657e793fffdb Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 11 Jul 2019 16:18:46 -0600 Subject: [PATCH 10/12] do some fancy magic stuff --- src/__node_tests__/index.js | 47 ---------- src/__tests__/fake-timers.js | 85 +++++++++++++++++++ src/__tests__/helpers.js | 46 +--------- src/__tests__/wait-for-dom-change.js | 26 ------ ...t-for-element-to-be-removed.fake-timers.js | 39 --------- .../wait-for-element-to-be-removed.js | 24 ++++++ src/__tests__/wait-for-element.js | 26 ------ src/helpers.js | 72 ++++------------ src/wait-for-dom-change.js | 10 +-- src/wait-for-element-to-be-removed.js | 9 +- src/wait-for-element.js | 10 +-- tests/setup-env-node.js | 3 - tests/setup-env.js | 4 - 13 files changed, 134 insertions(+), 267 deletions(-) create mode 100644 src/__tests__/fake-timers.js delete mode 100644 src/__tests__/wait-for-element-to-be-removed.fake-timers.js diff --git a/src/__node_tests__/index.js b/src/__node_tests__/index.js index 7a5ae2dd..fabe3534 100644 --- a/src/__node_tests__/index.js +++ b/src/__node_tests__/index.js @@ -76,50 +76,3 @@ test('works without a browser context on a dom node (JSDOM Fragment)', () => { /> `) }) - -describe('getClearTimeout', () => { - beforeEach(jest.resetModules) - - test('returns original clearTimeout from global if window is undefined', () => { - global.useFakeTimers = false - const {getClearTimeout} = require('../helpers') - const clearTimeout = getClearTimeout() - - expect(clearTimeout).toBe(global.clearTimeout) - expect(clearTimeout._isMockFunction).toBe(undefined) - }) - test('returns mocked clearTimeout from global if jest.useFaketimers is used', () => { - jest.useFakeTimers() - const {getClearTimeout} = require('../helpers') - - const clearTimeout = getClearTimeout() - - expect(clearTimeout).toBe(global.clearTimeout) - expect(clearTimeout._isMockFunction).toBe(true) - - jest.useRealTimers() - }) -}) - -describe('getSetTimeout', () => { - beforeEach(jest.resetModules) - - test('returns original getSetTimeout from global if window is undefined', () => { - global.useFakeTimers = false - const {getSetTimeout} = require('../helpers') - const setTimeout = getSetTimeout() - - expect(setTimeout).toBe(global.setTimeout) - expect(setTimeout._isMockFunction).toBe(undefined) - }) - test('returns mocked getSetTimeout from global if jest.useFaketimers is used', () => { - jest.useFakeTimers() - const {getSetTimeout} = require('../helpers') - const setTimeout = getSetTimeout() - - expect(setTimeout).toBe(global.setTimeout) - expect(setTimeout._isMockFunction).toBe(true) - - jest.useRealTimers() - }) -}) diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js new file mode 100644 index 00000000..b5fa6289 --- /dev/null +++ b/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(`
`) + 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) +}) diff --git a/src/__tests__/helpers.js b/src/__tests__/helpers.js index a22baaa0..cbf6e628 100644 --- a/src/__tests__/helpers.js +++ b/src/__tests__/helpers.js @@ -1,53 +1,9 @@ -import { - getDocument, - newMutationObserver, - getSetTimeout, - getClearTimeout, -} from '../helpers' +import {getDocument, newMutationObserver} from '../helpers' test('returns global document if exists', () => { expect(getDocument()).toBe(document) }) -describe('getSetTimeout', () => { - it('returns mocked setTimeout if global.useFakeTimers is set and jest.useFakeTimers is used', () => { - // global.useFakeTimers is set to true for all tests in tests/setup-env.js - jest.useFakeTimers() - - const setTimeout = getSetTimeout() - - expect(setTimeout).toBe(window.setTimeout) - expect(setTimeout._isMockFunction).toBe(true) - - jest.useRealTimers() - }) - it('returns original getSetTimeout from window', () => { - const setTimeout = getSetTimeout() - - expect(setTimeout).toBe(window.setTimeout) - expect(setTimeout._isMockFunction).toBe(undefined) - }) -}) - -describe('getClearTimeout', () => { - it('returns mocked clearTimeout if global.useFakeTimers is set and jest.useFakeTimers is used', () => { - // global.useFakeTimers is set to true for all tests in tests/setup-env.js - jest.useFakeTimers() - - const clearTimeout = getClearTimeout() - - expect(clearTimeout._isMockFunction).toBe(true) - - jest.useRealTimers() - }) - it('returns original clearTimeout from window', () => { - const clearTimeout = getClearTimeout() - - expect(clearTimeout).toBe(window.clearTimeout) - expect(clearTimeout._isMockFunction).toBe(undefined) - }) -}) - class DummyClass { constructor(args) { this.args = args diff --git a/src/__tests__/wait-for-dom-change.js b/src/__tests__/wait-for-dom-change.js index d54781dd..f5ae91ae 100644 --- a/src/__tests__/wait-for-dom-change.js +++ b/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() -}) diff --git a/src/__tests__/wait-for-element-to-be-removed.fake-timers.js b/src/__tests__/wait-for-element-to-be-removed.fake-timers.js deleted file mode 100644 index a6744d17..00000000 --- a/src/__tests__/wait-for-element-to-be-removed.fake-timers.js +++ /dev/null @@ -1,39 +0,0 @@ -import {waitForElementToBeRemoved} from '../' -import {render} from './helpers/test-utils' - -jest.useFakeTimers() - -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."`, - ) -}) - -test('times out after 4500ms by default', () => { - const {container} = render(`
`) - const promise = expect( - waitForElementToBeRemoved(() => container), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"Timed out in waitForElementToBeRemoved."`, - ) - jest.advanceTimersByTime(4501) - return promise -}) diff --git a/src/__tests__/wait-for-element-to-be-removed.js b/src/__tests__/wait-for-element-to-be-removed.js index 3eeaff0a..e541d86f 100644 --- a/src/__tests__/wait-for-element-to-be-removed.js +++ b/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."`, + ) +}) diff --git a/src/__tests__/wait-for-element.js b/src/__tests__/wait-for-element.js index c15b6caa..e9ec640f 100644 --- a/src/__tests__/wait-for-element.js +++ b/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('
') let error diff --git a/src/helpers.js b/src/helpers.js index f12c50b4..cc3c52c2 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -1,5 +1,17 @@ import MutationObserver from '@sheerun/mutationobserver-shim' +const globalObj = typeof window === 'undefined' ? global : window + +function setImmediatePolyfill(fn) { + return globalObj.setTimeout(fn, 0) +} + +const { + setTimeout, + clearTimeout, + setImmediate = setImmediatePolyfill, +} = globalObj + function newMutationObserver(onMutation) { const MutationObserverConstructor = typeof window !== 'undefined' && @@ -18,64 +30,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) - } - } -} - -let originalSetTimeout = global.setTimeout - -if (typeof window !== 'undefined') { - originalSetTimeout = window.setTimeout -} - -let originalClearTimeout = global.clearTimeout - -if (typeof window !== 'undefined') { - originalClearTimeout = window.clearTimeout -} - -/* - * Return the original setTimeout function so that all functions work as expected by a user - * if they utilize dom-testing-library in a test where jest.fakeTimers() is used. - * - * global.useFakeTimers is used by us to make sure that we can still utilize fakeTimers in our tests. - * - * see: https://github.com/testing-library/dom-testing-library/issues/300 - */ -function getSetTimeout() { - if (global.useFakeTimers) { - return typeof window === 'undefined' ? global.setTimeout : window.setTimeout - } - - return originalSetTimeout -} - -function getClearTimeout() { - if (global.useFakeTimers) { - return typeof window === 'undefined' - ? global.clearTimeout - : window.clearTimeout - } - - return originalClearTimeout -} - export { getDocument, newMutationObserver, - getSetImmediate, - getSetTimeout, - getClearTimeout, + setImmediate, + setTimeout, + clearTimeout, } diff --git a/src/wait-for-dom-change.js b/src/wait-for-dom-change.js index 47c7a714..86f3c7df 100644 --- a/src/wait-for-dom-change.js +++ b/src/wait-for-dom-change.js @@ -1,9 +1,9 @@ import { newMutationObserver, getDocument, - getSetImmediate, - getSetTimeout, - getClearTimeout, + setImmediate, + setTimeout, + clearTimeout, } from './helpers' import {getConfig} from './config' @@ -18,10 +18,6 @@ function waitForDomChange({ }, } = {}) { return new Promise((resolve, reject) => { - const setImmediate = getSetImmediate() - const setTimeout = getSetTimeout() - const clearTimeout = getClearTimeout() - const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) observer.observe(container, mutationObserverOptions) diff --git a/src/wait-for-element-to-be-removed.js b/src/wait-for-element-to-be-removed.js index 999bd411..e269599e 100644 --- a/src/wait-for-element-to-be-removed.js +++ b/src/wait-for-element-to-be-removed.js @@ -1,9 +1,9 @@ import { getDocument, - getSetImmediate, newMutationObserver, - getSetTimeout, - getClearTimeout, + setImmediate, + setTimeout, + clearTimeout, } from './helpers' import {getConfig} from './config' @@ -28,8 +28,6 @@ function waitForElementToBeRemoved( ), ) } - const setTimeout = getSetTimeout() - const clearTimeout = getClearTimeout() const timer = setTimeout(onTimeout, timeout) const observer = newMutationObserver(onMutation) @@ -52,7 +50,6 @@ function waitForElementToBeRemoved( } function onDone(error, result) { - const setImmediate = getSetImmediate() clearTimeout(timer) setImmediate(() => observer.disconnect()) if (error) { diff --git a/src/wait-for-element.js b/src/wait-for-element.js index 178df703..5dc676e3 100644 --- a/src/wait-for-element.js +++ b/src/wait-for-element.js @@ -1,9 +1,9 @@ import { newMutationObserver, getDocument, - getSetImmediate, - getSetTimeout, - getClearTimeout, + setImmediate, + setTimeout, + clearTimeout, } from './helpers' import {getConfig} from './config' @@ -28,15 +28,11 @@ function waitForElement( return } let lastError - const setTimeout = getSetTimeout() - const clearTimeout = getClearTimeout() - 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) { diff --git a/tests/setup-env-node.js b/tests/setup-env-node.js index f42d9184..e69de29b 100644 --- a/tests/setup-env-node.js +++ b/tests/setup-env-node.js @@ -1,3 +0,0 @@ -// Make sure that our tests do the use fakeTimer mock: -// https://github.com/testing-library/dom-testing-library/issues/300 -global.useFakeTimers = true diff --git a/tests/setup-env.js b/tests/setup-env.js index 73cc0eba..1ced399a 100644 --- a/tests/setup-env.js +++ b/tests/setup-env.js @@ -1,8 +1,4 @@ import 'jest-dom/extend-expect' import jestSerializerAnsi from 'jest-serializer-ansi' -// Make sure that our tests do the use fakeTimer mock: -// https://github.com/testing-library/dom-testing-library/issues/300 -global.useFakeTimers = true - expect.addSnapshotSerializer(jestSerializerAnsi) From f79a2d5e2497af4dfec74010aca93a98252652de Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 11 Jul 2019 16:22:10 -0600 Subject: [PATCH 11/12] no longer needed --- tests/jest.config.node.js | 1 - tests/setup-env-node.js | 0 2 files changed, 1 deletion(-) delete mode 100644 tests/setup-env-node.js diff --git a/tests/jest.config.node.js b/tests/jest.config.node.js index 7a0a6547..4e30443d 100644 --- a/tests/jest.config.node.js +++ b/tests/jest.config.node.js @@ -4,7 +4,6 @@ const baseConfig = require('kcd-scripts/jest') module.exports = { ...baseConfig, rootDir: path.join(__dirname, '..'), - setupFilesAfterEnv: ['/tests/setup-env-node.js'], displayName: 'node', testEnvironment: 'jest-environment-node', testMatch: ['**/__node_tests__/**.js'], diff --git a/tests/setup-env-node.js b/tests/setup-env-node.js deleted file mode 100644 index e69de29b..00000000 From f53c2143553d81c56261fe3d11babd292dac7f24 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 11 Jul 2019 16:26:19 -0600 Subject: [PATCH 12/12] fix coverage --- src/helpers.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/helpers.js b/src/helpers.js index cc3c52c2..354911bf 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -2,10 +2,13 @@ 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,