Skip to content

fix(testing): remove test zone for now and rely on returned promises #6601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
juliemr wants to merge 1 commit into from
Closed

fix(testing): remove test zone for now and rely on returned promises #6601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 18 additions & 70 deletions modules/angular2/src/testing/testing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,77 +126,23 @@ function _isPromiseLike(input): boolean {
return input && !!(input.then);
}

function runInTestZone(fnToExecute, finishCallback, failCallback): any {
var pendingMicrotasks = 0;
var pendingTimeouts = [];

var ngTestZone = (<Zone>global.zone)
.fork({
onError: function(e) { failCallback(e); },
'$run': function(parentRun) {
return function() {
try {
return parentRun.apply(this, arguments);
} finally {
if (pendingMicrotasks == 0 && pendingTimeouts.length == 0) {
finishCallback();
}
}
};
},
'$scheduleMicrotask': function(parentScheduleMicrotask) {
return function(fn) {
pendingMicrotasks++;
var microtask = function() {
try {
fn();
} finally {
pendingMicrotasks--;
}
};
parentScheduleMicrotask.call(this, microtask);
};
},
'$setTimeout': function(parentSetTimeout) {
return function(fn: Function, delay: number, ...args) {
var id;
var cb = function() {
fn();
ListWrapper.remove(pendingTimeouts, id);
};
id = parentSetTimeout(cb, delay, args);
pendingTimeouts.push(id);
return id;
};
},
'$clearTimeout': function(parentClearTimeout) {
return function(id: number) {
parentClearTimeout(id);
ListWrapper.remove(pendingTimeouts, id);
};
},
});

return ngTestZone.run(fnToExecute);
}

function _it(jsmFn: Function, name: string, testFn: FunctionWithParamTokens | AnyTestFn,
testTimeOut: number): void {
var timeOut = testTimeOut;

if (testFn instanceof FunctionWithParamTokens) {
jsmFn(name, (done) => {
var finishCallback = () => {
// Wait one more event loop to make sure we catch unreturned promises and
// promise rejections.
setTimeout(done, 0);
};
var returnedTestValue =
runInTestZone(() => testInjector.execute(testFn), finishCallback, done.fail);
var returnedTestValue;
try {
returnedTestValue = testInjector.execute(testFn);
} catch (err) {
done.fail(err);
return;
}

if (testFn.isAsync) {
if (_isPromiseLike(returnedTestValue)) {
(<Promise<any>>returnedTestValue).then(null, (err) => { done.fail(err); });
(<Promise<any>>returnedTestValue).then(() => { done(); }, (err) => { done.fail(err); });
} else {
done.fail('Error: injectAsync was expected to return a promise, but the ' +
' returned value was: ' + returnedTestValue);
Expand All @@ -206,6 +152,7 @@ function _it(jsmFn: Function, name: string, testFn: FunctionWithParamTokens | An
done.fail('Error: inject returned a value. Did you mean to use injectAsync? Returned ' +
'value was: ' + returnedTestValue);
}
done();
Copy link
Copy Markdown
Contributor

@alexeagle alexeagle Jan 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems wrong, why should we be done() here? The promise may not have resolved yet.

Copy link
Copy Markdown
Contributor

@alexeagle alexeagle Jan 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind, not used to the github collapsed lines view...

}
}, timeOut);
} else {
Expand All @@ -232,17 +179,17 @@ export function beforeEach(fn: FunctionWithParamTokens | AnyTestFn): void {
// The test case uses inject(). ie `beforeEach(inject([ClassA], (a) => { ...
// }));`
jsmBeforeEach((done) => {
var finishCallback = () => {
// Wait one more event loop to make sure we catch unreturned promises and
// promise rejections.
setTimeout(done, 0);
};

var returnedTestValue =
runInTestZone(() => testInjector.execute(fn), finishCallback, done.fail);
var returnedTestValue;
try {
returnedTestValue = testInjector.execute(fn);
} catch (err) {
done.fail(err);
return;
}
if (fn.isAsync) {
if (_isPromiseLike(returnedTestValue)) {
(<Promise<any>>returnedTestValue).then(null, (err) => { done.fail(err); });
(<Promise<any>>returnedTestValue).then(() => { done(); }, (err) => { done.fail(err); });
} else {
done.fail('Error: injectAsync was expected to return a promise, but the ' +
' returned value was: ' + returnedTestValue);
Expand All @@ -252,6 +199,7 @@ export function beforeEach(fn: FunctionWithParamTokens | AnyTestFn): void {
done.fail('Error: inject returned a value. Did you mean to use injectAsync? Returned ' +
'value was: ' + returnedTestValue);
}
done();
}
});
} else {
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/test/testing/static_assets/test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span>from external template</span>
38 changes: 36 additions & 2 deletions modules/angular2/test/testing/testing_public_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ class TestViewProvidersComp {
constructor(private fancyService: FancyService) {}
}

@Component({selector: 'external-template-comp'})
@View({templateUrl: '/base/modules/angular2/test/testing/static_assets/test.html'})
Copy link
Copy Markdown
Contributor

@alexeagle alexeagle Jan 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty strange that karma URLs leak into the component definitions, but seems like the right thing here, since it's defined in the test

class ExternalTemplateComp {
}

@Component({selector: 'bad-template-comp'})
@View({templateUrl: 'non-existant.html'})
class BadTemplateUrl {
}

export function main() {
describe('angular2 jasmine matchers', () => {
Expand Down Expand Up @@ -273,11 +282,12 @@ export function main() {
restoreJasmineIt();
});

it('should fail when an asynchronous error is thrown', (done) => {
// TODO(juliemr): reenable this test when we are using a test zone and can capture this error.
xit('should fail when an asynchronous error is thrown', (done) => {
var itPromise = patchJasmineIt();

it('throws an async error',
inject([], () => { setTimeout(() => { throw new Error('bar'); }, 0); }));
injectAsync([], () => { setTimeout(() => { throw new Error('bar'); }, 0); }));

itPromise.then(() => { done.fail('Expected test to fail, but it did not'); }, (err) => {
expect(err.message).toEqual('bar');
Expand All @@ -304,6 +314,19 @@ export function main() {
restoreJasmineIt();
});

it('should fail when an XHR fails', (done) => {
var itPromise = patchJasmineIt();

it('should fail with an error from a promise',
injectAsync([TestComponentBuilder], (tcb) => { return tcb.createAsync(BadTemplateUrl); }));

itPromise.then(() => { done.fail('Expected test to fail, but it did not'); }, (err) => {
expect(err).toEqual('Failed to load non-existant.html');
done();
});
restoreJasmineIt();
});

describe('using beforeEachProviders', () => {
beforeEachProviders(() => [bind(FancyService).toValue(new FancyService())]);

Expand Down Expand Up @@ -428,5 +451,16 @@ export function main() {
.toHaveText('injected value: mocked out value');
});
}));

it('should allow an external templateUrl',
injectAsync([TestComponentBuilder], (tcb: TestComponentBuilder) => {

return tcb.createAsync(ExternalTemplateComp)
.then((componentFixture) => {
componentFixture.detectChanges();
expect(componentFixture.debugElement.nativeElement)
.toHaveText('from external template\n');
});
}));
});
}
16 changes: 16 additions & 0 deletions modules_dart/angular2_testing/test/angular2_testing_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ class TestService {
}
}

@Component(selector: 'external-template-cmp')
@View(templateUrl: 'test_template.html')
class ExternalTemplateComponent {
ExternalTemplateComponent() {
}
}

class MyToken {}

const TEMPLATE =
Expand Down Expand Up @@ -79,6 +86,15 @@ void main() {
expect(rootTC.debugElement.nativeElement.text, equals('1;2;3;'));
});

ngTest('should allow a component using a templateUrl', (TestComponentBuilder tcb) async {
var rootTC = await tcb
.createAsync(ExternalTemplateComponent);

rootTC.detectChanges();

expect(rootTC.debugElement.nativeElement.text, equals('from external template\n'));
});

group('expected failures', () {
ngTest('no type in param list', (notTyped) {
expect(1, equals(2));
Expand Down
1 change: 1 addition & 0 deletions modules_dart/angular2_testing/test/test_template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span>from external template</span>