fix(tasks): do not drain the microtask queue early.

mhevery · mhevery · commit ff88bb43d297 · 2016-08-19T10:16:54.000-07:00
Fixes a bug where a event task invoked inside another task
would drain the microtask queue too early. This would mean that microtasks
would be called unexpectedly in the middle of what should have been
a block of synchronous code.
diff --git a/lib/jasmine/jasmine.ts b/lib/jasmine/jasmine.ts
@@ -102,7 +102,16 @@
     execute() {
       if(Zone.current !== ambientZone) throw new Error("Unexpected Zone: " + Zone.current.name);
       testProxyZone = ambientZone.fork(new ProxyZoneSpec());
-      super.execute();
+      if (!Zone.currentTask) {
+        // if we are not running in a task then if someone would register a
+        // element.addEventListener and then calling element.click() the
+        // addEventListener callback would think that it is the top most task and would
+        // drain the microtask queue on element.click() which would be incorrect.
+        // For this reason we always force a task when running jasmine tests.
+        Zone.current.scheduleMicroTask('jasmine.execute().forceTask', () => super.execute());
+      } else {
+        super.execute();
+      }
     }
   };
 })();
diff --git a/test/browser/element.spec.ts b/test/browser/element.spec.ts
@@ -51,16 +51,15 @@ describe('element', function () {
   });
 
   it('should not call microtasks early when an event is invoked', function(done) {
-    // we have to run this test in setTimeout to guarantee that we are running in an existing task
-    setTimeout(() => {
-      var log = '';
+    var log = '';
+    button.addEventListener('click', () => {
       Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
-      button.addEventListener('click', () => log += 'click;');
-      button.click();
-
-      expect(log).toEqual('click;');
-      done();
+      log += 'click;'
     });
+    button.click();
+
+    expect(log).toEqual('click;');
+    done();
   });
 
   it('should call microtasks early when an event is invoked', function(done) {
@@ -80,11 +79,16 @@ describe('element', function () {
      *    eager drainage.
      * 3. Pay the cost of throwing an exception in event tasks and verifying that we are the
      *    top most frame.
+     *
+     * For now we are choosing to ignore it and assume that this arrises in tests only.
+     * As an added measure we make sure that all jasmine tests always run in a task. See: jasmine.ts
      */
     global[Zone['__symbol__']('setTimeout')](() => {
       var log = '';
-      Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
-      button.addEventListener('click', () => log += 'click;');
+      button.addEventListener('click', () => {
+        Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
+        log += 'click;'
+      });
       button.click();
 
       expect(log).toEqual('click;microtask;');
diff --git a/test/jasmine-patch.spec.ts b/test/jasmine-patch.spec.ts
@@ -1,3 +1,8 @@
+beforeEach(() => {
+  // assert that each jasmine run has a task, so that drainMicrotask works properly.
+  expect(Zone.currentTask).toBeTruthy();
+});
+
 describe('jasmine', () => {
   let throwOnAsync = false;
   let beforeEachZone: Zone = null;
