From 5b7e2f5e635b0f540374a986db9ce0c1a8b30dcf Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 25 Aug 2020 14:22:52 -0700 Subject: [PATCH] jest: Make "modern" fake-timer implementation the default. Also, remove several now-unnecessary calls of `jest.useFakeTimers('modern')`, but keep a few assertions that the "modern" timers are actually being used. In particular, our `jestSetup` is a central place where we make the assertion. Not only is it good to check that we still intentionally set the "modern" implementation, but we want to make sure that the setting is correctly applied. See the note in fb23341c3 about it being silently not applied until we added @jest/source-map as a direct dependency. We have an ESLint rule, from 2faad06a8, preventing imports from '**/__tests__/**'; the rule is active in all files not matching that same pattern. Add an additional override so that we can make the "modern"-timers assertion from within `jest/jestSetup.js`. --- .eslintrc.yaml | 2 +- jest.config.js | 2 ++ jest/jestSetup.js | 8 +++++- src/__tests__/lib/fakeTimers.js | 29 +++++++++++++++++++--- src/__tests__/metatests.js | 6 ++--- src/api/__tests__/queueMarkAsRead-test.js | 4 --- src/message/__tests__/fetchActions-test.js | 4 --- src/presence/__tests__/heartbeat-test.js | 5 ---- src/utils/__tests__/async-test.js | 3 ++- src/utils/__tests__/backoffMachine-test.js | 2 -- src/utils/__tests__/promiseTimeout-test.js | 4 --- 11 files changed, 41 insertions(+), 28 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 05f2df46cf6..60aad3b3f11 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -326,7 +326,7 @@ overrides: # # ================================================================ # Our test suite. - - files: ['**/__tests__/**'] + - files: ['**/__tests__/**', 'jest/jestSetup.js'] rules: no-restricted-imports: off diff --git a/jest.config.js b/jest.config.js index d6c126982a7..aa184a3740c 100644 --- a/jest.config.js +++ b/jest.config.js @@ -26,6 +26,8 @@ const transformIgnorePattern = `node_modules/(?!${transformModulesWhitelist.join module.exports = { preset: './jest/preset.js', + timers: 'modern', + // Finding and transforming source code. testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'], diff --git a/jest/jestSetup.js b/jest/jestSetup.js index 8487db8412c..519b321f03c 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -1,9 +1,10 @@ import * as ReactNative from 'react-native'; import { polyfillGlobal } from 'react-native/Libraries/Utilities/PolyfillFunctions'; import { URL, URLSearchParams } from 'react-native-url-polyfill'; - import mockAsyncStorage from '@react-native-community/async-storage/jest/async-storage-mock'; +import { assertUsingModernFakeTimers } from '../src/__tests__/lib/fakeTimers'; + // Use the same `URL` polyfill we do in the app. // // In the app we let `react-native-url-polyfill` handle doing this, by @@ -17,6 +18,11 @@ import mockAsyncStorage from '@react-native-community/async-storage/jest/async-s polyfillGlobal('URL', () => URL); polyfillGlobal('URLSearchParams', () => URLSearchParams); +/** + * Default should be set with `timers: 'modern'` in Jest config. + */ +assertUsingModernFakeTimers(); + // Mock `react-native` ourselves, following upstream advice [1] [2]. // // Note that React Native's Jest setup (in their jest/setup.js) diff --git a/src/__tests__/lib/fakeTimers.js b/src/__tests__/lib/fakeTimers.js index 4756d938d47..64464d29265 100644 --- a/src/__tests__/lib/fakeTimers.js +++ b/src/__tests__/lib/fakeTimers.js @@ -1,14 +1,37 @@ /* @flow strict-local */ import { sleep } from '../../utils/async'; +/** + * Ensure the "modern" fake-timer implementation is used. + * + * By setting `timers: 'modern'` in our Jest config, the modern + * implementation is the default. May be used to double-check that + * this default is in fact set, in our Jest setup file. + * + * Also, in one or two files, we switch over to using real timers, + * with `jest.useRealTimers()`. May be used in those files to make + * sure this setting doesn't linger where we don't want it to. + */ +export const assertUsingModernFakeTimers = () => { + // "Note: This function is only available when using modern fake + // timers implementation" + // + // -- https://jestjs.io/docs/en/jest-object#jestgetrealsystemtime + jest.getRealSystemTime(); +}; + /** * Return a promise to sleep `ms` after advancing fake timers by `ms`. */ export const fakeSleep = async (ms: number): Promise => { - // Only available if using the "modern" implementation - if (typeof jest.getRealSystemTime !== 'function') { - throw new Error("Tried to call `fakeSleep` without `jest.useFakeTimers('modern')` in effect."); + try { + assertUsingModernFakeTimers(); + } catch (e) { + throw new Error( + 'Tried to call `fakeSleep` without "modern" fake-timer implementation enabled.', + ); } + const sleepPromise = sleep(ms); jest.advanceTimersByTime(ms); return sleepPromise; diff --git a/src/__tests__/metatests.js b/src/__tests__/metatests.js index 02a54a78642..73ab99a2ac0 100644 --- a/src/__tests__/metatests.js +++ b/src/__tests__/metatests.js @@ -1,6 +1,8 @@ /** @jest-environment jest-environment-jsdom-global */ // @flow strict-local +import { assertUsingModernFakeTimers } from './lib/fakeTimers'; + /** * This file should not test any part of the application. It exists to test that * certain functionality is present in the development environment used to run @@ -75,9 +77,7 @@ describe('jsdom-global', () => { }); describe('Jest modern fake timers', () => { - jest.useFakeTimers('modern'); - // Will throw if not actually using the "modern" implementation - jest.getRealSystemTime(); + assertUsingModernFakeTimers(); afterEach(() => { // clear any unset timers diff --git a/src/api/__tests__/queueMarkAsRead-test.js b/src/api/__tests__/queueMarkAsRead-test.js index 2d5a024a59b..e1f6f43a774 100644 --- a/src/api/__tests__/queueMarkAsRead-test.js +++ b/src/api/__tests__/queueMarkAsRead-test.js @@ -7,10 +7,6 @@ import * as eg from '../../__tests__/lib/exampleData'; messagesFlags.default = jest.fn(() => {}); describe('queueMarkAsRead', () => { - jest.useFakeTimers('modern'); - // Will throw if not actually using the "modern" implementation - jest.getRealSystemTime(); - afterEach(() => { jest.clearAllMocks(); jest.clearAllTimers(); diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 30b3034243c..6eeb793f828 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -31,10 +31,6 @@ const streamNarrowStr = keyFromNarrow(narrow); global.FormData = class FormData {}; -beforeAll(() => { - jest.useFakeTimers('modern'); -}); - afterEach(() => { expect(jest.getTimerCount()).toBe(0); jest.clearAllTimers(); diff --git a/src/presence/__tests__/heartbeat-test.js b/src/presence/__tests__/heartbeat-test.js index 7bd5db5adcf..8216de85806 100644 --- a/src/presence/__tests__/heartbeat-test.js +++ b/src/presence/__tests__/heartbeat-test.js @@ -93,11 +93,6 @@ describe('Heartbeat', () => { // =================================================================== // Jest hooks - // before running tests: set up fake timer API - beforeAll(() => { - jest.useFakeTimers('modern'); - }); - afterAll(() => { JestHeartbeatHelper.clearExtant(); }); diff --git a/src/utils/__tests__/async-test.js b/src/utils/__tests__/async-test.js index 73045b540ed..a36f815d011 100644 --- a/src/utils/__tests__/async-test.js +++ b/src/utils/__tests__/async-test.js @@ -1,5 +1,6 @@ /* @flow strict-local */ import { sleep } from '../async'; +import { assertUsingModernFakeTimers } from '../../__tests__/lib/fakeTimers'; const sleepMeasure = async (expectedMs: number) => { const start = Date.now(); @@ -11,7 +12,7 @@ const sleepMeasure = async (expectedMs: number) => { describe('sleep (ideal)', () => { beforeAll(() => { - jest.useFakeTimers('modern'); + assertUsingModernFakeTimers(); }); afterEach(() => { diff --git a/src/utils/__tests__/backoffMachine-test.js b/src/utils/__tests__/backoffMachine-test.js index dad6f037b72..40548e80f07 100644 --- a/src/utils/__tests__/backoffMachine-test.js +++ b/src/utils/__tests__/backoffMachine-test.js @@ -9,8 +9,6 @@ import { BackoffMachine } from '../async'; // (https://github.com/facebook/jest/pull/8897). But as of 2020-03, putting them // in a separate file is our workaround. -jest.useFakeTimers('modern'); - describe('BackoffMachine', () => { afterEach(() => { expect(jest.getTimerCount()).toBe(0); diff --git a/src/utils/__tests__/promiseTimeout-test.js b/src/utils/__tests__/promiseTimeout-test.js index f64f1cade57..2e3cf1cfbc0 100644 --- a/src/utils/__tests__/promiseTimeout-test.js +++ b/src/utils/__tests__/promiseTimeout-test.js @@ -4,10 +4,6 @@ import { fakeSleep } from '../../__tests__/lib/fakeTimers'; const ONE_MINUTE_MS: number = 1000 * 60; -jest.useFakeTimers('modern'); -// Will throw if not actually using the "modern" implementation -jest.getRealSystemTime(); - describe('promiseTimeout', () => { afterEach(() => { // clear any unset timers