From c22afd3ae3a57537d28fb47265f1c0984d0d8ca6 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 12 Dec 2023 12:27:32 -0500 Subject: [PATCH 1/2] fix(TestScheduler): explicit unsubscribe works properly with toEqual Fixes #7399 --- .../spec/schedulers/TestScheduler-spec.ts | 25 ++++++++ .../src/internal/testing/TestScheduler.ts | 60 ++++++++++--------- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/packages/rxjs/spec/schedulers/TestScheduler-spec.ts b/packages/rxjs/spec/schedulers/TestScheduler-spec.ts index cd709fc78f..fd24603c91 100644 --- a/packages/rxjs/spec/schedulers/TestScheduler-spec.ts +++ b/packages/rxjs/spec/schedulers/TestScheduler-spec.ts @@ -707,5 +707,30 @@ describe('TestScheduler', () => { }); }); }); + + describe('TestScheduler', () => { + let testScheduler: TestScheduler; + let disposed = false; + + beforeEach(() => { + testScheduler = new TestScheduler(assertDeepEquals); + disposed = false; + }); + + it('should unsubscribe toBe', () => { + testScheduler.run(({ expectObservable }) => { + expectObservable(new Observable((_) => () => (disposed = true)), '^!').toBe(''); + }); + expect(disposed).to.be.true; + }); + + // This fails. + it('should unsubscribe toEqual', () => { + testScheduler.run(({ cold, expectObservable }) => { + expectObservable(new Observable((_) => () => (disposed = true)), '^!').toEqual(cold('')); + }); + expect(disposed).to.be.true; + }); + }); }); }); diff --git a/packages/rxjs/src/internal/testing/TestScheduler.ts b/packages/rxjs/src/internal/testing/TestScheduler.ts index b4fd51a37b..338f3eacca 100644 --- a/packages/rxjs/src/internal/testing/TestScheduler.ts +++ b/packages/rxjs/src/internal/testing/TestScheduler.ts @@ -1,4 +1,4 @@ -import type { Subscription} from '../Observable.js'; +import { Subscription } from '../Observable.js'; import { Observable, COMPLETE_NOTIFICATION, errorNotification, nextNotification } from '../Observable.js'; import { ColdObservable } from './ColdObservable.js'; import { HotObservable } from './HotObservable.js'; @@ -135,22 +135,24 @@ export class TestScheduler extends VirtualTimeScheduler { const subscriptionParsed = TestScheduler.parseMarblesAsSubscriptions(subscriptionMarbles, this.runMode); const subscriptionFrame = subscriptionParsed.subscribedFrame === Infinity ? 0 : subscriptionParsed.subscribedFrame; const unsubscriptionFrame = subscriptionParsed.unsubscribedFrame; - let subscription: Subscription; + const subscription = new Subscription(); this.schedule(() => { - subscription = observable.subscribe({ - next: (x) => { - // Support Observable-of-Observables - const value = x instanceof Observable ? this.materializeInnerObservable(x, this.frame) : x; - actual.push({ frame: this.frame, notification: nextNotification(value) }); - }, - error: (error) => { - actual.push({ frame: this.frame, notification: errorNotification(error) }); - }, - complete: () => { - actual.push({ frame: this.frame, notification: COMPLETE_NOTIFICATION }); - }, - }); + subscription.add( + observable.subscribe({ + next: (x) => { + // Support Observable-of-Observables + const value = x instanceof Observable ? this.materializeInnerObservable(x, this.frame) : x; + actual.push({ frame: this.frame, notification: nextNotification(value) }); + }, + error: (error) => { + actual.push({ frame: this.frame, notification: errorNotification(error) }); + }, + complete: () => { + actual.push({ frame: this.frame, notification: COMPLETE_NOTIFICATION }); + }, + }) + ); }, subscriptionFrame); if (unsubscriptionFrame !== Infinity) { @@ -169,19 +171,21 @@ export class TestScheduler extends VirtualTimeScheduler { flushTest.ready = true; flushTest.expected = []; this.schedule(() => { - subscription = other.subscribe({ - next: (x) => { - // Support Observable-of-Observables - const value = x instanceof Observable ? this.materializeInnerObservable(x, this.frame) : x; - flushTest.expected!.push({ frame: this.frame, notification: nextNotification(value) }); - }, - error: (error) => { - flushTest.expected!.push({ frame: this.frame, notification: errorNotification(error) }); - }, - complete: () => { - flushTest.expected!.push({ frame: this.frame, notification: COMPLETE_NOTIFICATION }); - }, - }); + subscription.add( + other.subscribe({ + next: (x) => { + // Support Observable-of-Observables + const value = x instanceof Observable ? this.materializeInnerObservable(x, this.frame) : x; + flushTest.expected!.push({ frame: this.frame, notification: nextNotification(value) }); + }, + error: (error) => { + flushTest.expected!.push({ frame: this.frame, notification: errorNotification(error) }); + }, + complete: () => { + flushTest.expected!.push({ frame: this.frame, notification: COMPLETE_NOTIFICATION }); + }, + }) + ); }, subscriptionFrame); }, }; From 0bbd8c6e317f5a9b4b0804b23a36c02367c59fe7 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 12 Dec 2023 12:33:26 -0500 Subject: [PATCH 2/2] chore: clean up tests, add expected observable test --- .../spec/schedulers/TestScheduler-spec.ts | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/rxjs/spec/schedulers/TestScheduler-spec.ts b/packages/rxjs/spec/schedulers/TestScheduler-spec.ts index fd24603c91..7c01f6f8bb 100644 --- a/packages/rxjs/spec/schedulers/TestScheduler-spec.ts +++ b/packages/rxjs/spec/schedulers/TestScheduler-spec.ts @@ -708,29 +708,31 @@ describe('TestScheduler', () => { }); }); - describe('TestScheduler', () => { - let testScheduler: TestScheduler; + it('should unsubscribe source with toBe', () => { + const testScheduler = new TestScheduler(assertDeepEquals); let disposed = false; - - beforeEach(() => { - testScheduler = new TestScheduler(assertDeepEquals); - disposed = false; + testScheduler.run(({ expectObservable }) => { + expectObservable(new Observable((_) => () => (disposed = true)), '^!').toBe(''); }); + expect(disposed).to.be.true; + }); - it('should unsubscribe toBe', () => { - testScheduler.run(({ expectObservable }) => { - expectObservable(new Observable((_) => () => (disposed = true)), '^!').toBe(''); - }); - expect(disposed).to.be.true; + it('should unsubscribe source with toEqual', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + let disposed = false; + testScheduler.run(({ cold, expectObservable }) => { + expectObservable(new Observable((_) => () => (disposed = true)), '^!').toEqual(cold('')); }); + expect(disposed).to.be.true; + }); - // This fails. - it('should unsubscribe toEqual', () => { - testScheduler.run(({ cold, expectObservable }) => { - expectObservable(new Observable((_) => () => (disposed = true)), '^!').toEqual(cold('')); - }); - expect(disposed).to.be.true; + it('should unsubscribe the expected observable with toEqual', () => { + const testScheduler = new TestScheduler(assertDeepEquals); + let disposed = false; + testScheduler.run(({ cold, expectObservable }) => { + expectObservable(cold(''), '^!').toEqual(new Observable((_) => () => (disposed = true))); }); + expect(disposed).to.be.true; }); }); });