Skip to content

Commit

Permalink
jest: Make "modern" fake-timer implementation the default.
Browse files Browse the repository at this point in the history
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 fb23341 about it
being silently not applied until we added @jest/source-map as a
direct dependency.

We have an ESLint rule, from 2faad06, 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`.
  • Loading branch information
chrisbobbe committed May 20, 2021
1 parent 44fcc9d commit 2415359
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ overrides:
#
# ================================================================
# Our test suite.
- files: ['**/__tests__/**']
- files: ['**/__tests__/**', 'jest/jestSetup.js']
rules:
no-restricted-imports: off

Expand Down
2 changes: 2 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ const projectForPlatform = platform => {
// 3) assign `global.Promise` back to what we saved in step 1
preset: platform === 'ios' ? './jest/presetIos' : './jest/presetAndroid',

timers: 'modern',

// Finding and transforming source code.
testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'],

Expand Down
7 changes: 7 additions & 0 deletions jest/jestSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { polyfillGlobal } from 'react-native/Libraries/Utilities/PolyfillFunctio
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
Expand All @@ -18,6 +20,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)
Expand Down
29 changes: 26 additions & 3 deletions src/__tests__/lib/fakeTimers.js
Original file line number Diff line number Diff line change
@@ -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<void> => {
// 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;
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/metatests.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions src/api/__tests__/queueMarkAsRead-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ 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();
Expand Down
2 changes: 0 additions & 2 deletions src/message/__tests__/fetchActions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ describe('fetchActions', () => {

describe('tryFetch', () => {
beforeAll(() => {
jest.useFakeTimers('modern');

// So we don't have to think about the (random, with jitter)
// duration of these waits in these tests. `BackoffMachine` has
// its own unit tests already, so we don't have to test that it
Expand Down
5 changes: 0 additions & 5 deletions src/presence/__tests__/heartbeat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ describe('Heartbeat', () => {
// ===================================================================
// Jest hooks

// before running tests: set up fake timer API
beforeAll(() => {
jest.useFakeTimers('modern');
});

afterAll(() => {
JestHeartbeatHelper.clearExtant();
});
Expand Down
3 changes: 2 additions & 1 deletion src/utils/__tests__/async-test.js
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -11,7 +12,7 @@ const sleepMeasure = async (expectedMs: number) => {

describe('sleep (ideal)', () => {
beforeAll(() => {
jest.useFakeTimers('modern');
assertUsingModernFakeTimers();
});

afterEach(() => {
Expand Down
2 changes: 0 additions & 2 deletions src/utils/__tests__/backoffMachine-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions src/utils/__tests__/promiseTimeout-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2415359

Please sign in to comment.