Skip to content

Conversation

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Apr 12, 2023

This PR introduces a simple internal utility API that can be leveraged by ember-qunit and @ember/test-helpers to allow applications to more easily detect memory leaks within their application.


Example of running a test that intentionally holds on to the owner instance (e.g. "bad test"):

Screenshot 2023-04-12 at 5 53 11 PM
Screenshot 2023-04-12 at 5 56 46 PM

@mixonic
Copy link
Member

mixonic commented Apr 24, 2023

@rwjblue is there any particular reason this shouldn't be made public API instead of intimate?

@rwjblue
Copy link
Member Author

rwjblue commented Apr 24, 2023

@rwjblue is there any particular reason this shouldn't be made public API instead of intimate?

@mixonic - Nope. I'd love for it to be a public API (though someone else can write the RFC :P ), but I figured it would be easier (and non-controversial) to land as private API initially (while we flesh out the ember-qunit and @ember/test-helpers parts of the consumption).

@kategengler
Copy link
Member

Is this still relevant? It is a draft PR

@chriskrycho
Copy link
Contributor

It is definitely still relevant in principle, but neither Rob nor I are driving it from the LinkedIn side (as I am not there and he is focused on totally different things now!) and it is exceedingly unlikely anyone else at LinkedIn is going to be able to land it at this point, unfortunately. Having this as part of both Ember’s own test suite and readily available for apps and addons would be invaluable to the ecosystem in my view, though, as leaked Owners are one of the easiest ways to end up with very bad memory issues in either tests or FastBoot.

@sukima
Copy link
Contributor

sukima commented Dec 29, 2023

Read the code, Not sure I see any outstanding issues with it. Thus, what are the current blockers to moving this from draft to ready for review?

@chriskrycho
Copy link
Contributor

Someone to take ownership of it, for one; and someone to actually write tests for this for another. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants