Skip to content

Commit 33289b5

Browse files
authored
Tests and fixes for 'timing out' behavior (#12858)
**what is the change?:** Test coverage checking that callbacks are called when they time out. This test surfaced a bug and this commit includes the fix. I want to refine this approach, but basically we can simulate time outs by controlling the return value of 'now()' and the argument passed to the rAF callback. Next we will write a browser fixture to further test this against real browser APIs. **why make this change?:** Better tests will keep this module working smoothly while we continue refactoring and improving it. **test plan:** Run the new tests, see that it fails without the bug fix.
1 parent ad27845 commit 33289b5

2 files changed

Lines changed: 122 additions & 5 deletions

File tree

packages/react-scheduler/src/ReactScheduler.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,10 @@ if (!ExecutionEnvironment.canUseDOM) {
276276
if (options != null && typeof options.timeout === 'number') {
277277
timeoutTime = now() + options.timeout;
278278
}
279-
if (timeoutTime > nextSoonestTimeoutTime) {
279+
if (
280+
nextSoonestTimeoutTime === -1 ||
281+
(timeoutTime !== -1 && timeoutTime < nextSoonestTimeoutTime)
282+
) {
280283
nextSoonestTimeoutTime = timeoutTime;
281284
}
282285

packages/react-scheduler/src/__tests__/ReactScheduler-test.js

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,43 @@
1212
let ReactScheduler;
1313

1414
describe('ReactScheduler', () => {
15+
let rAFCallbacks = [];
1516
let postMessageCallback;
1617
let postMessageEvents = [];
1718

1819
function drainPostMessageQueue() {
20+
// default to this falling about 15 ms before next frame
21+
currentTime = startOfLatestFrame + frameSize - 15;
1922
if (postMessageCallback) {
2023
while (postMessageEvents.length) {
2124
postMessageCallback(postMessageEvents.shift());
2225
}
2326
}
2427
}
28+
function runRAFCallbacks() {
29+
startOfLatestFrame += frameSize;
30+
currentTime = startOfLatestFrame;
31+
rAFCallbacks.forEach(cb => cb());
32+
rAFCallbacks = [];
33+
}
2534
function advanceAll() {
26-
jest.runAllTimers();
35+
runRAFCallbacks();
2736
drainPostMessageQueue();
2837
}
38+
39+
let frameSize = 33;
40+
let startOfLatestFrame = Date.now();
41+
let currentTime = Date.now();
42+
2943
beforeEach(() => {
3044
// TODO pull this into helper method, reduce repetition.
3145
// mock the browser APIs which are used in react-scheduler:
3246
// - requestAnimationFrame should pass the DOMHighResTimeStamp argument
3347
// - calling 'window.postMessage' should actually fire postmessage handlers
48+
// - Date.now should return the correct thing
3449
global.requestAnimationFrame = function(cb) {
35-
return setTimeout(() => {
36-
cb(Date.now());
50+
return rAFCallbacks.push(() => {
51+
cb(startOfLatestFrame);
3752
});
3853
};
3954
const originalAddEventListener = global.addEventListener;
@@ -50,6 +65,9 @@ describe('ReactScheduler', () => {
5065
const postMessageEvent = {source: window, data: messageKey};
5166
postMessageEvents.push(postMessageEvent);
5267
};
68+
global.Date.now = function() {
69+
return currentTime;
70+
};
5371
jest.resetModules();
5472
ReactScheduler = require('react-scheduler');
5573
});
@@ -102,7 +120,7 @@ describe('ReactScheduler', () => {
102120
scheduleWork(callbackA);
103121
// initially waits to call the callback
104122
expect(callbackLog).toEqual([]);
105-
jest.runAllTimers();
123+
runRAFCallbacks();
106124
// this should schedule work *after* the requestAnimationFrame but before the message handler
107125
scheduleWork(callbackB);
108126
expect(callbackLog).toEqual([]);
@@ -208,6 +226,102 @@ describe('ReactScheduler', () => {
208226
expect(callbackLog).toEqual(['A0', 'B', 'A1']);
209227
});
210228
});
229+
230+
describe('when callbacks time out: ', () => {
231+
// USEFUL INFO:
232+
// startOfLatestFrame is a global that goes up every time rAF runs
233+
// currentTime defaults to startOfLatestFrame inside rAF callback
234+
// and currentTime defaults to 15 before next frame inside idleTick
235+
236+
describe('when there is no more time left in the frame', () => {
237+
it('calls any callback which has timed out, waits for others', () => {
238+
const {scheduleWork} = ReactScheduler;
239+
startOfLatestFrame = 1000000000000;
240+
currentTime = startOfLatestFrame - 10;
241+
const callbackLog = [];
242+
// simple case of one callback which times out, another that won't.
243+
const callbackA = jest.fn(() => callbackLog.push('A'));
244+
const callbackB = jest.fn(() => callbackLog.push('B'));
245+
const callbackC = jest.fn(() => callbackLog.push('C'));
246+
247+
scheduleWork(callbackA); // won't time out
248+
scheduleWork(callbackB, {timeout: 100}); // times out later
249+
scheduleWork(callbackC, {timeout: 2}); // will time out fast
250+
251+
runRAFCallbacks(); // runs rAF callback
252+
// push time ahead a bit so that we have no idle time
253+
startOfLatestFrame += 16;
254+
drainPostMessageQueue(); // runs postMessage callback, idleTick
255+
256+
// callbackC should have timed out
257+
expect(callbackLog).toEqual(['C']);
258+
259+
runRAFCallbacks(); // runs rAF callback
260+
// push time ahead a bit so that we have no idle time
261+
startOfLatestFrame += 16;
262+
drainPostMessageQueue(); // runs postMessage callback, idleTick
263+
264+
// callbackB should have timed out
265+
expect(callbackLog).toEqual(['C', 'B']);
266+
267+
runRAFCallbacks(); // runs rAF callback
268+
drainPostMessageQueue(); // runs postMessage callback, idleTick
269+
270+
// we should have run callbackA in the idle time
271+
expect(callbackLog).toEqual(['C', 'B', 'A']);
272+
});
273+
});
274+
275+
describe('when there is some time left in the frame', () => {
276+
it('calls timed out callbacks and then any more pending callbacks, defers others if time runs out', () => {
277+
// TODO first call timed out callbacks
278+
// then any non-timed out callbacks if there is time
279+
const {scheduleWork} = ReactScheduler;
280+
startOfLatestFrame = 1000000000000;
281+
currentTime = startOfLatestFrame - 10;
282+
const callbackLog = [];
283+
// simple case of one callback which times out, others that won't.
284+
const callbackA = jest.fn(() => {
285+
callbackLog.push('A');
286+
// time passes, causing us to run out of idle time
287+
currentTime += 25;
288+
});
289+
const callbackB = jest.fn(() => callbackLog.push('B'));
290+
const callbackC = jest.fn(() => callbackLog.push('C'));
291+
const callbackD = jest.fn(() => callbackLog.push('D'));
292+
293+
scheduleWork(callbackA); // won't time out
294+
scheduleWork(callbackB, {timeout: 100}); // times out later
295+
scheduleWork(callbackC, {timeout: 2}); // will time out fast
296+
scheduleWork(callbackD); // won't time out
297+
298+
advanceAll(); // runs rAF and postMessage callbacks
299+
300+
// callbackC should have timed out
301+
// we should have had time to call A also, then we run out of time
302+
expect(callbackLog).toEqual(['C', 'A']);
303+
304+
runRAFCallbacks(); // runs rAF callback
305+
// push time ahead a bit so that we have no idle time
306+
startOfLatestFrame += 16;
307+
drainPostMessageQueue(); // runs postMessage callback, idleTick
308+
309+
// callbackB should have timed out
310+
// but we should not run callbackD because we have no idle time
311+
expect(callbackLog).toEqual(['C', 'A', 'B']);
312+
313+
advanceAll(); // runs rAF and postMessage callbacks
314+
315+
// we should have run callbackD in the idle time
316+
expect(callbackLog).toEqual(['C', 'A', 'B', 'D']);
317+
318+
advanceAll(); // runs rAF and postMessage callbacks
319+
320+
// we should not have run anything again, nothing is scheduled
321+
expect(callbackLog).toEqual(['C', 'A', 'B', 'D']);
322+
});
323+
});
324+
});
211325
});
212326

213327
describe('cancelScheduledWork', () => {

0 commit comments

Comments
 (0)