From d8c40fa20b992903bafe13095404e07b40d1a4da Mon Sep 17 00:00:00 2001 From: Oleksandr Sherekin Date: Mon, 5 Oct 2020 21:53:52 +0200 Subject: [PATCH 1/4] test(audit): 5730 --- spec/operators/audit-spec.ts | 77 ++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/spec/operators/audit-spec.ts b/spec/operators/audit-spec.ts index 1e4abede3a..684e028640 100644 --- a/spec/operators/audit-spec.ts +++ b/spec/operators/audit-spec.ts @@ -120,17 +120,17 @@ describe('audit operator', () => { it('should handle a busy producer emitting a regular repeating sequence', () => { testScheduler.run(({ hot, cold, expectObservable, expectSubscriptions }) => { - const e1 = hot(' abcdefabcdefabcdefabcdefa|'); - const e1subs = ' ^------------------------!'; - const e2 = cold(' -----| '); + const e1 = hot(' abcdefabcdefabcdefabcdefa| '); + const e1subs = ' ^------------------------! '; + const e2 = cold(' -----| '); const e2subs = [ - ' ^----! ', - ' ------^----! ', - ' ------------^----! ', - ' ------------------^----! ', - ' ------------------------^!' + ' ^----! ', + ' ------^----! ', + ' ------------^----! ', + ' ------------------^----! ', + ' ------------------------^----!' ]; - const expected = '-----f-----f-----f-----f-|'; + const expected = '-----f-----f-----f-----f-----(a|)'; const result = e1.pipe(audit(() => e2)); @@ -168,13 +168,13 @@ describe('audit operator', () => { }); }); - it('should emit no values if duration is a never', () => { + it('should emit no values and never complete if duration is a never', () => { testScheduler.run(({ hot, cold, expectObservable, expectSubscriptions }) => { const e1 = hot(' ----abcdefabcdefabcdefabcdefa|'); const e1subs = ' ^----------------------------!'; const e2 = cold(' -'); - const e2subs = ' ----^------------------------!'; - const expected = '-----------------------------|'; + const e2subs = ' ----^-------------------------'; + const expected = '------------------------------'; const result = e1.pipe(audit(() => e2)); @@ -232,23 +232,23 @@ describe('audit operator', () => { it('should audit using durations of varying lengths', () => { testScheduler.run(({ hot, cold, expectObservable, expectSubscriptions }) => { - const e1 = hot(' abcdefabcdabcdefghabca|'); - const e1subs = ' ^---------------------!'; + const e1 = hot(' abcdefabcdabcdefghabca| '); + const e1subs = ' ^---------------------! '; const e2 = [ - cold(' -----| '), - cold(' ---| '), - cold(' -------| '), - cold(' --| '), - cold(' ----| ') + cold(' -----| '), + cold(' ---| '), + cold(' -------| '), + cold(' --| '), + cold(' ----| ') ]; const e2subs = [ - ' ^----! ', - ' ------^--! ', - ' ----------^------! ', - ' ------------------^-! ', - ' ---------------------^! ' + ' ^----! ', + ' ------^--! ', + ' ----------^------! ', + ' ------------------^-! ', + ' ---------------------^---! ' ]; - const expected = '-----f---d-------h--c-| '; + const expected = '-----f---d-------h--c----(a|)'; let i = 0; const result = e1.pipe(audit(() => e2[i++])); @@ -391,12 +391,10 @@ describe('audit operator', () => { it('should audit by promise resolves', (done: MochaDone) => { const e1 = interval(10).pipe(take(5)); - const expected = [0, 1, 2, 3]; + const expected = [0, 1, 2, 3, 4, 5]; e1.pipe( - audit(() => { - return new Promise((resolve: any) => { resolve(42); }); - }) + audit(() => Promise.resolve(42)) ).subscribe( (x: number) => { expect(x).to.equal(expected.shift()); }, @@ -404,7 +402,7 @@ describe('audit operator', () => { done(new Error('should not be called')); }, () => { - expect(expected.length).to.equal(0); + expect(expected.length).to.equal(1); done(); } ); @@ -455,4 +453,23 @@ describe('audit operator', () => { expect(sideEffects).to.deep.equal([0, 1, 2]); }); + + it('should emit last value after duration completes if source completes first', () => { + testScheduler.run(({ hot, cold, expectObservable, expectSubscriptions }) => { + const e1 = hot('-a--------xy| '); + const e1subs = ' ^-----------! '; + const e2 = cold(' ----| '); + const e2subs = [ + ' -^---! ', + ' ----------^---!' + ]; + const expected = '-----a--------(y|)'; + + const result = e1.pipe(audit(() => e2)); + + expectObservable(result).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(e1subs); + expectSubscriptions(e2.subscriptions).toBe(e2subs); + }); + }); }); From fae75ed619aceae4f8bc8ec86ee5b5e9aa7c9a14 Mon Sep 17 00:00:00 2001 From: Oleksandr Sherekin Date: Mon, 5 Oct 2020 21:54:15 +0200 Subject: [PATCH 2/4] test(auditTime): 5730 --- spec/operators/auditTime-spec.ts | 38 ++++++++++++-------------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/spec/operators/auditTime-spec.ts b/spec/operators/auditTime-spec.ts index 99eec7f7d0..4697c21eb1 100644 --- a/spec/operators/auditTime-spec.ts +++ b/spec/operators/auditTime-spec.ts @@ -16,7 +16,7 @@ describe('auditTime operator', () => { testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => { const e1 = hot(' -a-x-y----b---x-cx---|'); const subs = ' ^--------------------!'; - const expected = '------y--------x-----|'; + const expected = '------y--------x-----(x|)'; const result = e1.pipe(auditTime(5, testScheduler)); @@ -26,40 +26,30 @@ describe('auditTime operator', () => { }); it('should auditTime events by 5 time units', (done: MochaDone) => { + const expected = 3; of(1, 2, 3).pipe( auditTime(5) ).subscribe((x: number) => { - done(new Error('should not be called')); - }, null, () => { + expect(x).to.equal(expected); done(); }); }); it('should auditTime events multiple times', () => { - const expected = ['1-2', '2-2']; - concat( - timer(0, 10, testScheduler).pipe( - take(3), - map((x: number) => '1-' + x) - ), - timer(80, 10, testScheduler).pipe( - take(5), - map((x: number) => '2-' + x) - ) - ).pipe( - auditTime(50, testScheduler) - ).subscribe((x: string) => { - expect(x).to.equal(expected.shift()); - }); - - testScheduler.flush(); + testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => { + const e1 = hot(' -012-----01234---|'); + const subs = ' ^----------------!'; + const expected = '------2-------4--|'; + expectObservable(e1.pipe(auditTime(5, testScheduler))).toBe(expected); + expectSubscriptions(e1.subscriptions).toBe(subs); + }); }); it('should delay the source if values are not emitted often enough', () => { testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => { const e1 = hot(' -a--------b-----c----|'); const subs = ' ^--------------------!'; - const expected = '------a--------b-----|'; + const expected = '------a--------b-----(c|)'; expectObservable(e1.pipe(auditTime(5, testScheduler))).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(subs); @@ -68,9 +58,9 @@ describe('auditTime operator', () => { it('should handle a busy producer emitting a regular repeating sequence', () => { testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => { - const e1 = hot(' abcdefabcdefabcdefabcdefa|'); - const subs = ' ^------------------------!'; - const expected = '-----f-----f-----f-----f-|'; + const e1 = hot(' abcdefabcdefabcdefabcdefa |'); + const subs = ' ^------------------------ !'; + const expected = '-----f-----f-----f-----f-----(a|)'; expectObservable(e1.pipe(auditTime(5, testScheduler))).toBe(expected); expectSubscriptions(e1.subscriptions).toBe(subs); From 6d66de880906f7bb379f7fed7b2c02d2d4293f76 Mon Sep 17 00:00:00 2001 From: Oleksandr Sherekin Date: Mon, 5 Oct 2020 21:55:23 +0200 Subject: [PATCH 3/4] fix(audit,auditTime): audit and auditTime emit last value after source completes --- src/internal/operators/audit.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/internal/operators/audit.ts b/src/internal/operators/audit.ts index 31ef7a8f78..8ae55d6f40 100644 --- a/src/internal/operators/audit.ts +++ b/src/internal/operators/audit.ts @@ -56,6 +56,7 @@ export function audit(durationSelector: (value: T) => SubscribableOrPromise | null = null; + let isComplete = false; const endDuration = () => { durationSubscriber?.unsubscribe(); @@ -66,18 +67,27 @@ export function audit(durationSelector: (value: T) => SubscribableOrPromise { - hasValue = true; - lastValue = value; - if (!durationSubscriber) { - innerFrom(durationSelector(value)).subscribe( - (durationSubscriber = new OperatorSubscriber(subscriber, endDuration, undefined, endDuration)) - ); + new OperatorSubscriber( + subscriber, + (value) => { + hasValue = true; + lastValue = value; + if (!durationSubscriber) { + innerFrom(durationSelector(value)).subscribe( + (durationSubscriber = new OperatorSubscriber(subscriber, endDuration, undefined, endDuration)) + ); + } + }, + undefined, + () => { + isComplete = true; + (!hasValue || !durationSubscriber || durationSubscriber.closed) && subscriber.complete(); } - }) + ) ); }); } From b5f93683492887f49ee910fee239a1a588b05ec4 Mon Sep 17 00:00:00 2001 From: Oleksandr Sherekin Date: Wed, 7 Oct 2020 17:40:21 +0200 Subject: [PATCH 4/4] fix(review): address review comments --- spec/operators/audit-spec.ts | 4 ++-- spec/operators/auditTime-spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/operators/audit-spec.ts b/spec/operators/audit-spec.ts index 684e028640..34a89687df 100644 --- a/spec/operators/audit-spec.ts +++ b/spec/operators/audit-spec.ts @@ -391,7 +391,7 @@ describe('audit operator', () => { it('should audit by promise resolves', (done: MochaDone) => { const e1 = interval(10).pipe(take(5)); - const expected = [0, 1, 2, 3, 4, 5]; + const expected = [0, 1, 2, 3, 4]; e1.pipe( audit(() => Promise.resolve(42)) @@ -402,7 +402,7 @@ describe('audit operator', () => { done(new Error('should not be called')); }, () => { - expect(expected.length).to.equal(1); + expect(expected.length).to.equal(0); done(); } ); diff --git a/spec/operators/auditTime-spec.ts b/spec/operators/auditTime-spec.ts index 4697c21eb1..d727712d8f 100644 --- a/spec/operators/auditTime-spec.ts +++ b/spec/operators/auditTime-spec.ts @@ -58,8 +58,8 @@ describe('auditTime operator', () => { it('should handle a busy producer emitting a regular repeating sequence', () => { testScheduler.run(({ hot, expectObservable, expectSubscriptions }) => { - const e1 = hot(' abcdefabcdefabcdefabcdefa |'); - const subs = ' ^------------------------ !'; + const e1 = hot(' abcdefabcdefabcdefabcdefa|'); + const subs = ' ^------------------------!'; const expected = '-----f-----f-----f-----f-----(a|)'; expectObservable(e1.pipe(auditTime(5, testScheduler))).toBe(expected);