qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
59 stars 18 forks source link

Recommend purging actual/expected values of assertions #100

Closed trentmwillis closed 3 years ago

trentmwillis commented 7 years ago

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 commented 3 years ago

This was removed in dce64599d541, 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 https://github.com/js-reporters/js-reporters/issues/117.

Two alternative ideas:

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 commented 3 years ago

Superseded by https://github.com/js-reporters/js-reporters/pull/129.