Skip to content
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

Recommend purging actual/expected values of assertions #100

Closed
trentmwillis opened this issue Apr 4, 2017 · 2 comments
Closed

Recommend purging actual/expected values of assertions #100

trentmwillis opened this issue Apr 4, 2017 · 2 comments

Comments

@trentmwillis
Copy link
Member

The current spec states that Assertion data should contain the actual/expected values as properties. This is good for reporting, but also prone to memory leaks. By holding onto references for values that are non-primitive, we leak memory over the life of a test suite since Tests hold onto an array of Assertions.

Thus, I think we should recommend that actual/expected values of assertions get "purged" (deleted) from Assertion objects once the corresponding Test has finished executing. I can't think of a good reason to hold on to that information throughout the entire test suite and it will reduce the potential for leaking memory.

@Krinkle
Copy link
Member

Krinkle commented Sep 24, 2020

This was removed in dce6459, as I wasn't sure whether it still applies. I'm re-opening to make sure the issue is well-understood and dealt as part of the RFC at #117.

Two alternative ideas:

  • Remove assertions from the TestEnd event data. Perhaps introduce an optional assertion event instead. This would mean testing frameworks can let go of it after emitting this event. It is then up to reporters to decide whether to listen. If they do, the reporter will be the sole reference holder has control over its allocation.
  • Or, keep assertions on TestEnd but instead remove tests from SuiteEnd. (ref Test end assertions prop #80)

Either of these would effectively set a direction where, to obtain information about an event, the reporter has to listen to it. We would not require testing frameworks to hold on and build up large object structures over an entire test run. I think this direction is viable, so long as we're absolutely sure that it isn't cutting off any use cases.

I think building up a large object is a legit use case, and would actually make for a really good demonstration of the CRI standard. We could even ship a simple reporter that effectively does this, e.g. a reporter that gives you a Promise that will resolve to the large object akin to what we emit from runEnd in the current draft. This would leave CRI itself more low-level, with stream-like and memory-friendly characteristics.

Thoughts?

@Krinkle
Copy link
Member

Krinkle commented Feb 14, 2021

Superseded by #129.

@Krinkle Krinkle closed this as completed Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants