From 8bf103f1994fc86f8043571c7a1921c671c31840 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 29 Jul 2018 07:39:50 +1000 Subject: [PATCH 1/5] test(find): add failing unsubscribe test --- spec/operators/find-spec.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/operators/find-spec.ts b/spec/operators/find-spec.ts index 2bc480b196..d6dc1d85d7 100644 --- a/spec/operators/find-spec.ts +++ b/spec/operators/find-spec.ts @@ -1,10 +1,13 @@ import { expect } from 'chai'; -import { find, mergeMap } from 'rxjs/operators'; +import { find, mergeMap, delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; import { of, Observable, from } from 'rxjs'; declare function asDiagram(arg: string): Function; +declare const rxTestScheduler: TestScheduler; + /** @test {find} */ describe('find operator', () => { function truePredicate(x: any) { @@ -132,6 +135,20 @@ describe('find operator', () => { expectSubscriptions(source.subscriptions).toBe(subs); }); + it('should unsubscribe when the predicate is matched', () => { + const source = hot('--a--b---c-|'); + const subs = '^ !'; + const expected = '-------(b|)'; + + const duration = rxTestScheduler.createTime('--|'); + + expectObservable((source).pipe( + find((value: string) => value === 'b'), + delay(duration, rxTestScheduler) + )).toBe(expected); + expectSubscriptions(source.subscriptions).toBe(subs); + }); + it('should raise if source raise error while element does not match with predicate', () => { const source = hot('--a--b--#'); const subs = '^ !'; From 8377129c3b8d2ee7198907336006593f1adb8eda Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 29 Jul 2018 07:40:09 +1000 Subject: [PATCH 2/5] test(findIndex): add failing unsubscribe test --- spec/operators/findIndex-spec.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/operators/findIndex-spec.ts b/spec/operators/findIndex-spec.ts index 018e402214..6e75619003 100644 --- a/spec/operators/findIndex-spec.ts +++ b/spec/operators/findIndex-spec.ts @@ -1,9 +1,12 @@ -import { findIndex, mergeMap } from 'rxjs/operators'; +import { findIndex, mergeMap, delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; import { of } from 'rxjs'; declare function asDiagram(arg: string): Function; +declare const rxTestScheduler: TestScheduler; + /** @test {findIndex} */ describe('findIndex operator', () => { function truePredicate(x: any) { @@ -125,6 +128,20 @@ describe('findIndex operator', () => { expectSubscriptions(source.subscriptions).toBe(subs); }); + it('should unsubscribe when the predicate is matched', () => { + const source = hot('--a--b---c-|'); + const subs = '^ !'; + const expected = '-------(x|)'; + + const duration = rxTestScheduler.createTime('--|'); + + expectObservable((source).pipe( + findIndex((value: string) => value === 'b'), + delay(duration, rxTestScheduler) + )).toBe(expected, { x: 1 }); + expectSubscriptions(source.subscriptions).toBe(subs); + }); + it('should raise if source raise error while element does not match with predicate', () => { const source = hot('--a--b--#'); const subs = '^ !'; From 9679b5241c621bf33fbed37b3623224b86868d27 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 29 Jul 2018 07:44:49 +1000 Subject: [PATCH 3/5] test(first): add unsubscription test --- spec/operators/first-spec.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/operators/first-spec.ts b/spec/operators/first-spec.ts index 11e58d7226..738fac0a9f 100644 --- a/spec/operators/first-spec.ts +++ b/spec/operators/first-spec.ts @@ -1,10 +1,13 @@ import { expect } from 'chai'; import { hot, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; -import { first, mergeMap } from 'rxjs/operators'; +import { first, mergeMap, delay } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; import { of, from, Observable, Subject, EmptyError } from 'rxjs'; declare function asDiagram(arg: string): Function; +declare const rxTestScheduler: TestScheduler; + /** @test {first} */ describe('Observable.prototype.first', () => { asDiagram('first')('should take the first value of an observable with many values', () => { @@ -101,6 +104,20 @@ describe('Observable.prototype.first', () => { expectSubscriptions(e1.subscriptions).toBe(e1subs); }); + it('should unsubscribe when the first value is receiv', () => { + const source = hot('--a--b---c-|'); + const subs = '^ !'; + const expected = '----(a|)'; + + const duration = rxTestScheduler.createTime('--|'); + + expectObservable((source).pipe( + first(), + delay(duration, rxTestScheduler) + )).toBe(expected); + expectSubscriptions(source.subscriptions).toBe(subs); + }); + it('should return first value that matches a predicate', () => { const e1 = hot('--a-^--b--c--a--c--|'); const expected = '------(c|)'; From 1b42fb032a825c9999fdeb13e2694b2a6096722f Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 29 Jul 2018 07:49:28 +1000 Subject: [PATCH 4/5] fix(find): unsubscribe from source when found --- src/internal/operators/find.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/internal/operators/find.ts b/src/internal/operators/find.ts index cbd2fcab38..005cb4245f 100644 --- a/src/internal/operators/find.ts +++ b/src/internal/operators/find.ts @@ -88,6 +88,7 @@ export class FindValueSubscriber extends Subscriber { destination.next(value); destination.complete(); + this.unsubscribe(); } protected _next(value: T): void { From 7f259cb7854124ffe0172527e8f6848aecea6a1e Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 29 Jul 2018 08:17:14 +1000 Subject: [PATCH 5/5] chore(test): remove any --- spec/operators/find-spec.ts | 26 +++++++++++++------------- spec/operators/findIndex-spec.ts | 26 +++++++++++++------------- spec/operators/first-spec.ts | 2 +- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/spec/operators/find-spec.ts b/spec/operators/find-spec.ts index d6dc1d85d7..8b85c503e1 100644 --- a/spec/operators/find-spec.ts +++ b/spec/operators/find-spec.ts @@ -22,13 +22,13 @@ describe('find operator', () => { const predicate = function (x: number) { return x % 5 === 0; }; - expectObservable((source).pipe(find(predicate))).toBe(expected, values); + expectObservable(source.pipe(find(predicate))).toBe(expected, values); expectSubscriptions(source.subscriptions).toBe(subs); }); it('should throw if not provided a function', () => { expect(() => { - (of('yut', 'yee', 'sam')).pipe(find('yee' as any)); + of('yut', 'yee', 'sam').pipe(find('yee' as any)); }).to.throw(TypeError, 'predicate is not a function'); }); @@ -37,7 +37,7 @@ describe('find operator', () => { const subs = '^'; const expected = '-'; - expectObservable((source).pipe(find(truePredicate))).toBe(expected); + expectObservable(source.pipe(find(truePredicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -46,7 +46,7 @@ describe('find operator', () => { const subs = '(^!)'; const expected = '(x|)'; - const result = (source).pipe(find(truePredicate)); + const result = source.pipe(find(truePredicate)); expectObservable(result).toBe(expected, {x: undefined}); expectSubscriptions(source.subscriptions).toBe(subs); @@ -61,7 +61,7 @@ describe('find operator', () => { return value === 'a'; }; - expectObservable((source).pipe(find(predicate))).toBe(expected); + expectObservable(source.pipe(find(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -74,7 +74,7 @@ describe('find operator', () => { return value === 'b'; }; - expectObservable((source).pipe(find(predicate))).toBe(expected); + expectObservable(source.pipe(find(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -90,7 +90,7 @@ describe('find operator', () => { return value === this.target; }; - expectObservable((source).pipe(find(predicate, finder))).toBe(expected); + expectObservable(source.pipe(find(predicate, finder))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -103,7 +103,7 @@ describe('find operator', () => { return value === 'z'; }; - expectObservable((source).pipe(find(predicate))).toBe(expected, { x: undefined }); + expectObservable(source.pipe(find(predicate))).toBe(expected, { x: undefined }); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -113,7 +113,7 @@ describe('find operator', () => { const expected = '------- '; const unsub = ' ! '; - const result = (source).pipe(find((value: string) => value === 'z')); + const result = source.pipe(find((value: string) => value === 'z')); expectObservable(result, unsub).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); @@ -125,7 +125,7 @@ describe('find operator', () => { const expected = '------- '; const unsub = ' ! '; - const result = (source).pipe( + const result = source.pipe( mergeMap((x: string) => of(x)), find((value: string) => value === 'z'), mergeMap((x: string) => of(x)) @@ -142,7 +142,7 @@ describe('find operator', () => { const duration = rxTestScheduler.createTime('--|'); - expectObservable((source).pipe( + expectObservable(source.pipe( find((value: string) => value === 'b'), delay(duration, rxTestScheduler) )).toBe(expected); @@ -158,7 +158,7 @@ describe('find operator', () => { return value === 'z'; }; - expectObservable((source).pipe(find(predicate))).toBe(expected); + expectObservable(source.pipe(find(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -171,7 +171,7 @@ describe('find operator', () => { throw 'error'; }; - expectObservable((source).pipe(find(predicate))).toBe(expected); + expectObservable(source.pipe(find(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); diff --git a/spec/operators/findIndex-spec.ts b/spec/operators/findIndex-spec.ts index 6e75619003..1dd023bf4c 100644 --- a/spec/operators/findIndex-spec.ts +++ b/spec/operators/findIndex-spec.ts @@ -21,7 +21,7 @@ describe('findIndex operator', () => { const predicate = function (x: number) { return x % 5 === 0; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected, { x: 2 }); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected, { x: 2 }); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -30,7 +30,7 @@ describe('findIndex operator', () => { const subs = '^'; const expected = '-'; - expectObservable((source).pipe(findIndex(truePredicate))).toBe(expected); + expectObservable(source.pipe(findIndex(truePredicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -39,7 +39,7 @@ describe('findIndex operator', () => { const subs = '(^!)'; const expected = '(x|)'; - const result = (source).pipe(findIndex(truePredicate)); + const result = source.pipe(findIndex(truePredicate)); expectObservable(result).toBe(expected, {x: -1}); expectSubscriptions(source.subscriptions).toBe(subs); @@ -55,7 +55,7 @@ describe('findIndex operator', () => { return value === sourceValue; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected, { x: 0 }); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected, { x: 0 }); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -68,7 +68,7 @@ describe('findIndex operator', () => { return value === 7; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected, { x: 1 }); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected, { x: 1 }); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -81,7 +81,7 @@ describe('findIndex operator', () => { const predicate = function (this: typeof sourceValues, value: number) { return value === this.b; }; - const result = (source).pipe(findIndex(predicate, sourceValues)); + const result = source.pipe(findIndex(predicate, sourceValues)); expectObservable(result).toBe(expected, { x: 1 }); expectSubscriptions(source.subscriptions).toBe(subs); @@ -96,7 +96,7 @@ describe('findIndex operator', () => { return value === 'z'; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected, { x: -1 }); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected, { x: -1 }); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -106,7 +106,7 @@ describe('findIndex operator', () => { const expected = '------- '; const unsub = ' ! '; - const result = (source).pipe(findIndex((value: string) => value === 'z')); + const result = source.pipe(findIndex((value: string) => value === 'z')); expectObservable(result, unsub).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); @@ -118,10 +118,10 @@ describe('findIndex operator', () => { const expected = '------- '; const unsub = ' ! '; - const result = (source).pipe( + const result = source.pipe( mergeMap((x: string) => of(x)), findIndex((value: string) => value === 'z'), - mergeMap((x: string) => of(x)) + mergeMap((x: number) => of(x)) ); expectObservable(result, unsub).toBe(expected); @@ -135,7 +135,7 @@ describe('findIndex operator', () => { const duration = rxTestScheduler.createTime('--|'); - expectObservable((source).pipe( + expectObservable(source.pipe( findIndex((value: string) => value === 'b'), delay(duration, rxTestScheduler) )).toBe(expected, { x: 1 }); @@ -151,7 +151,7 @@ describe('findIndex operator', () => { return value === 'z'; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); @@ -164,7 +164,7 @@ describe('findIndex operator', () => { throw 'error'; }; - expectObservable((source).pipe(findIndex(predicate))).toBe(expected); + expectObservable(source.pipe(findIndex(predicate))).toBe(expected); expectSubscriptions(source.subscriptions).toBe(subs); }); }); diff --git a/spec/operators/first-spec.ts b/spec/operators/first-spec.ts index 738fac0a9f..1a64c12b32 100644 --- a/spec/operators/first-spec.ts +++ b/spec/operators/first-spec.ts @@ -111,7 +111,7 @@ describe('Observable.prototype.first', () => { const duration = rxTestScheduler.createTime('--|'); - expectObservable((source).pipe( + expectObservable(source.pipe( first(), delay(duration, rxTestScheduler) )).toBe(expected);