From 7e3f9a482a475a1366d770e93818575183a7568d Mon Sep 17 00:00:00 2001 From: Benjamin Ingberg Date: Sun, 10 Dec 2017 17:08:12 +0100 Subject: [PATCH] fix(core): fix chained http call (#20924) Fixes an issue where chained http calls would prematurely call testability whenStable callbacks after the first http call. Fixes #20921 PR Close #20924 --- packages/core/src/testability/testability.ts | 21 +++++-- .../core/test/testability/testability_spec.ts | 60 +++++++++---------- 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/packages/core/src/testability/testability.ts b/packages/core/src/testability/testability.ts index f39e0bb823..f007707880 100644 --- a/packages/core/src/testability/testability.ts +++ b/packages/core/src/testability/testability.ts @@ -98,13 +98,22 @@ export class Testability implements PublicTestability { /** @internal */ _runCallbacksIfReady(): void { if (this.isStable()) { - // Schedules the call backs in a new frame so that it is always async. - scheduleMicroTask(() => { - while (this._callbacks.length !== 0) { - (this._callbacks.pop() !)(this._didWork); - } + if (this._callbacks.length !== 0) { + // Schedules the call backs after a macro task run outside of the angular zone to make sure + // no new task are added + this._ngZone.runOutsideAngular(() => { + setTimeout(() => { + if (this.isStable()) { + while (this._callbacks.length !== 0) { + (this._callbacks.pop() !)(this._didWork); + } + this._didWork = false; + } + }); + }); + } else { this._didWork = false; - }); + } } else { // Not Ready this._didWork = true; diff --git a/packages/core/test/testability/testability_spec.ts b/packages/core/test/testability/testability_spec.ts index ee5e7560c5..ec5deb1af9 100644 --- a/packages/core/test/testability/testability_spec.ts +++ b/packages/core/test/testability/testability_spec.ts @@ -16,13 +16,11 @@ import {scheduleMicroTask} from '../../src/util'; -// Schedules a microtasks (using a resolved promise .then()) -function microTask(fn: Function): void { - scheduleMicroTask(() => { - // We do double dispatch so that we can wait for scheduleMicrotask in the Testability when - // NgZone becomes stable. - scheduleMicroTask(fn); - }); +// Schedules a task to be run after Testability checks for oustanding tasks. Since Testability +// uses a 0 second timeout to check for outstanding tasks we add our 0 second timeout after a +// micro task (which ensures Testability's timeout is run first). +function afterTestabilityCheck(fn: Function): void { + scheduleMicroTask(() => setTimeout(fn)); } @Injectable() @@ -65,7 +63,7 @@ class MockNgZone extends NgZone { it('should fire whenstable callbacks if pending count is 0', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -82,11 +80,11 @@ class MockNgZone extends NgZone { testability.increasePendingRequestCount(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); async.done(); }); @@ -98,11 +96,11 @@ class MockNgZone extends NgZone { testability.increasePendingRequestCount(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -120,7 +118,7 @@ class MockNgZone extends NgZone { it('should fire whenstable callbacks with didWork if pending count is 0', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalledWith(false); async.done(); }); @@ -131,14 +129,14 @@ class MockNgZone extends NgZone { testability.increasePendingRequestCount(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalledWith(true); testability.whenStable(execute2); - microTask(() => { + afterTestabilityCheck(() => { expect(execute2).toHaveBeenCalledWith(false); async.done(); }); @@ -154,7 +152,7 @@ class MockNgZone extends NgZone { ngZone.stable(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -173,11 +171,11 @@ class MockNgZone extends NgZone { ngZone.unstable(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); ngZone.stable(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -198,15 +196,15 @@ class MockNgZone extends NgZone { testability.increasePendingRequestCount(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); ngZone.stable(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -221,19 +219,19 @@ class MockNgZone extends NgZone { testability.increasePendingRequestCount(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); ngZone.stable(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).not.toHaveBeenCalled(); testability.decreasePendingRequestCount(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalled(); async.done(); }); @@ -248,11 +246,11 @@ class MockNgZone extends NgZone { ngZone.stable(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalledWith(true); testability.whenStable(execute2); - microTask(() => { + afterTestabilityCheck(() => { expect(execute2).toHaveBeenCalledWith(false); async.done(); }); @@ -264,14 +262,14 @@ class MockNgZone extends NgZone { ngZone.unstable(); testability.whenStable(execute); - microTask(() => { + afterTestabilityCheck(() => { ngZone.stable(); - microTask(() => { + afterTestabilityCheck(() => { expect(execute).toHaveBeenCalledWith(true); testability.whenStable(execute2); - microTask(() => { + afterTestabilityCheck(() => { expect(execute2).toHaveBeenCalledWith(false); async.done(); });