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.
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
feat(testability): Expose function frameworkStabilizers #5485
Conversation
94ed92f
to
16c6428
Compare
Julie, can you take a look? |
16c6428
to
606fc4b
Compare
@@ -16,6 +16,8 @@ export class Testability { | |||
/** @internal */ | |||
_pendingCount: number = 0; | |||
/** @internal */ | |||
_didWork: boolean = false; |
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.
A comment on what this is for might be useful for someone unfamiliar with the goal of our framework stabilizers.
Sorry for the slow review! Looks like the dart tests are failing. Other than that looks ok. |
Also, modify tests at https://github.com/angular/angular/blob/master/modules/angular2/test/core/testability/testability_spec.ts to look for |
606fc4b
to
a869393
Compare
Added comment for didWork and added tests. Still trying to figure out what's wrong with the dart tests ... |
19c4f05
to
3530d8c
Compare
Still can't figure out what is wrong and angular does not build on my local machine properly, so it's making life difficult. |
3530d8c
to
9aa8956
Compare
Ok...slight (although not very useful) update. The same code passes when I run the e2e tests locally =/ |
This looks good to me! Assigning to @tbosch for a quick sanity check. cc @goderbauer |
if (!global.frameworkStabilizers) { | ||
global.frameworkStabilizers = ListWrapper.createGrowableSize(0); | ||
} | ||
global.frameworkStabilizers.push(whenAllStable); |
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.
@juliemr Can we write a test that uses global.frameworkStabilizers
?
Looks good as well. Would be nice to have a test for the new feature though... (i.e. access |
0c68225
to
666913a
Compare
Added e2e test |
Looks good! |
User @hankduan does not have PR merging privileges. |
666913a
to
8152833
Compare
squashed both commits together |
Merging PR #5485 on behalf of @rkirov to branch presubmit-rkirov-pr-5485. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.