fix(spec): fix flush() behavior in handling periodic timers (#881)

vikerman · mhevery · commit eed776cd332b · 2017-08-18T15:15:36.000-07:00
FakeAsyncTest Zone now just finds the last task currently in the timer queue
and tick-s till the endTime of that task.

Solves the issue when there is a short running setInterval in front of a
longer setInterval/setTimeout.
diff --git a/lib/zone-spec/fake-async-test.ts b/lib/zone-spec/fake-async-test.ts
@@ -70,8 +70,13 @@
       }
     }
 
-    tick(millis: number = 0): void {
+    tick(millis: number = 0, doTick?: (elapsed: number) => void): void {
       let finalTime = this._currentTime + millis;
+      let lastCurrentTime = 0;
+      if (this._schedulerQueue.length === 0 && doTick) {
+        doTick(millis);
+        return;
+      }
       while (this._schedulerQueue.length > 0) {
         let current = this._schedulerQueue[0];
         if (finalTime < current.endTime) {
@@ -80,7 +85,11 @@
         } else {
           // Time to run scheduled function. Remove it from the head of queue.
           let current = this._schedulerQueue.shift();
+          lastCurrentTime = this._currentTime;
           this._currentTime = current.endTime;
+          if (doTick) {
+            doTick(this._currentTime - lastCurrentTime);
+          }
           let retval = current.func.apply(global, current.args);
           if (!retval) {
             // Uncaught exception in the current scheduled function. Stop processing the queue.
@@ -91,45 +100,51 @@
       this._currentTime = finalTime;
     }
 
-    flush(limit = 20, flushPeriodic = false, tick?: (elapsed: number) => void): number {
+    flush(limit = 20, flushPeriodic = false, doTick?: (elapsed: number) => void): number {
+      if (flushPeriodic) {
+        return this.flushPeriodic(doTick);
+      } else {
+        return this.flushNonPeriodic(limit, doTick);
+      }
+    }
+
+    private flushPeriodic(doTick?: (elapsed: number) => void): number {
+      if (this._schedulerQueue.length === 0) {
+        return 0;
+      }
+      // Find the last task currently queued in the scheduler queue and tick
+      // till that time.
+      const startTime = this._currentTime;
+      const lastTask = this._schedulerQueue[this._schedulerQueue.length - 1];
+      this.tick(lastTask.endTime - startTime, doTick);
+      return this._currentTime - startTime;
+    }
+
+    private flushNonPeriodic(limit: number, doTick?: (elapsed: number) => void): number {
       const startTime = this._currentTime;
-      let lastCurrentTime = this._currentTime;
+      let lastCurrentTime = 0;
       let count = 0;
-      const seenTimers: number[] = [];
       while (this._schedulerQueue.length > 0) {
         count++;
         if (count > limit) {
           throw new Error(
               'flush failed after reaching the limit of ' + limit +
               ' tasks. Does your code use a polling timeout?');
         }
-        if (!flushPeriodic) {
-          // flush only non-periodic timers.
-          // If the only remaining tasks are periodic(or requestAnimationFrame), finish flushing.
-          if (this._schedulerQueue.filter(task => !task.isPeriodic && !task.isRequestAnimationFrame)
-                  .length === 0) {
-            break;
-          }
-        } else {
-          // flushPeriodic has been requested.
-          // Stop when all timer id-s have been seen at least once.
-          if (this._schedulerQueue
-                  .filter(
-                      task =>
-                          seenTimers.indexOf(task.id) === -1 || this._currentTime === task.endTime)
-                  .length === 0) {
-            break;
-          }
+
+        // flush only non-periodic timers.
+        // If the only remaining tasks are periodic(or requestAnimationFrame), finish flushing.
+        if (this._schedulerQueue.filter(task => !task.isPeriodic && !task.isRequestAnimationFrame)
+                .length === 0) {
+          break;
         }
+
         const current = this._schedulerQueue.shift();
-        if (seenTimers.indexOf(current.id) === -1) {
-          seenTimers.push(current.id);
-        }
         lastCurrentTime = this._currentTime;
         this._currentTime = current.endTime;
-        if (tick) {
-          // Tick any secondary schedulers like Jasmine mock Date.
-          tick(this._currentTime - lastCurrentTime);
+        if (doTick) {
+          // Update any secondary schedulers like Jasmine mock Date.
+          doTick(this._currentTime - lastCurrentTime);
         }
         const retval = current.func.apply(global, current.args);
         if (!retval) {
@@ -253,10 +268,10 @@
       throw error;
     }
 
-    tick(millis: number = 0): void {
+    tick(millis: number = 0, doTick?: (elapsed: number) => void): void {
       FakeAsyncTestZoneSpec.assertInZone();
       this.flushMicrotasks();
-      this._scheduler.tick(millis);
+      this._scheduler.tick(millis, doTick);
       if (this._lastError !== null) {
         this._resetLastErrorAndThrow();
       }
@@ -277,10 +292,10 @@
       flushErrors();
     }
 
-    flush(limit?: number, flushPeriodic?: boolean, tick?: (elapsed: number) => void): number {
+    flush(limit?: number, flushPeriodic?: boolean, doTick?: (elapsed: number) => void): number {
       FakeAsyncTestZoneSpec.assertInZone();
       this.flushMicrotasks();
-      const elapsed = this._scheduler.flush(limit, flushPeriodic, tick);
+      const elapsed = this._scheduler.flush(limit, flushPeriodic, doTick);
       if (this._lastError !== null) {
         this._resetLastErrorAndThrow();
       }
diff --git a/test/zone-spec/fake-async-test.spec.ts b/test/zone-spec/fake-async-test.spec.ts
@@ -557,6 +557,31 @@ describe('FakeAsyncTestZoneSpec', () => {
         expect(y).toEqual(1);
       });
     });
+
+    it('can flush till the last periodic task is processed', () => {
+      fakeAsyncTestZone.run(() => {
+        let x = 0;
+        let y = 0;
+
+        setInterval(() => {
+          x++;
+        }, 10);
+
+        // This shouldn't cause the flush to throw an exception even though
+        // it would require 100 iterations of the shorter timer.
+        setInterval(() => {
+          y++;
+        }, 1000);
+
+        let elapsed = testZoneSpec.flush(20, true);
+
+        // Should stop right after the longer timer has been processed.
+        expect(elapsed).toEqual(1000);
+
+        expect(x).toEqual(100);
+        expect(y).toEqual(1);
+      });
+    });
   });
 
   describe('outside of FakeAsync Zone', () => {
