From 3a054be7837e4c1cd12bad76f69904e303a82c47 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 16 Aug 2016 17:02:03 -0700 Subject: [PATCH] feat(jasmine): patch jasmine to understand zones. jasmine now understands zones and follows these rules: - Jasmine itself runs in ambient zone (most likely the root Zone). - Describe calls run in SyncZone which prevent async operations from being spawned from within the describe blocks. - beforeEach/it/afterEach run in ProxyZone, which allows tests to retroactively set zone rules. - Each test runs in a new instance of the ProxyZone. --- lib/jasmine/jasmine.ts | 130 +++++++++++++++++++++------ test/browser/XMLHttpRequest.spec.ts | 2 +- test/common/setInterval.spec.ts | 14 +-- test/common/setTimeout.spec.ts | 14 +-- test/common/zone.spec.ts | 7 +- test/jasmine-patch.spec.ts | 29 ++++++ test/zone-spec/proxy.spec.ts | 20 +++-- test/zone-spec/task-tracking.spec.ts | 11 +-- 8 files changed, 168 insertions(+), 59 deletions(-) create mode 100644 test/jasmine-patch.spec.ts diff --git a/lib/jasmine/jasmine.ts b/lib/jasmine/jasmine.ts index 8c2cdf39f..890fa3c1f 100644 --- a/lib/jasmine/jasmine.ts +++ b/lib/jasmine/jasmine.ts @@ -1,32 +1,108 @@ 'use strict'; -// Patch jasmine's it and fit functions so that the `done` wrapCallback always resets the zone -// to the jasmine zone, which should be the root zone. (angular/zone.js#91) -if (!Zone) { - throw new Error('zone.js does not seem to be installed'); -} -// When you have in async test (test with `done` argument) jasmine will -// execute the next test synchronously in the done handler. This makes sense -// for most tests, but now with zones. With zones running next test -// synchronously means that the current zone does not get cleared. This -// results in a chain of nested zones, which makes it hard to reason about -// it. We override the `clearStack` method which forces jasmine to always -// drain the stack before next test gets executed. -(jasmine).QueueRunner = (function (SuperQueueRunner) { - const originalZone = Zone.current; - // Subclass the `QueueRunner` and override the `clearStack` method. - - function alwaysClearStack(fn) { - const zone: Zone = Zone.current.getZoneWith('JasmineClearStackZone') - || Zone.current.getZoneWith('ProxyZoneSpec') - || originalZone; - zone.scheduleMicroTask('jasmineCleanStack', fn); +(() => { + // Patch jasmine's describe/it/beforeEach/afterEach functions so test code always runs + // in a testZone (ProxyZone). (See: angular/zone.js#91 & angular/angular#10503) + if (!Zone) throw new Error("Missing: zone.js"); + if (!jasmine) throw new Error("Missing: jasmine.js"); + if (jasmine['__zone_patch__']) throw new Error("'jasmine' has already been patched with 'Zone'."); + jasmine['__zone_patch__'] = true; + + const SyncTestZoneSpec: {new (name: string): ZoneSpec} = Zone['SyncTestZoneSpec']; + const ProxyZoneSpec: {new (): ZoneSpec} = Zone['ProxyZoneSpec']; + if (!SyncTestZoneSpec) throw new Error("Missing: SyncTestZoneSpec"); + if (!ProxyZoneSpec) throw new Error("Missing: ProxyZoneSpec"); + + const ambientZone = Zone.current; + // Create a synchronous-only zone in which to run `describe` blocks in order to raise an + // error if any asynchronous operations are attempted inside of a `describe` but outside of + // a `beforeEach` or `it`. + const syncZone = ambientZone.fork(new SyncTestZoneSpec('jasmine.describe')); + + // This is the zone which will be used for running individual tests. + // It will be a proxy zone, so that the tests function can retroactively install + // different zones. + // Example: + // - In beforeEach() do childZone = Zone.current.fork(...); + // - In it() try to do fakeAsync(). The issue is that because the beforeEach forked the + // zone outside of fakeAsync it will be able to escope the fakeAsync rules. + // - Because ProxyZone is parent fo `childZone` fakeAsync can retroactively add + // fakeAsync behavior to the childZone. + let testProxyZone: Zone = null; + + // Monkey patch all of the jasmine DSL so that each function runs in appropriate zone. + const jasmineEnv = jasmine.getEnv(); + ['desribe', 'xdescribe', 'fdescribe'].forEach((methodName) => { + let originalJasmineFn: Function = jasmineEnv[methodName]; + jasmineEnv[methodName] = function(description: string, specDefinitions: Function) { + return originalJasmineFn.call(this, description, wrapDescribeInZone(specDefinitions)); + } + }); + ['it', 'xit', 'fit'].forEach((methodName) => { + let originalJasmineFn: Function = jasmineEnv[methodName]; + jasmineEnv[methodName] = function(description: string, specDefinitions: Function) { + return originalJasmineFn.call(this, description, wrapTestInZone(specDefinitions)); + } + }); + ['beforeEach', 'afterEach'].forEach((methodName) => { + let originalJasmineFn: Function = jasmineEnv[methodName]; + jasmineEnv[methodName] = function(specDefinitions: Function) { + return originalJasmineFn.call(this, wrapTestInZone(specDefinitions)); + } + }); + + /** + * Gets a function wrapping the body of a Jasmine `describe` block to execute in a + * synchronous-only zone. + */ + function wrapDescribeInZone(describeBody: Function): Function { + return function() { + return syncZone.run(describeBody, this, arguments as any as any[]); + } } - function QueueRunner(options) { - options.clearStack = alwaysClearStack; - SuperQueueRunner.call(this, options); + /** + * Gets a function wrapping the body of a Jasmine `it/beforeEach/afterEach` block to + * execute in a ProxyZone zone. + * This will run in `testProxyZone`. The `testProxyZone` will be reset by the `ZoneQueueRunner` + */ + function wrapTestInZone(testBody: Function): Function { + // The `done` callback is only passed through if the function expects at least one argument. + // Note we have to make a function with correct number of arguments, otherwise jasmine will + // think that all functions are sync or async. + return (testBody.length == 0) + ? function() { return testProxyZone.run(testBody, this); } + : function(done) { return testProxyZone.run(testBody, this, [done]); }; + } + interface QueueRunner { + execute(): void; } - QueueRunner.prototype = SuperQueueRunner.prototype; - return QueueRunner; -})((jasmine).QueueRunner); + interface QueueRunnerAttrs { + queueableFns: {fn: Function}[]; + onComplete: () => void; + clearStack: (fn) => void; + onException: (error) => void; + catchException: () => boolean; + userContext: any; + timeout: {setTimeout: Function, clearTimeout: Function}; + fail: ()=> void; + } + + const QueueRunner = (jasmine as any).QueueRunner as { new(attrs: QueueRunnerAttrs): QueueRunner }; + (jasmine as any).QueueRunner = class ZoneQueueRunner extends QueueRunner { + constructor(attrs: QueueRunnerAttrs) { + attrs.clearStack = (fn) => fn(); // Don't clear since onComplete will clear. + attrs.onComplete = ((fn) => () => { + // All functions are done, clear the test zone. + testProxyZone = null; + ambientZone.scheduleMicroTask('jasmine.onComplete', fn); + })(attrs.onComplete); + super(attrs); + } + execute() { + if(Zone.current !== ambientZone) throw new Error("Unexpected Zone: " + Zone.current.name); + testProxyZone = ambientZone.fork(new ProxyZoneSpec()); + super.execute(); + } + }; +})(); diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 11d9c8271..463ca17a0 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -15,7 +15,7 @@ describe('XMLHttpRequest', function () { expect(wtfMock.log[wtfMock.log.length - 5]).toMatch( /\> Zone\:invokeTask.*addEventListener\:readystatechange/); expect(wtfMock.log[wtfMock.log.length - 4]).toEqual( - '> Zone:invokeTask:XMLHttpRequest.send("::WTF::TestZone")'); + '> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); expect(wtfMock.log[wtfMock.log.length - 3]).toEqual( '< Zone:invokeTask:XMLHttpRequest.send'); expect(wtfMock.log[wtfMock.log.length - 2]).toMatch( diff --git a/test/common/setInterval.spec.ts b/test/common/setInterval.spec.ts index e520877d1..2c50774c3 100644 --- a/test/common/setInterval.spec.ts +++ b/test/common/setInterval.spec.ts @@ -12,11 +12,11 @@ describe('setInterval', function () { expect(Zone.current.name).toEqual(('TestZone')); global[zoneSymbol('setTimeout')](function() { expect(wtfMock.log).toEqual([ - '# Zone:fork("::WTF", "TestZone")', - '> Zone:invoke:unit-test("::WTF::TestZone")', - '# Zone:schedule:macroTask:setInterval("::WTF::TestZone", ' + id + ')', + '# Zone:fork("::ProxyZone::WTF", "TestZone")', + '> Zone:invoke:unit-test("::ProxyZone::WTF::TestZone")', + '# Zone:schedule:macroTask:setInterval("::ProxyZone::WTF::TestZone", ' + id + ')', '< Zone:invoke:unit-test', - '> Zone:invokeTask:setInterval("::WTF::TestZone")', + '> Zone:invokeTask:setInterval("::ProxyZone::WTF::TestZone")', '< Zone:invokeTask:setInterval' ]); clearInterval(cancelId); @@ -37,9 +37,9 @@ describe('setInterval', function () { return value; }); expect(wtfMock.log).toEqual([ - '# Zone:fork("::WTF", "TestZone")', - '> Zone:invoke:unit-test("::WTF::TestZone")', - '# Zone:schedule:macroTask:setInterval("::WTF::TestZone", ' + id + ')' + '# Zone:fork("::ProxyZone::WTF", "TestZone")', + '> Zone:invoke:unit-test("::ProxyZone::WTF::TestZone")', + '# Zone:schedule:macroTask:setInterval("::ProxyZone::WTF::TestZone", ' + id + ')' ]); }, null, null, 'unit-test'); }); diff --git a/test/common/setTimeout.spec.ts b/test/common/setTimeout.spec.ts index 0623f603d..827406fa6 100644 --- a/test/common/setTimeout.spec.ts +++ b/test/common/setTimeout.spec.ts @@ -10,11 +10,11 @@ describe('setTimeout', function () { expect(Zone.current.name).toEqual(('TestZone')); global[zoneSymbol('setTimeout')](function () { expect(wtfMock.log).toEqual([ - '# Zone:fork("::WTF", "TestZone")', - '> Zone:invoke:unit-test("::WTF::TestZone")', - '# Zone:schedule:macroTask:setTimeout("::WTF::TestZone", ' + id + ')', + '# Zone:fork("::ProxyZone::WTF", "TestZone")', + '> Zone:invoke:unit-test("::ProxyZone::WTF::TestZone")', + '# Zone:schedule:macroTask:setTimeout("::ProxyZone::WTF::TestZone", ' + id + ')', '< Zone:invoke:unit-test', - '> Zone:invokeTask:setTimeout("::WTF::TestZone")', + '> Zone:invokeTask:setTimeout("::ProxyZone::WTF::TestZone")', '< Zone:invokeTask:setTimeout' ]); done(); @@ -33,9 +33,9 @@ describe('setTimeout', function () { return value; }); expect(wtfMock.log).toEqual([ - '# Zone:fork("::WTF", "TestZone")', - '> Zone:invoke:unit-test("::WTF::TestZone")', - '# Zone:schedule:macroTask:setTimeout("::WTF::TestZone", ' + id + ')' + '# Zone:fork("::ProxyZone::WTF", "TestZone")', + '> Zone:invoke:unit-test("::ProxyZone::WTF::TestZone")', + '# Zone:schedule:macroTask:setTimeout("::ProxyZone::WTF::TestZone", ' + id + ')' ]); }, null, null, 'unit-test'); }); diff --git a/test/common/zone.spec.ts b/test/common/zone.spec.ts index e8107da59..e453a0b0b 100644 --- a/test/common/zone.spec.ts +++ b/test/common/zone.spec.ts @@ -34,8 +34,9 @@ describe('Zone', function () { }); it('should allow zones to be run from within another zone', function () { - var zoneA = Zone.current.fork({ name: 'A' }); - var zoneB = Zone.current.fork({ name: 'B' }); + var zone = Zone.current; + var zoneA = zone.fork({ name: 'A' }); + var zoneB = zone.fork({ name: 'B' }); zoneA.run(function () { zoneB.run(function () { @@ -43,7 +44,7 @@ describe('Zone', function () { }); expect(Zone.current).toBe(zoneA); }); - expect(Zone.current).toBe(rootZone); + expect(Zone.current).toBe(zone); }); diff --git a/test/jasmine-patch.spec.ts b/test/jasmine-patch.spec.ts new file mode 100644 index 000000000..345191feb --- /dev/null +++ b/test/jasmine-patch.spec.ts @@ -0,0 +1,29 @@ +describe('jasmine', () => { + let throwOnAsync = false; + let beforeEachZone: Zone = null; + let itZone: Zone = null; + const syncZone = Zone.current; + try { + Zone.current.scheduleMicroTask('dontallow', () => null); + } catch(e) { + throwOnAsync = true; + } + + beforeEach(() => beforeEachZone = Zone.current); + + it('should throw on async in describe', () => { + expect(throwOnAsync).toBe(true); + expect(syncZone.name).toEqual('syncTestZone for jasmine.describe'); + itZone = Zone.current; + }); + + afterEach(() => { + let zone = Zone.current; + expect(zone.name).toEqual('ProxyZone'); + expect(beforeEachZone).toBe(zone); + expect(itZone).toBe(zone); + }); + +}); + +export var _something_so_that_i_am_treated_as_es6_module; diff --git a/test/zone-spec/proxy.spec.ts b/test/zone-spec/proxy.spec.ts index c3d2b3fd6..bfa3ec801 100644 --- a/test/zone-spec/proxy.spec.ts +++ b/test/zone-spec/proxy.spec.ts @@ -20,13 +20,19 @@ describe('ProxySpec', () => { }); it('should assert that it is in or out of ProxyZone', () => { - expect(() => ProxyZoneSpec.assertPresent()).toThrow(); - expect(ProxyZoneSpec.isLoaded()).toBe(false); - expect(ProxyZoneSpec.get()).toBe(undefined); - proxyZone.run(() => { - expect(ProxyZoneSpec.isLoaded()).toBe(true); - expect(() => ProxyZoneSpec.assertPresent()).not.toThrow(); - expect(ProxyZoneSpec.get()).toBe(proxyZoneSpec); + let rootZone = Zone.current; + while(rootZone.parent) { + rootZone = rootZone.parent; + } + rootZone.run(() => { + expect(() => ProxyZoneSpec.assertPresent()).toThrow(); + expect(ProxyZoneSpec.isLoaded()).toBe(false); + expect(ProxyZoneSpec.get()).toBe(undefined); + proxyZone.run(() => { + expect(ProxyZoneSpec.isLoaded()).toBe(true); + expect(() => ProxyZoneSpec.assertPresent()).not.toThrow(); + expect(ProxyZoneSpec.get()).toBe(proxyZoneSpec); + }); }); }); diff --git a/test/zone-spec/task-tracking.spec.ts b/test/zone-spec/task-tracking.spec.ts index e014ae2fb..dabe0f39a 100644 --- a/test/zone-spec/task-tracking.spec.ts +++ b/test/zone-spec/task-tracking.spec.ts @@ -27,6 +27,8 @@ describe('TaskTrackingZone', function() { expect(taskTrackingZoneSpec.macroTasks.length).toBe(0); expect(taskTrackingZoneSpec.microTasks.length).toBe(0); + // If a browser does not have XMLHttpRequest, then end test here. + if (global['XMLHttpRequest']) return done(); const xhr = new XMLHttpRequest(); xhr.open('get', '/', true); xhr.onreadystatechange = () => { @@ -35,7 +37,7 @@ describe('TaskTrackingZone', function() { setTimeout(() => { expect(taskTrackingZoneSpec.macroTasks.length).toBe(0); expect(taskTrackingZoneSpec.microTasks.length).toBe(0); - expect(taskTrackingZoneSpec.eventTasks.length).toBe(2); + expect(taskTrackingZoneSpec.eventTasks.length).not.toBe(0); taskTrackingZoneSpec.clearEvents(); expect(taskTrackingZoneSpec.eventTasks.length).toBe(0); done(); @@ -45,12 +47,7 @@ describe('TaskTrackingZone', function() { xhr.send(); expect(taskTrackingZoneSpec.macroTasks.length).toBe(1); expect(taskTrackingZoneSpec.macroTasks[0].source).toBe('XMLHttpRequest.send'); - - expect(taskTrackingZoneSpec.eventTasks.length).toBe(2); - // one for me - expect(taskTrackingZoneSpec.eventTasks[0].source).toBe('XMLHttpRequest.addEventListener:readystatechange'); - // one for internall tracking of XHRs. - expect(taskTrackingZoneSpec.eventTasks[1].source).toBe('XMLHttpRequest.addEventListener:readystatechange'); + expect(taskTrackingZoneSpec.eventTasks[0].source).toMatch(/\.addEventListener:readystatechange/); }); });