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/unmounting select #512
Fix/unmounting select #512
Conversation
this.foundation_.destroy(); | ||
if (this.foundation_) { | ||
this.foundation_.destroy(); | ||
} |
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.
This change (and all the others like it below) are not testable.
This means the PR here will bring the coverage amount down substantially.
It's a bit of a chicken/egg scenario as I need the foundation to not exist to check that destroy()
is not called, but I need the foundation to exist to stub out destroy()
. Sadface.
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.
Merging #512 into master will increase coverage by 0.01%
Huh? If you say so codecov.
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.
Ha - ya that makes sense. I believe all components have a foo calls destory on unmount
test so we should be fine. Thanks for opening PR!
Codecov Report
@@ Coverage Diff @@
## master #512 +/- ##
==========================================
+ Coverage 96.62% 96.63% +0.01%
==========================================
Files 59 59
Lines 1984 1990 +6
Branches 235 241 +6
==========================================
+ Hits 1917 1923 +6
Misses 67 67
Continue to review full report at Codecov.
|
@hvolschenk please sign :) |
opened test PR: #514 |
I signed it |
Fixes #489