From c63de0d380a923987aab587720473fad1d205d71 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Tue, 4 Aug 2020 12:21:08 +1000 Subject: [PATCH] feat: support schedulers within run (#5619) * chore: add more providers * refactor: use shorter provider names * chore: implement delegates * test: add failing test with schedulers * refactor: remove delegate from schedule method * fix: one immediate/interval handler per run * refactor: remove AsyncScheduler.delegate * test: use run() for some scheduler tests * docs: remove AsyncScheduler-only bits * chore: add comments --- .../content/guide/testing/marble-testing.md | 6 +- spec/observables/dom/animationFrames-spec.ts | 10 +- spec/operators/concat-spec.ts | 8 +- spec/operators/concatAll-spec.ts | 6 +- .../AnimationFrameScheduler-spec.ts | 77 ++++++--- spec/schedulers/AsapScheduler-spec.ts | 73 ++++++--- spec/schedulers/QueueScheduler-spec.ts | 36 +++-- spec/schedulers/TestScheduler-spec.ts | 152 ++++++++++++++++-- .../observable/dom/animationFrames.ts | 4 +- .../scheduler/AnimationFrameAction.ts | 5 +- src/internal/scheduler/AsapAction.ts | 7 +- src/internal/scheduler/AsyncAction.ts | 5 +- src/internal/scheduler/AsyncScheduler.ts | 22 +-- ...eProvider.ts => animationFrameProvider.ts} | 20 ++- src/internal/scheduler/immediateProvider.ts | 31 ++++ src/internal/scheduler/intervalProvider.ts | 28 ++++ src/internal/testing/TestScheduler.ts | 136 ++++++++++++++-- 17 files changed, 485 insertions(+), 141 deletions(-) rename src/internal/scheduler/{requestAnimationFrameProvider.ts => animationFrameProvider.ts} (55%) create mode 100644 src/internal/scheduler/immediateProvider.ts create mode 100644 src/internal/scheduler/intervalProvider.ts diff --git a/docs_app/content/guide/testing/marble-testing.md b/docs_app/content/guide/testing/marble-testing.md index 83cc6f45c7..34b11c98df 100644 --- a/docs_app/content/guide/testing/marble-testing.md +++ b/docs_app/content/guide/testing/marble-testing.md @@ -6,7 +6,7 @@ We can test our _asynchronous_ RxJS code _synchronously_ and deterministically by virtualizing time using the TestScheduler. ASCII **marble diagrams** provide a visual way for us to represent the behavior of an Observable. We can use them to assert that a particular Observable behaves as expected, as well as to create [hot and cold Observables](https://medium.com/@benlesh/hot-vs-cold-observables-f8094ed53339) we can use as mocks. -> At this time the TestScheduler can only be used to test code that uses timers, like `delay`, `debounceTime`, etc., (i.e. it uses `AsyncScheduler` with delays > 1). If the code consumes a Promise or does scheduling with `AsapScheduler`, `AnimationFrameScheduler`, etc., it cannot be reliably tested with `TestScheduler`, but instead should be tested more traditionally. See the [Known Issues](#known-issues) section for more details. +> At this time, the TestScheduler can only be used to test code that uses RxJS schedulers - `AsyncScheduler`, etc. If the code consumes a Promise, for example, it cannot be reliably tested with `TestScheduler`, but instead should be tested more traditionally. See the [Known Issues](#known-issues) section for more details. ```ts import { TestScheduler } from 'rxjs/testing'; @@ -242,9 +242,9 @@ In the above situation we need the observable stream to complete so that we can ## Known issues -### RxJS code that consumes Promises or uses any of the other schedulers (e.g. AsapScheduler) cannot be directly tested +### RxJS code that consumes Promises cannot be directly tested -If you have RxJS code that uses any other form of asynchronous scheduling other than `AsyncScheduler`, e.g. Promises, `AsapScheduler`, etc. you can't reliably use marble diagrams _for that particular code_. This is because those other scheduling methods won't be virtualized or known to TestScheduler. +If you have RxJS code that uses asynchronous scheduling - e.g. Promises, etc. - you can't reliably use marble diagrams _for that particular code_. This is because those other scheduling methods won't be virtualized or known to TestScheduler. The solution is to test that code in isolation, with the traditional asynchronous testing methods of your testing framework. The specifics depend on your testing framework of choice, but here's a pseudo-code example: diff --git a/spec/observables/dom/animationFrames-spec.ts b/spec/observables/dom/animationFrames-spec.ts index 761510f198..3d040968d8 100644 --- a/spec/observables/dom/animationFrames-spec.ts +++ b/spec/observables/dom/animationFrames-spec.ts @@ -5,7 +5,7 @@ import { animationFrames } from 'rxjs'; import { mergeMapTo, take, takeUntil } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; import { observableMatcher } from '../../helpers/observableMatcher'; -import { requestAnimationFrameProvider } from 'rxjs/internal/scheduler/requestAnimationFrameProvider'; +import { animationFrameProvider } from 'rxjs/internal/scheduler/animationFrameProvider'; describe('animationFrames', () => { let testScheduler: TestScheduler; @@ -59,8 +59,8 @@ describe('animationFrames', () => { it('should compose with take', () => { testScheduler.run(({ animate, cold, expectObservable, time }) => { - const requestSpy = sinon.spy(requestAnimationFrameProvider.delegate!, 'requestAnimationFrame'); - const cancelSpy = sinon.spy(requestAnimationFrameProvider.delegate!, 'cancelAnimationFrame'); + const requestSpy = sinon.spy(animationFrameProvider.delegate!, 'requestAnimationFrame'); + const cancelSpy = sinon.spy(animationFrameProvider.delegate!, 'cancelAnimationFrame'); animate(' ---x---x---x'); const mapped = cold('-m '); @@ -85,8 +85,8 @@ describe('animationFrames', () => { it('should compose with takeUntil', () => { testScheduler.run(({ animate, cold, expectObservable, hot, time }) => { - const requestSpy = sinon.spy(requestAnimationFrameProvider.delegate!, 'requestAnimationFrame'); - const cancelSpy = sinon.spy(requestAnimationFrameProvider.delegate!, 'cancelAnimationFrame'); + const requestSpy = sinon.spy(animationFrameProvider.delegate!, 'requestAnimationFrame'); + const cancelSpy = sinon.spy(animationFrameProvider.delegate!, 'cancelAnimationFrame'); animate(' ---x---x---x'); const mapped = cold('-m '); diff --git a/spec/operators/concat-spec.ts b/spec/operators/concat-spec.ts index 788cfcb8a1..57916a5a85 100644 --- a/spec/operators/concat-spec.ts +++ b/spec/operators/concat-spec.ts @@ -4,8 +4,6 @@ import { concat, mergeMap } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; import { observableMatcher } from '../helpers/observableMatcher'; -declare const rxTestScheduler: TestScheduler; - /** @test {concat} */ describe('concat operator', () => { let testScheduler: TestScheduler; @@ -20,7 +18,7 @@ describe('concat operator', () => { const e2 = cold(' --x---y--|'); const expected = ' --a--b---x---y--|'; - expectObservable(e1.pipe(concat(e2, rxTestScheduler))).toBe(expected); + expectObservable(e1.pipe(concat(e2, testScheduler))).toBe(expected); }); }); @@ -347,7 +345,7 @@ describe('concat operator', () => { const e3subs = ' ----------^-----!'; const expected = ' ---a---b-----c--|'; - expectObservable(e1.pipe(concat(e2, e3, rxTestScheduler))).toBe(expected); + expectObservable(e1.pipe(concat(e2, e3, testScheduler))).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(e1subs); expectSubscriptions(e2.subscriptions).toBe(e2subs); expectSubscriptions(e3.subscriptions).toBe(e3subs); @@ -360,7 +358,7 @@ describe('concat operator', () => { const e1subs = ' ^----!'; const expected = ' ---a-|'; - expectObservable(e1.pipe(concat(rxTestScheduler))).toBe(expected); + expectObservable(e1.pipe(concat(testScheduler))).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(e1subs); }); }); diff --git a/spec/operators/concatAll-spec.ts b/spec/operators/concatAll-spec.ts index f3185d87ba..dd242a94ef 100644 --- a/spec/operators/concatAll-spec.ts +++ b/spec/operators/concatAll-spec.ts @@ -4,8 +4,6 @@ import { concatAll, take, mergeMap } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; import { observableMatcher } from '../helpers/observableMatcher'; -declare const rxTestScheduler: TestScheduler; - /** @test {concatAll} */ describe('concatAll operator', () => { let testScheduler: TestScheduler; @@ -488,7 +486,7 @@ describe('concatAll operator', () => { const e3subs = ' ----------^-----!'; const expected = ' ---a---b-----c--|'; - const result = of(e1, e2, e3, rxTestScheduler).pipe(concatAll()); + const result = of(e1, e2, e3, testScheduler).pipe(concatAll()); expectObservable(result).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(e1subs); @@ -516,7 +514,7 @@ describe('concatAll operator', () => { const e1subs = ' ^----!'; const expected = ' ---a-|'; - const result = of(e1, rxTestScheduler).pipe(concatAll()); + const result = of(e1, testScheduler).pipe(concatAll()); expectObservable(result).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(e1subs); diff --git a/spec/schedulers/AnimationFrameScheduler-spec.ts b/spec/schedulers/AnimationFrameScheduler-spec.ts index ef4985c2c3..57a0d0f88f 100644 --- a/spec/schedulers/AnimationFrameScheduler-spec.ts +++ b/spec/schedulers/AnimationFrameScheduler-spec.ts @@ -1,43 +1,68 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { animationFrameScheduler, Subscription } from 'rxjs'; +import { animationFrameScheduler, Subscription, merge } from 'rxjs'; +import { delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; +import { observableMatcher } from '../helpers/observableMatcher'; +import { animationFrameProvider } from 'rxjs/internal/scheduler/animationFrameProvider'; +import { intervalProvider } from 'rxjs/internal/scheduler/intervalProvider'; const animationFrame = animationFrameScheduler; /** @test {Scheduler} */ describe('Scheduler.animationFrame', () => { + let testScheduler: TestScheduler; + + beforeEach(() => { + testScheduler = new TestScheduler(observableMatcher); + }); + it('should exist', () => { expect(animationFrame).exist; }); it('should act like the async scheduler if delay > 0', () => { - let actionHappened = false; - const sandbox = sinon.createSandbox(); - const fakeTimer = sandbox.useFakeTimers(); - animationFrame.schedule(() => { - actionHappened = true; - }, 50); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.true; - sandbox.restore(); + testScheduler.run(({ animate, cold, expectObservable, time }) => { + animate(' ----------x--'); + const a = cold(' a '); + const ta = time(' ----| '); + const b = cold(' b '); + const tb = time(' --------| '); + const expected = '----a---b----'; + + const result = merge( + a.pipe(delay(ta, animationFrame)), + b.pipe(delay(tb, animationFrame)) + ); + expectObservable(result).toBe(expected); + }); }); - it('should cancel animationFrame actions when unsubscribed', () => { - let actionHappened = false; - const sandbox = sinon.createSandbox(); - const fakeTimer = sandbox.useFakeTimers(); - animationFrame.schedule(() => { - actionHappened = true; - }, 50).unsubscribe(); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - sandbox.restore(); + it('should cancel animationFrame actions when delay > 0', () => { + testScheduler.run(({ animate, cold, expectObservable, flush, time }) => { + const requestSpy = sinon.spy(animationFrameProvider, 'requestAnimationFrame'); + const setSpy = sinon.spy(intervalProvider, 'setInterval'); + const clearSpy = sinon.spy(intervalProvider, 'clearInterval'); + + animate(' ----------x--'); + const a = cold(' a '); + const ta = time(' ----| '); + const subs = ' ^-! '; + const expected = '-------------'; + + const result = merge( + a.pipe(delay(ta, animationFrame)) + ); + expectObservable(result, subs).toBe(expected); + + flush(); + expect(requestSpy).to.have.not.been.called; + expect(setSpy).to.have.been.calledOnce; + expect(clearSpy).to.have.been.calledOnce; + requestSpy.restore(); + setSpy.restore(); + clearSpy.restore(); + }); }); it('should schedule an action to happen later', (done: MochaDone) => { diff --git a/spec/schedulers/AsapScheduler-spec.ts b/spec/schedulers/AsapScheduler-spec.ts index 75cf6ef2c6..22a3ced199 100644 --- a/spec/schedulers/AsapScheduler-spec.ts +++ b/spec/schedulers/AsapScheduler-spec.ts @@ -1,43 +1,66 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { asapScheduler, Subscription, SchedulerAction } from 'rxjs'; +import { asapScheduler, Subscription, SchedulerAction, merge } from 'rxjs'; +import { delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; +import { observableMatcher } from '../helpers/observableMatcher'; +import { immediateProvider } from 'rxjs/internal/scheduler/immediateProvider'; +import { intervalProvider } from 'rxjs/internal/scheduler/intervalProvider'; const asap = asapScheduler; /** @test {Scheduler} */ describe('Scheduler.asap', () => { + let testScheduler: TestScheduler; + + beforeEach(() => { + testScheduler = new TestScheduler(observableMatcher); + }); + it('should exist', () => { expect(asap).exist; }); it('should act like the async scheduler if delay > 0', () => { - let actionHappened = false; - const sandbox = sinon.createSandbox(); - const fakeTimer = sandbox.useFakeTimers(); - asap.schedule(() => { - actionHappened = true; - }, 50); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.true; - sandbox.restore(); + testScheduler.run(({ cold, expectObservable, time }) => { + const a = cold(' a '); + const ta = time(' ----| '); + const b = cold(' b '); + const tb = time(' --------| '); + const expected = '----a---b----'; + + const result = merge( + a.pipe(delay(ta, asap)), + b.pipe(delay(tb, asap)) + ); + expectObservable(result).toBe(expected); + }); }); it('should cancel asap actions when delay > 0', () => { - let actionHappened = false; - const sandbox = sinon.createSandbox(); - const fakeTimer = sandbox.useFakeTimers(); - asap.schedule(() => { - actionHappened = true; - }, 50).unsubscribe(); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - sandbox.restore(); + testScheduler.run(({ cold, expectObservable, flush, time }) => { + const setImmediateSpy = sinon.spy(immediateProvider, 'setImmediate'); + const setSpy = sinon.spy(intervalProvider, 'setInterval'); + const clearSpy = sinon.spy(intervalProvider, 'clearInterval'); + + const a = cold(' a '); + const ta = time(' ----| '); + const subs = ' ^-! '; + const expected = '-------------'; + + const result = merge( + a.pipe(delay(ta, asap)) + ); + expectObservable(result, subs).toBe(expected); + + flush(); + expect(setImmediateSpy).to.have.not.been.called; + expect(setSpy).to.have.been.calledOnce; + expect(clearSpy).to.have.been.calledOnce; + setImmediateSpy.restore(); + setSpy.restore(); + clearSpy.restore(); + }); }); it('should reuse the interval for recursively scheduled actions with the same delay', () => { diff --git a/spec/schedulers/QueueScheduler-spec.ts b/spec/schedulers/QueueScheduler-spec.ts index 2950964b71..8c7f6f9219 100644 --- a/spec/schedulers/QueueScheduler-spec.ts +++ b/spec/schedulers/QueueScheduler-spec.ts @@ -1,24 +1,34 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { queueScheduler, Subscription } from 'rxjs'; +import { queueScheduler, Subscription, merge } from 'rxjs'; +import { delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; +import { observableMatcher } from '../helpers/observableMatcher'; const queue = queueScheduler; /** @test {Scheduler} */ describe('Scheduler.queue', () => { + let testScheduler: TestScheduler; + + beforeEach(() => { + testScheduler = new TestScheduler(observableMatcher); + }); + it('should act like the async scheduler if delay > 0', () => { - let actionHappened = false; - const sandbox = sinon.createSandbox(); - const fakeTimer = sandbox.useFakeTimers(); - queue.schedule(() => { - actionHappened = true; - }, 50); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.false; - fakeTimer.tick(25); - expect(actionHappened).to.be.true; - sandbox.restore(); + testScheduler.run(({ cold, expectObservable, time }) => { + const a = cold(' a '); + const ta = time(' ----| '); + const b = cold(' b '); + const tb = time(' --------| '); + const expected = '----a---b----'; + + const result = merge( + a.pipe(delay(ta, queue)), + b.pipe(delay(tb, queue)) + ); + expectObservable(result).toBe(expected); + }); }); it('should switch from synchronous to asynchronous at will', () => { diff --git a/spec/schedulers/TestScheduler-spec.ts b/spec/schedulers/TestScheduler-spec.ts index cd669b863d..b617fa595c 100644 --- a/spec/schedulers/TestScheduler-spec.ts +++ b/spec/schedulers/TestScheduler-spec.ts @@ -1,11 +1,12 @@ import { expect } from 'chai'; import { hot, cold, expectObservable, expectSubscriptions, time } from '../helpers/marble-testing'; -import { AsyncScheduler } from 'rxjs/internal/scheduler/AsyncScheduler'; import { TestScheduler } from 'rxjs/testing'; -import { Observable, NEVER, EMPTY, Subject, of, merge } from 'rxjs'; -import { delay, debounceTime, concatMap } from 'rxjs/operators'; +import { Observable, NEVER, EMPTY, Subject, of, merge, animationFrameScheduler, asapScheduler, asyncScheduler, interval } from 'rxjs'; +import { delay, debounceTime, concatMap, mergeMap, mapTo, take } from 'rxjs/operators'; import { nextNotification, COMPLETE_NOTIFICATION, errorNotification } from 'rxjs/internal/Notification'; -import { requestAnimationFrameProvider } from 'rxjs/internal/scheduler/requestAnimationFrameProvider'; +import { animationFrameProvider } from 'rxjs/internal/scheduler/animationFrameProvider'; +import { immediateProvider } from 'rxjs/internal/scheduler/immediateProvider'; +import { intervalProvider } from 'rxjs/internal/scheduler/intervalProvider'; declare const rxTestScheduler: TestScheduler; @@ -474,7 +475,6 @@ describe('TestScheduler', () => { const frameTimeFactor = TestScheduler['frameTimeFactor']; const maxFrames = testScheduler.maxFrames; const runMode = testScheduler['runMode']; - const delegate = AsyncScheduler.delegate; try { testScheduler.run(() => { @@ -485,7 +485,6 @@ describe('TestScheduler', () => { expect(TestScheduler['frameTimeFactor']).to.equal(frameTimeFactor); expect(testScheduler.maxFrames).to.equal(maxFrames); expect(testScheduler['runMode']).to.equal(runMode); - expect(AsyncScheduler.delegate).to.equal(delegate); }); it('should flush expectations correctly', () => { @@ -505,7 +504,7 @@ describe('TestScheduler', () => { it('should throw if animate() is not called when needed', () => { const testScheduler = new TestScheduler(assertDeepEquals); expect(() => testScheduler.run(() => { - requestAnimationFrameProvider.schedule(() => { /* pointless lint rule */ }); + animationFrameProvider.schedule(() => { /* pointless lint rule */ }); })).to.throw(); }); @@ -537,7 +536,7 @@ describe('TestScheduler', () => { animate('--x'); const values: string[] = []; - const { schedule } = requestAnimationFrameProvider; + const { schedule } = animationFrameProvider; testScheduler.schedule(() => { schedule(t => values.push(`a@${t}`)); @@ -559,7 +558,7 @@ describe('TestScheduler', () => { animate('--x'); const values: string[] = []; - const { schedule } = requestAnimationFrameProvider; + const { schedule } = animationFrameProvider; testScheduler.schedule(() => { schedule(t => values.push(`a@${t}`)); @@ -578,7 +577,7 @@ describe('TestScheduler', () => { animate('--x'); const values: string[] = []; - const { schedule } = requestAnimationFrameProvider; + const { schedule } = animationFrameProvider; testScheduler.schedule(() => { const subscription = schedule(t => values.push(`a@${t}`)); @@ -592,5 +591,138 @@ describe('TestScheduler', () => { }); }); }); + + describe('immediate and interval', () => { + it('should schedule immediates', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setImmediate } = immediateProvider; + setImmediate(() => { + values.push(`a@${testScheduler.now()}`); + }); + expect(values).to.deep.equal([]); + testScheduler.schedule(() => { + expect(values).to.deep.equal(['a@0']); + }, 10); + }); + }); + + it('should support clearing immediates', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setImmediate, clearImmediate } = immediateProvider; + const handle = setImmediate(() => { + values.push(`a@${testScheduler.now()}`); + }); + expect(values).to.deep.equal([]); + clearImmediate(handle); + testScheduler.schedule(() => { + expect(values).to.deep.equal([]); + }, 10); + }); + }); + + it('should schedule intervals', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setInterval, clearInterval } = intervalProvider; + const handle = setInterval(() => { + values.push(`a@${testScheduler.now()}`); + clearInterval(handle); + }, 1); + expect(values).to.deep.equal([]); + testScheduler.schedule(() => { + expect(values).to.deep.equal(['a@1']); + }, 10); + }); + }); + + it('should reschedule intervals until cleared', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setInterval, clearInterval } = intervalProvider; + const handle = setInterval(() => { + if (testScheduler.now() <= 3) { + values.push(`a@${testScheduler.now()}`); + } else { + clearInterval(handle); + } + }, 1); + expect(values).to.deep.equal([]); + testScheduler.schedule(() => { + expect(values).to.deep.equal(['a@1', 'a@2', 'a@3']); + }, 10); + }); + }); + + it('should schedule immediates before intervals', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(() => { + const values: string[] = []; + const { setImmediate } = immediateProvider; + const { setInterval, clearInterval } = intervalProvider; + const handle = setInterval(() => { + values.push(`a@${testScheduler.now()}`); + clearInterval(handle); + }, 0); + setImmediate(() => { + values.push(`b@${testScheduler.now()}`); + }); + expect(values).to.deep.equal([]); + testScheduler.schedule(() => { + expect(values).to.deep.equal(['b@0', 'a@0']); + }, 10); + }); + }); + }); + + describe('schedulers', () => { + it('should support animationFrame, async and asap schedulers', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(({ animate, cold, expectObservable, time }) => { + animate(' ---------x'); + const mapped = cold('--m-------'); + const tb = time(' -----| '); + const expected = ' --(dc)-b-a'; + const result = mapped.pipe(mergeMap(() => merge( + of('a').pipe(delay(0, animationFrameScheduler)), + of('b').pipe(delay(tb, asyncScheduler)), + of('c').pipe(delay(0, asyncScheduler)), + of('d').pipe(delay(0, asapScheduler)) + ))); + expectObservable(result).toBe(expected); + }); + }); + + it('should emit asap notifications before async notifications', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(({ cold, expectObservable }) => { + const mapped = cold('--ab------'); + const expected = ' ---(ba)---'; + const result = mapped.pipe(mergeMap((value) => value === 'a' + ? of(value).pipe(delay(1, asyncScheduler)) + : of(value).pipe(delay(0, asapScheduler)) + )); + expectObservable(result).toBe(expected); + }); + }); + + it('should support intervals with zero duration', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + testScheduler.run(({ cold, expectObservable }) => { + const mapped = cold('--m-------'); + const expected = ' --(bbbaaa)'; + const result = mapped.pipe(mergeMap(() => merge( + interval(0, asyncScheduler).pipe(mapTo('a'), take(3)), + interval(0, asapScheduler).pipe(mapTo('b'), take(3)) + ))); + expectObservable(result).toBe(expected); + }); + }); + }); }); }); diff --git a/src/internal/observable/dom/animationFrames.ts b/src/internal/observable/dom/animationFrames.ts index 6fcd2217ca..29ad719c7b 100644 --- a/src/internal/observable/dom/animationFrames.ts +++ b/src/internal/observable/dom/animationFrames.ts @@ -2,7 +2,7 @@ import { Observable } from '../../Observable'; import { Subscription } from '../../Subscription'; import { TimestampProvider } from "../../types"; import { performanceTimestampProvider } from '../../scheduler/performanceTimestampProvider'; -import { requestAnimationFrameProvider } from '../../scheduler/requestAnimationFrameProvider'; +import { animationFrameProvider } from '../../scheduler/animationFrameProvider'; /** * An observable of animation frames @@ -85,7 +85,7 @@ export function animationFrames(timestampProvider?: TimestampProvider) { * @param timestampProvider The timestamp provider to use to create the observable */ function animationFramesFactory(timestampProvider?: TimestampProvider) { - const { schedule } = requestAnimationFrameProvider; + const { schedule } = animationFrameProvider; return new Observable<{ timestamp: number, elapsed: number }>(subscriber => { let subscription: Subscription; // If no timestamp provider is specified, use performance.now() - as it diff --git a/src/internal/scheduler/AnimationFrameAction.ts b/src/internal/scheduler/AnimationFrameAction.ts index cf5c7d26c6..ec218f687e 100644 --- a/src/internal/scheduler/AnimationFrameAction.ts +++ b/src/internal/scheduler/AnimationFrameAction.ts @@ -1,6 +1,7 @@ import { AsyncAction } from './AsyncAction'; import { AnimationFrameScheduler } from './AnimationFrameScheduler'; import { SchedulerAction } from '../types'; +import { animationFrameProvider } from './animationFrameProvider'; /** * We need this JSDoc comment for affecting ESDoc. @@ -24,7 +25,7 @@ export class AnimationFrameAction extends AsyncAction { // If an animation frame has already been requested, don't request another // one. If an animation frame hasn't been requested yet, request one. Return // the current animation frame request id. - return scheduler.scheduled || (scheduler.scheduled = requestAnimationFrame( + return scheduler.scheduled || (scheduler.scheduled = animationFrameProvider.requestAnimationFrame( () => scheduler.flush(undefined))); } protected recycleAsyncId(scheduler: AnimationFrameScheduler, id?: any, delay: number = 0): any { @@ -38,7 +39,7 @@ export class AnimationFrameAction extends AsyncAction { // set the scheduled flag to undefined so the next AnimationFrameAction will // request its own. if (scheduler.actions.length === 0) { - cancelAnimationFrame(id); + animationFrameProvider.cancelAnimationFrame(id); scheduler.scheduled = undefined; } // Return undefined so the action knows to request a new async id if it's rescheduled. diff --git a/src/internal/scheduler/AsapAction.ts b/src/internal/scheduler/AsapAction.ts index 7ed07c0d46..c51277edfd 100644 --- a/src/internal/scheduler/AsapAction.ts +++ b/src/internal/scheduler/AsapAction.ts @@ -1,7 +1,8 @@ -import { Immediate } from '../util/Immediate'; import { AsyncAction } from './AsyncAction'; import { AsapScheduler } from './AsapScheduler'; import { SchedulerAction } from '../types'; +import { immediateProvider } from './immediateProvider'; + /** * We need this JSDoc comment for affecting ESDoc. * @ignore @@ -24,7 +25,7 @@ export class AsapAction extends AsyncAction { // If a microtask has already been scheduled, don't schedule another // one. If a microtask hasn't been scheduled yet, schedule one now. Return // the current scheduled microtask id. - return scheduler.scheduled || (scheduler.scheduled = Immediate.setImmediate( + return scheduler.scheduled || (scheduler.scheduled = immediateProvider.setImmediate( scheduler.flush.bind(scheduler, undefined) )); } @@ -39,7 +40,7 @@ export class AsapAction extends AsyncAction { // set the scheduled flag to undefined so the next AsapAction will schedule // its own. if (scheduler.actions.length === 0) { - Immediate.clearImmediate(id); + immediateProvider.clearImmediate(id); scheduler.scheduled = undefined; } // Return undefined so the action knows to request a new async id if it's rescheduled. diff --git a/src/internal/scheduler/AsyncAction.ts b/src/internal/scheduler/AsyncAction.ts index 6389d3ca27..d5403c656f 100644 --- a/src/internal/scheduler/AsyncAction.ts +++ b/src/internal/scheduler/AsyncAction.ts @@ -2,6 +2,7 @@ import { Action } from './Action'; import { SchedulerAction } from '../types'; import { Subscription } from '../Subscription'; import { AsyncScheduler } from './AsyncScheduler'; +import { intervalProvider } from './intervalProvider'; /** * We need this JSDoc comment for affecting ESDoc. @@ -70,7 +71,7 @@ export class AsyncAction extends Action { } protected requestAsyncId(scheduler: AsyncScheduler, id?: any, delay: number = 0): any { - return setInterval(scheduler.flush.bind(scheduler, this), delay); + return intervalProvider.setInterval(scheduler.flush.bind(scheduler, this), delay); } protected recycleAsyncId(scheduler: AsyncScheduler, id: any, delay: number | null = 0): any { @@ -80,7 +81,7 @@ export class AsyncAction extends Action { } // Otherwise, if the action's delay time is different from the current delay, // or the action has been rescheduled before it's executed, clear the interval id - clearInterval(id); + intervalProvider.clearInterval(id); return undefined; } diff --git a/src/internal/scheduler/AsyncScheduler.ts b/src/internal/scheduler/AsyncScheduler.ts index 713fa815ab..27912644a4 100644 --- a/src/internal/scheduler/AsyncScheduler.ts +++ b/src/internal/scheduler/AsyncScheduler.ts @@ -1,11 +1,8 @@ import { Scheduler } from '../Scheduler'; import { Action } from './Action'; import { AsyncAction } from './AsyncAction'; -import { SchedulerAction } from '../types'; -import { Subscription } from '../Subscription'; export class AsyncScheduler extends Scheduler { - public static delegate?: Scheduler; public actions: Array> = []; /** * A flag to indicate whether the Scheduler is currently executing a batch of @@ -23,23 +20,8 @@ export class AsyncScheduler extends Scheduler { */ public scheduled: any = undefined; - constructor(SchedulerAction: typeof Action, - now: () => number = Scheduler.now) { - super(SchedulerAction, () => { - if (AsyncScheduler.delegate && AsyncScheduler.delegate !== this) { - return AsyncScheduler.delegate.now(); - } else { - return now(); - } - }); - } - - public schedule(work: (this: SchedulerAction, state?: T) => void, delay: number = 0, state?: T): Subscription { - if (AsyncScheduler.delegate && AsyncScheduler.delegate !== this) { - return AsyncScheduler.delegate.schedule(work, delay, state); - } else { - return super.schedule(work, delay, state); - } + constructor(SchedulerAction: typeof Action, now: () => number = Scheduler.now) { + super(SchedulerAction, now); } public flush(action: AsyncAction): void { diff --git a/src/internal/scheduler/requestAnimationFrameProvider.ts b/src/internal/scheduler/animationFrameProvider.ts similarity index 55% rename from src/internal/scheduler/requestAnimationFrameProvider.ts rename to src/internal/scheduler/animationFrameProvider.ts index 37fecdaa4a..107dc68098 100644 --- a/src/internal/scheduler/requestAnimationFrameProvider.ts +++ b/src/internal/scheduler/animationFrameProvider.ts @@ -1,8 +1,10 @@ /** @prettier */ import { Subscription } from '../Subscription'; -type RequestAnimationFrameProvider = { +type AnimationFrameProvider = { schedule(callback: FrameRequestCallback): Subscription; + requestAnimationFrame: typeof requestAnimationFrame; + cancelAnimationFrame: typeof cancelAnimationFrame; delegate: | { requestAnimationFrame: typeof requestAnimationFrame; @@ -11,13 +13,13 @@ type RequestAnimationFrameProvider = { | undefined; }; -export const requestAnimationFrameProvider: RequestAnimationFrameProvider = { +export const animationFrameProvider: AnimationFrameProvider = { + // When accessing the delegate, use the variable rather than `this` so that + // the functions can be called without being bound to the provider. schedule(callback) { let request = requestAnimationFrame; let cancel: typeof cancelAnimationFrame | undefined = cancelAnimationFrame; - // Use the variable rather than `this` so that the function can be called - // without being bound to the provider. - const { delegate } = requestAnimationFrameProvider; + const { delegate } = animationFrameProvider; if (delegate) { request = delegate.requestAnimationFrame; cancel = delegate.cancelAnimationFrame; @@ -31,5 +33,13 @@ export const requestAnimationFrameProvider: RequestAnimationFrameProvider = { }); return new Subscription(() => cancel?.(handle)); }, + requestAnimationFrame(...args) { + const { delegate } = animationFrameProvider; + return (delegate?.requestAnimationFrame || requestAnimationFrame)(...args); + }, + cancelAnimationFrame(...args) { + const { delegate } = animationFrameProvider; + return (delegate?.cancelAnimationFrame || cancelAnimationFrame)(...args); + }, delegate: undefined, }; diff --git a/src/internal/scheduler/immediateProvider.ts b/src/internal/scheduler/immediateProvider.ts new file mode 100644 index 0000000000..7344f31988 --- /dev/null +++ b/src/internal/scheduler/immediateProvider.ts @@ -0,0 +1,31 @@ +/** @prettier */ +import { Immediate } from '../util/Immediate'; +const { setImmediate, clearImmediate } = Immediate; + +type SetImmediateFunction = (handler: () => void, ...args: any[]) => number; +type ClearImmediateFunction = (handle: number) => void; + +type ImmediateProvider = { + setImmediate: SetImmediateFunction; + clearImmediate: ClearImmediateFunction; + delegate: + | { + setImmediate: SetImmediateFunction; + clearImmediate: ClearImmediateFunction; + } + | undefined; +}; + +export const immediateProvider: ImmediateProvider = { + // When accessing the delegate, use the variable rather than `this` so that + // the functions can be called without being bound to the provider. + setImmediate(...args) { + const { delegate } = immediateProvider; + return (delegate?.setImmediate || setImmediate)(...args); + }, + clearImmediate(handle) { + const { delegate } = immediateProvider; + return (delegate?.clearImmediate || clearImmediate)(handle); + }, + delegate: undefined, +}; diff --git a/src/internal/scheduler/intervalProvider.ts b/src/internal/scheduler/intervalProvider.ts new file mode 100644 index 0000000000..06d8ece900 --- /dev/null +++ b/src/internal/scheduler/intervalProvider.ts @@ -0,0 +1,28 @@ +/** @prettier */ +type SetIntervalFunction = (handler: () => void, timeout?: number, ...args: any[]) => number; +type ClearIntervalFunction = (handle: number) => void; + +type IntervalProvider = { + setInterval: SetIntervalFunction; + clearInterval: ClearIntervalFunction; + delegate: + | { + setInterval: SetIntervalFunction; + clearInterval: ClearIntervalFunction; + } + | undefined; +}; + +export const intervalProvider: IntervalProvider = { + // When accessing the delegate, use the variable rather than `this` so that + // the functions can be called without being bound to the provider. + setInterval(...args) { + const { delegate } = intervalProvider; + return (delegate?.setInterval || setInterval)(...args); + }, + clearInterval(handle) { + const { delegate } = intervalProvider; + return (delegate?.clearInterval || clearInterval)(handle); + }, + delegate: undefined, +}; diff --git a/src/internal/testing/TestScheduler.ts b/src/internal/testing/TestScheduler.ts index 89f2bfe5d4..e6344d9026 100644 --- a/src/internal/testing/TestScheduler.ts +++ b/src/internal/testing/TestScheduler.ts @@ -10,7 +10,9 @@ import { ObservableNotification } from '../types'; import { COMPLETE_NOTIFICATION, errorNotification, nextNotification } from '../Notification'; import { dateTimestampProvider } from '../scheduler/dateTimestampProvider'; import { performanceTimestampProvider } from '../scheduler/performanceTimestampProvider'; -import { requestAnimationFrameProvider } from '../scheduler/requestAnimationFrameProvider'; +import { animationFrameProvider } from '../scheduler/animationFrameProvider'; +import { immediateProvider } from '../scheduler/immediateProvider'; +import { intervalProvider } from '../scheduler/intervalProvider'; const defaultMaxFrame: number = 750; @@ -419,34 +421,34 @@ export class TestScheduler extends VirtualTimeScheduler { // gives the author full control over when - or if - animation frames are // 'painted'. - let animationFramesHandle = 0; - let animationFramesQueue: Map | undefined; + let lastHandle = 0; + let map: Map | undefined; const delegate = { requestAnimationFrame(callback: FrameRequestCallback) { - if (!animationFramesQueue) { + if (!map) { throw new Error("animate() was not called within run()"); } - const handle = ++animationFramesHandle; - animationFramesQueue.set(handle, callback); + const handle = ++lastHandle; + map.set(handle, callback); return handle; }, cancelAnimationFrame(handle: number) { - if (!animationFramesQueue) { + if (!map) { throw new Error("animate() was not called within run()"); } - animationFramesQueue.delete(handle); + map.delete(handle); } }; const animate = (marbles: string) => { - if (animationFramesQueue) { + if (map) { throw new Error('animate() must not be called more than once within run()'); } if (/[|#]/.test(marbles)) { throw new Error('animate() must not complete or error') } - animationFramesQueue = new Map(); + map = new Map(); const messages = TestScheduler.parseMarbles(marbles, undefined, undefined, undefined, true); for (const message of messages) { this.schedule(() => { @@ -456,8 +458,8 @@ export class TestScheduler extends VirtualTimeScheduler { // reschedule themselves. (And, yeah, we're using a Map to represent // the queue, but the values are guaranteed to be returned in // insertion order, so it's all good. Trust me, I've read the docs.) - const callbacks = Array.from(animationFramesQueue!.values()); - animationFramesQueue!.clear(); + const callbacks = Array.from(map!.values()); + map!.clear(); for (const callback of callbacks) { callback(now); } @@ -468,6 +470,104 @@ export class TestScheduler extends VirtualTimeScheduler { return { animate, delegate }; } + private createDelegates() { + // When in run mode, the TestScheduler provides alternate implementations + // of set/clearImmediate and set/clearInterval. These implementations are + // consumed by the scheduler implementations via the providers. This is + // done to effect deterministic asap and async scheduler behavior so that + // all of the schedulers are testable in 'run mode'. Prior to v7, + // delegation occurred at the scheduler level. That is, the asap and + // animation frame schedulers were identical in behavior to the async + // scheduler. Now, when in run mode, asap actions are prioritized over + // async actions and animation frame actions are coordinated using the + // animate run helper. + + let lastHandle = 0; + let map = new Map void; + subscription: Subscription; + type: 'immediate' | 'interval'; + }>(); + + const run = () => { + // Whenever a scheduled run is executed, it must run a single immediate + // or interval action - with immediate actions being prioritized over + // interval actions. + const now = this.now(); + const values = Array.from(map.values()); + const due = values.filter(({ due }) => due <= now); + const immediates = due.filter(({ type }) => type === 'immediate'); + if (immediates.length > 0) { + const { handle, handler } = immediates[0]; + map.delete(handle); + handler(); + return; + } + const intervals = due.filter(({ type }) => type === 'interval'); + if (intervals.length > 0) { + const interval = intervals[0]; + const { duration, handler } = interval; + interval.due = now + duration; + // The interval delegate must behave like setInterval, so run needs to + // be rescheduled. This will continue until the clearInterval delegate + // unsubscribes and deletes the handle from the map. + interval.subscription = this.schedule(run, duration); + handler(); + return; + } + throw new Error('Expected a due immediate or interval'); + }; + + const immediate = { + setImmediate: (handler: () => void) => { + const handle = ++lastHandle; + map.set(handle, { + due: this.now(), + duration: 0, + handle, + handler, + subscription: this.schedule(run, 0), + type: 'immediate', + }); + return handle; + }, + clearImmediate: (handle: number) => { + const value = map.get(handle); + if (value) { + value.subscription.unsubscribe(); + map.delete(handle); + } + } + }; + + const interval = { + setInterval: (handler: () => void, duration = 0) => { + const handle = ++lastHandle; + map.set(handle, { + due: this.now() + duration, + duration, + handle, + handler, + subscription: this.schedule(run, duration), + type: 'interval', + }); + return handle; + }, + clearInterval: (handle: number) => { + const value = map.get(handle); + if (value) { + value.subscription.unsubscribe(); + map.delete(handle); + } + } + }; + + return { immediate, interval }; + } + /** * The `run` method performs the test in 'run mode' - in which schedulers * used within the test automatically delegate to the `TestScheduler`. That @@ -485,10 +585,13 @@ export class TestScheduler extends VirtualTimeScheduler { this.runMode = true; const animator = this.createAnimator(); - requestAnimationFrameProvider.delegate = animator.delegate; + const delegates = this.createDelegates(); + + animationFrameProvider.delegate = animator.delegate; dateTimestampProvider.delegate = this; + immediateProvider.delegate = delegates.immediate; + intervalProvider.delegate = delegates.interval; performanceTimestampProvider.delegate = this; - AsyncScheduler.delegate = this; const helpers: RunHelpers = { cold: this.createColdObservable.bind(this), @@ -507,10 +610,11 @@ export class TestScheduler extends VirtualTimeScheduler { TestScheduler.frameTimeFactor = prevFrameTimeFactor; this.maxFrames = prevMaxFrames; this.runMode = false; - requestAnimationFrameProvider.delegate = undefined; + animationFrameProvider.delegate = undefined; dateTimestampProvider.delegate = undefined; + immediateProvider.delegate = undefined; + intervalProvider.delegate = undefined; performanceTimestampProvider.delegate = undefined; - AsyncScheduler.delegate = undefined; } } }