-
Notifications
You must be signed in to change notification settings - Fork 248
fix(DynamicParser): Correctly handle throwing exceptions from method. #1064
fix(DynamicParser): Correctly handle throwing exceptions from method. #1064
Conversation
it('should rethrow an error from a function', () { | ||
expect(() { | ||
parser("causeException()").eval(new TestData()); | ||
}).toThrow("Error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marko,
What about those tests:
- calling a method that does not exists ?
- calling a method that exists but refers to a non-existent mehod / fiel ?
and making sure that the exception / message is the correct one (refer to the source exception). I have a PR pending on guiness to make it easier to test Exception (expect a type).
otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vicb Test for calling a method that does not exists is already there. I updated the test to throw a NoSuchMethodError instead of just Error. The idea of this TC is that any error that is thrown from a method should not be swallowed/modified. It does not necessarily need to be caused by a call to non existent method/field.
you're right Marko |
@@ -60,7 +60,7 @@ packages: | |||
path: | |||
description: path | |||
source: hosted | |||
version: "1.1.0" | |||
version: "1.2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this file from the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the rationale for that ? (just to know, I have already committed updated lock files)
Fixedup PR here: https://github.com/angular/angular.dart/tree/pr-1064 |
Fixes #971