-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix(core): fail to clean up framework observers #11782
fix(core): fail to clean up framework observers #11782
Conversation
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.
LGTM
The merge-base changed after approval.
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.
LGTM!
f4dc3bf
6043df7
to
f4dc3bf
Compare
f4dc3bf
to
11c5509
Compare
}); | ||
|
||
afterAll(() => { | ||
jest.useRealTimers(); |
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.
nit: I'd prefer using afterEach
(see previous suggestion) for better isolation between tests, but not a blocker.
Description of changes
Added logic to clean up the Framework detection observers, and ignore new incoming observers after the detection completes.
Issue #, if available
#11770
Description of how you validated changes
Unit tests.
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.