diff --git a/src/utils.ts b/src/utils.ts index 6ecca32d04c..eb275e862d3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -954,7 +954,7 @@ export function makeInterruptibleAsyncInterval( ): InterruptibleAsyncInterval { let timerId: NodeJS.Timeout | undefined; let lastCallTime: number; - let lastWakeTime: number; + let cannotBeExpedited = false; let stopped = false; options = options ?? {}; @@ -965,10 +965,8 @@ export function makeInterruptibleAsyncInterval( function wake() { const currentTime = clock(); - const timeSinceLastWake = currentTime - lastWakeTime; - const timeSinceLastCall = currentTime - lastCallTime; - const timeUntilNextCall = interval - timeSinceLastCall; - lastWakeTime = currentTime; + const nextScheduledCallTime = lastCallTime + interval; + const timeUntilNextCall = nextScheduledCallTime - currentTime; // For the streaming protocol: there is nothing obviously stopping this // interval from being woken up again while we are waiting "infinitely" @@ -986,7 +984,7 @@ export function makeInterruptibleAsyncInterval( } // debounce multiple calls to wake within the `minInterval` - if (timeSinceLastWake < minInterval) { + if (cannotBeExpedited) { return; } @@ -994,6 +992,7 @@ export function makeInterruptibleAsyncInterval( // faster than the `minInterval` if (timeUntilNextCall > minInterval) { reschedule(minInterval); + cannotBeExpedited = true; } } @@ -1005,7 +1004,7 @@ export function makeInterruptibleAsyncInterval( } lastCallTime = 0; - lastWakeTime = 0; + cannotBeExpedited = false; } function reschedule(ms?: number) { @@ -1018,7 +1017,7 @@ export function makeInterruptibleAsyncInterval( } function executeAndReschedule() { - lastWakeTime = 0; + cannotBeExpedited = false; lastCallTime = clock(); fn(err => { diff --git a/test/unit/utils.test.js b/test/unit/utils.test.js index b35ef5287f6..bee1f80e136 100644 --- a/test/unit/utils.test.js +++ b/test/unit/utils.test.js @@ -41,149 +41,287 @@ describe('utils', function () { }); describe('#makeInterruptibleAsyncInterval', function () { - let clock; + let clock, executor, fnSpy; beforeEach(function () { clock = sinon.useFakeTimers(); + fnSpy = sinon.spy(cb => { + cb(); + }); }); afterEach(function () { + if (executor) { + executor.stop(); + } clock.restore(); }); context('when the immediate option is provided', function () { - const fn = callback => { - callback(); - }; - const fnSpy = sinon.spy(fn); - - it('executes the function immediately', function (done) { - const executor = makeInterruptibleAsyncInterval(fnSpy, { immediate: true, interval: 20 }); - setTimeout(() => { - // The provided function should be called exactly once, since we wait 10ms - // to perform the assertion and the interval is 20ms, so the executor is - // stopped before the scheduled next call. - expect(fnSpy.calledOnce).to.be.true; - executor.stop(); - done(); - }, 10); - clock.tick(10); + it('executes the function immediately and schedules the next execution on the interval', function () { + executor = makeInterruptibleAsyncInterval(fnSpy, { + immediate: true, + minInterval: 10, + interval: 30 + }); + // expect immediate invocation + expect(fnSpy.calledOnce).to.be.true; + // advance clock by less than the scheduled interval to ensure we don't execute early + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + // advance clock to the interval + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; }); }); context('when the immediate option is not provided', function () { - const fn = callback => { - callback(); - }; - const fnSpy = sinon.spy(fn); - - it('executes the function on the provided interval', function (done) { - const executor = makeInterruptibleAsyncInterval(fnSpy, { interval: 10 }); - setTimeout(() => { - // The provided function should be called exactly twice, since we wait 21ms - // to perform the assertion and the interval is 10ms, so the executor is - // stopped before the third call. - expect(fnSpy.calledTwice).to.be.true; - executor.stop(); - done(); - }, 21); - clock.tick(21); + it('executes the function on the provided interval', function () { + executor = makeInterruptibleAsyncInterval(fnSpy, { minInterval: 10, interval: 30 }); + // advance clock by less than the scheduled interval to ensure we don't execute early + clock.tick(29); + expect(fnSpy.callCount).to.equal(0); + // advance clock to the interval + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // advance clock by the interval + clock.tick(30); + expect(fnSpy.calledTwice).to.be.true; }); }); describe('#wake', function () { - context('when the time until next call is negative', function () { - const fn = callback => { - callback(); - }; - const fnSpy = sinon.spy(fn); - - it('calls the function immediately', function (done) { - const executor = makeInterruptibleAsyncInterval(fnSpy, { - interval: 10, + context('when the time until next call is negative', () => { + // somehow we missed the execution, due to an unreliable clock + + it('should execute immediately and schedule the next execution on the interval if this is the first wake', () => { + let fakeClockHasTicked = false; + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30, clock: () => { - // We have our fake clock return a value that will force - // the time until the next call to be a negative value, - // which will in turn force an immediate execution upon - // wake. - return 11; + if (fakeClockHasTicked) { + return 81; + } + fakeClockHasTicked = true; + return 50; } }); - // This will reset the last call time to 0 and ensure the function has - // not been called yet. - executor.stop(); - // Now we call our method under test with the expectation it will force - // an immediate execution. + // tick the environment clock by a smaller amount than the interval + clock.tick(2); + // sanity check to make sure we haven't called execute yet + expect(fnSpy.callCount).to.equal(0); executor.wake(); + // expect immediate execution since expected next call time was 50 + 30 = 80, but the clock shows 81 + expect(fnSpy.calledOnce).to.be.true; + // move forward by more than minInterval but less than full interval to ensure we're scheduling correctly + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + // move forward by the full interval to make sure the scheduled call executes + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; + }); + + it('should execute immediately and schedule the next execution on the interval if this is a repeated wake and the current execution is not rescheduled', () => { + let fakeClockTickCount = 0; + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30, + clock: () => { + if (fakeClockTickCount === 0) { + // on init, return arbitrary starting time + fakeClockTickCount++; + return 50; + } + if (fakeClockTickCount === 1) { + // expected execution time is 80 + // on first wake return a time so less than minInterval is left and no need to reschedule + fakeClockTickCount++; + return 71; + } + return 81; + } + }); - setTimeout(() => { - // The provided function should be called exactly once in this section. - // This is because we immediately stopped the executor, then force woke - // it to get an immediate call with time until the next call being a - // negative value. - expect(fnSpy.calledOnce).to.be.true; - executor.stop(); - done(); - }, 10); + // tick the clock by a small amount before and after the wake to make sure no unexpected async things are happening clock.tick(11); + executor.wake(); + clock.tick(5); + expect(fnSpy.callCount).to.equal(0); + // call our second wake that gets the overdue timer, so expect immediate execution + executor.wake(); + expect(fnSpy.calledOnce).to.be.true; + // move forward by more than minInterval but less than full interval to ensure we're scheduling correctly + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + // move forward by the full interval to make sure the scheduled call executes + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; }); - }); - context('when time since last wake is less than the minimum interval', function () { - const fn = callback => { - callback(); - }; - const fnSpy = sinon.spy(fn); - - it('does not call the function', function (done) { - const executor = makeInterruptibleAsyncInterval(fnSpy, { interval: 10 }); - - // This will reset the last wake time to 0 and ensure the function has - // not been called yet. - executor.stop(); - // Now we call our method under test with the expectation it will not be - // called immediately since our current time is still under the interval - // time. - executor.wake(); + it('should execute immediately and schedule the next execution on the interval if this is a repeated wake even if the current execution is rescheduled', () => { + let fakeClockTickCount = 0; + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30, + clock: () => { + if (fakeClockTickCount === 0) { + // on init, return arbitrary starting time + fakeClockTickCount++; + return 50; + } + if (fakeClockTickCount === 1) { + // expected execution time is 80 + // on first wake return a time so that more than minInterval is left + fakeClockTickCount++; + return 61; + } + return 81; + } + }); - setTimeout(() => { - // The provided function should never be called in this case. - // This is because we immediately stopped the executor, then force woke - // it but the current time is still under the interval time. - expect(fnSpy.callCount).to.equal(0); - executor.stop(); - done(); - }, 9); + // tick the clock by a small amount before and after the wake to make sure no unexpected async things are happening + clock.tick(2); + executor.wake(); clock.tick(9); + expect(fnSpy.callCount).to.equal(0); + // call our second wake that gets the overdue timer, so expect immediate execution + executor.wake(); + expect(fnSpy.calledOnce).to.be.true; + // move forward by more than minInterval but less than full interval to ensure we're scheduling correctly + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + // move forward by the full interval to make sure the scheduled call executes + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; }); }); - context('when time since last call is greater than the minimum interval', function () { - const fn = callback => { - callback(); - }; - const fnSpy = sinon.spy(fn); + context('when the time until next call is less than the minInterval', () => { + // we can't make it go any faster, so we should let the scheduled execution run - it('reschedules the function call for the minimum interval', function (done) { - const executor = makeInterruptibleAsyncInterval(fnSpy, { - interval: 50, - minInterval: 10 + it('should execute on the interval if this is the first wake', () => { + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30 }); + // tick the environment clock so that less than minInterval is left + clock.tick(21); + executor.wake(); + // move forward to just before exepected execution time + clock.tick(8); + expect(fnSpy.callCount).to.equal(0); + // move forward to the full interval to make sure the scheduled call executes + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // check to make sure the next execution runs as expected + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; + }); - // Calling wake here will force the reschedule to happen at the minimum interval - // provided, which is 10ms. + it('should execute on the original interval if this is a repeated wake and the current execution is not rescheduled', () => { + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30 + }); + // tick the environment clock so that less than minInterval is left + clock.tick(21); + executor.wake(); + // tick the environment clock some more so that the next wake is called at a different time + clock.tick(2); executor.wake(); + // tick to just before the expected execution time + clock.tick(6); + expect(fnSpy.callCount).to.equal(0); + // tick up to 20 for the expected execution + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // check to make sure the next execution runs as expected + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; + }); - setTimeout(() => { - // We expect function calls to happen after 10ms, which is the minimum interval, - // and then in 50ms intervals after that. The second call would happen at 60ms - // time from the original call so we've stopped the executor before a third. - expect(fnSpy.calledTwice).to.be.true; - executor.stop(); - done(); - }, 61); - clock.tick(61); + it('should execute on the minInterval from the first wake if this is a repeated wake and the current execution is rescheduled', () => { + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30 + }); + // tick the environment clock so that more than minInterval is left + clock.tick(13); + executor.wake(); + // the first wake should move up the execution to occur at 23 ticks from the start + // we tick 8 to get to 21, so that less than minInterval is left on the original interval expected execution + clock.tick(8); + executor.wake(); + // now we tick to just before the rescheduled execution time + clock.tick(1); + expect(fnSpy.callCount).to.equal(0); + // tick up to 23 for the expected execution + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // check to make sure the next execution runs as expected + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; + }); + }); + + context('when the time until next call is more than the minInterval', () => { + // expedite the execution to minInterval + + it('should execute on the minInterval if this is the first wake', () => { + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30 + }); + // tick the environment clock so that more than minInterval is left + clock.tick(3); + executor.wake(); + // the first wake should move up the execution to occur at 13 ticks from the start + // we tick to just before the rescheduled execution time + clock.tick(9); + expect(fnSpy.callCount).to.equal(0); + // tick up to 13 for the expected execution + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // check to make sure the next execution runs as expected + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; + }); + + it('should execute on the minInterval from the first wake if this is a repeated wake', () => { + // NOTE: under regular circumstances, if the second wake is early enough to warrant a reschedule + // then the first wake must have already warranted a reschedule + executor = makeInterruptibleAsyncInterval(fnSpy, { + minInterval: 10, + interval: 30 + }); + // tick the environment clock so that more than minInterval is left + clock.tick(3); + executor.wake(); + // the first wake should move up the execution to occur at 13 ticks from the start + // we tick a bit more so that more than minInterval is still left and call our repeated wake + clock.tick(2); + executor.wake(); + // tick up to just before the expected execution + clock.tick(7); + expect(fnSpy.callCount).to.equal(0); + // now go up to 13 + clock.tick(1); + expect(fnSpy.calledOnce).to.be.true; + // check to make sure the next execution runs as expected + clock.tick(29); + expect(fnSpy.calledOnce).to.be.true; + clock.tick(1); + expect(fnSpy.calledTwice).to.be.true; }); }); });