Memory leak fix: release console output reference after printed to stdout.

[scotthovestadt]

Summary

The reference to the console output is printed to stdout when the test completes and not used afterwards. Unfortunately, it's currently preserved, never being released. It's not used after that point and can be garbage collected.

I also standardized the "no buffer" output to undefined, when it previously could have been an empty array or null.

Test plan

• All tests pass.
• Console output still works as expected.
• Memory does not leak.
• JSON output is unchanged.

[thymikee]

Nice :)

[SimenB]

I don't think the reporter should be responsible for mutating the testresult object. The memory consumption would be reintroduced with custom reporters. Can we move this to the runner or jest-core instead?

[scotthovestadt]

Great feedback, moved to jest-core. To be honest, I just blindly followed the pattern of how coverage was previously released without thinking about it too carefully. Thanks for your review.

[codecov-io]

Codecov Report

Merging #8233 into master will not change coverage.
The diff coverage is 50%.

@@           Coverage Diff           @@
##           master    #8233   +/-   ##
=======================================
  Coverage   62.33%   62.33%           
=======================================
  Files         265      265           
  Lines       10553    10553           
  Branches     2565     2567    +2     
=======================================
  Hits         6578     6578           
  Misses       3387     3387           
  Partials      588      588

Impacted Files	Coverage Δ
...us/src/legacy-code-todo-rewrite/jestAdapterInit.ts	0% <ø> (ø)	⬆️
packages/jest-test-result/src/helpers.ts	0% <ø> (ø)	⬆️
packages/jest-console/src/CustomConsole.ts	68.75% <0%> (ø)	⬆️
packages/jest-reporters/src/default_reporter.ts	82.6% <0%> (-0.25%)	⬇️
packages/jest-console/src/BufferedConsole.ts	70% <100%> (ø)	⬆️
packages/jest-core/src/ReporterDispatcher.ts	82.35% <100%> (+1.1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e08be02...eb1d72c. Read the comment docs.

[alexey-temnikov]

This fix broke compatibility with ability to print console logs in the reports (like jest-html-reporter).
As console object is cleared up onTestResult it is not available any longer for other reports during onRunComplete in other reports so logs are eventually going missing.

The feature to incorporate certain logs from the test execution into the report is quite useful.

Reference issue is #Hargne/jest-html-reporter#61

Rather than completely disabling it, it would be great to see a parameter / hook, so developers have ability to define, if they want to keep logs, throw them right away, or process in a certain way (e.g. persist in a report file, or shorten, to avoid huge memory consumption in case tests generate huge amount of logs). Or at least document that capability (as it can be currently supported by using custom reporter).

[SimenB]

We can just move the cleanup to a later part of the lifecycle if needed - I think that's cleaner than more config. Mind opening up a separate issue with a quick reproduction?

[scotthovestadt]

It definitely should not be available in onRunComplete, since by definition that means it'll be held onto for the entire run time. This simply doesn't scale past a certain point.

The reporter should be able to get at the console logs as each test completes individually and store them for later access if needed.

If this doesn't work for some reason file an issue and I'm willing to fix that.

[alexey-temnikov]

@SimenB, @scotthovestadt
Thank you for your blazing fast reply :)

It make sense, reporter shall hook into onTestResult and efficiently (minimizing memory footprint) store logs. Perhaps we will see that support in future.

For a quick and dirty WA to make current reporters compatible with a newer jest version (again reference to jest-html-reporter) I'm just "restoring" the value for console variable in a custom "fake" reporter, hooking into onTestResult.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.