Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multiple callbacks in ReactScheduler #12682

Closed
wants to merge 14 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Assert callback order in scheduler test for multiple callback support
  • Loading branch information
flarnie committed May 2, 2018
commit 079bb73eb680aaf82ff5132d84ef426d2599597a
16 changes: 9 additions & 7 deletions packages/react-scheduler/src/__tests__/ReactScheduler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,21 @@ describe('ReactScheduler', () => {

it('rIC with multiple callbacks flushes previous cb when new one is passed', () => {
const {rIC} = ReactScheduler;
const callbackA = jest.fn();
const callbackB = jest.fn();
const callbackLog = [];
const callbackA = jest.fn(() => callbackLog.push('A'));
const callbackB = jest.fn(() => callbackLog.push('B'));
rIC(callbackA);
// initially waits to call the callback
expect(callbackA.mock.calls.length).toBe(0);
expect(callbackLog.length).toBe(0);
// when second callback is passed, flushes first one
rIC(callbackB);
expect(callbackA.mock.calls.length).toBe(1);
expect(callbackB.mock.calls.length).toBe(0);
expect(callbackLog.length).toBe(1);
expect(callbackLog[0]).toBe('A');
// after a delay, calls the latest callback passed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should assert that the callbacks were called in the correct order

jest.runAllTimers();
expect(callbackA.mock.calls.length).toBe(1);
expect(callbackB.mock.calls.length).toBe(1);
expect(callbackLog.length).toBe(2);
expect(callbackLog[0]).toBe('A');
expect(callbackLog[1]).toBe('B');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you assert on the entire array, like this...

expect(callbackLog).toEqual(['A', 'B']);

then it guarantees that the callbacks are only called once.

It's also easier to read, IMO. I have a harder time trying to follow what's happening when I have to recreate the list of operations in my head :D

// callbackA should not have timed out and should include a timeRemaining method
expect(callbackA.mock.calls[0][0].didTimeout).toBe(false);
expect(typeof callbackA.mock.calls[0][0].timeRemaining()).toBe('number');
Expand Down