Stefs cleanup (and some test suite memory leak fixes) #4838
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AssertionPrototype.prototype extensions in this file, where causing `let testStore` to be retained…
reopen introduced a mixin which was retained, and who’s new methods retained the current tests `env`. This more accurately patches the methods in-question. Future work should re-evaluate this test, and consider a simpler approach
stefanpenner
changed the title
|
The remaining leak may not be a leak on our end, as the profiler is unable to calculate path to the retainer...
I've walk as many paths as i could manually, and inspected the code in question and cant seem to find a reasonable reason for it to still be around.... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Noticed 8 containers leaked to the end of the test suite, this PR fixes 5 of them (2 of them are yet upstream). This leaves us with 1, and that one appears strange/tricky (will investigate next).
also found these leaks:
The goal of this, is to be sure our own test suite can be used (at-least manually) to detect leaks/tests. So if we can have our test suite not leak, it will be easier to catch leaks going forward (like the 2 RSVP / ES6 promise leaks).