Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Commit

Permalink
fix(tasks): do not drain the microtask queue early.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mhevery committed Aug 19, 2016
1 parent d4a1436 commit ff88bb4
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 11 deletions.
11 changes: 10 additions & 1 deletion lib/jasmine/jasmine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
};
})();
24 changes: 14 additions & 10 deletions test/browser/element.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;');
Expand Down
5 changes: 5 additions & 0 deletions test/jasmine-patch.spec.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down

0 comments on commit ff88bb4

Please sign in to comment.