Skip to content

Conversation

@b-laporte
Copy link

This PR unlocks the directive validation error tests for dart. Apparently some previous cleanup done by @vicb allows them to work without using Future.sync() - thanks !

@vicb
Copy link
Contributor

vicb commented Feb 26, 2015

@b-laporte actually I've tried this yesterday, it was not working, somebody else should have fixed someting...

@vicb
Copy link
Contributor

vicb commented Feb 26, 2015

Could it be a side effect of 9b08ab3 ?

@vicb
Copy link
Contributor

vicb commented Feb 26, 2015

Humm... I still have errors in Dart for

// TODO(vicb): Check why errors this fails with Dart
this is something I need to investigate

Copy link
Contributor

Choose a reason for hiding this comment

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

isn't expect().toEqual() ok here ?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't check toEqual() - but toBe() doesn't work as it seems we get a raw string in Dart... ?

Copy link
Author

Choose a reason for hiding this comment

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

ok - toEqual does work - so I will submit an update - thx!

@b-laporte
Copy link
Author

@vicb yes indeed something has changed as this PR didn't work yesterday - and I actually prepared another PR to include Future.sync() (through a new PromiseWrapper.sync()) as it was solving the issue - but when I rebased to submit the PR I discovered that everything was working without the new changes (!) - so I simply dropped my original PR for this new one... But the real discovery for me was to learn that try/catch in dart in an async call stack doesn't work at all as in JavaScript (good to know - but a bit scary to have a common code base..) - cf. https://www.dartlang.org/articles/futures-and-error-handling/

@vicb
Copy link
Contributor

vicb commented Feb 26, 2015

In Dart you can have string !== string if one of them is interned, there should be no pb with .toEqualthough.

I haven't had time to check what is wrong with the Promise/Future error handling. We should create an issue to investigate, can you do it ?

@vicb vicb added the @lgtm label Feb 26, 2015
@mhevery mhevery added the action: merge The PR is ready for merge by the caretaker label Feb 26, 2015
@mhevery mhevery modified the milestone: M5: Early Adopters Feb 26, 2015
@b-laporte
Copy link
Author

@ViB I would be fine to open an issue - but for this I need a error sample, and I can't reproduce it anymore - what happened was that the error was not caught in internal try/catch and was caught in the zone instead. But now everything works as expected..

@b-laporte b-laporte closed this in 5f0c0ea Mar 2, 2015
b-laporte pushed a commit that referenced this pull request Mar 2, 2015
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants