facebook-atom / jest-electron-runner

custom test runner for Jest that allows tests to be run in Electron environment
MIT License
189 stars 33 forks source link

Event emitter memory leak #14

Closed JM-Mendez closed 5 years ago

JM-Mendez commented 5 years ago

So I've noticed a node memory leak warning after failing a few tests.

Reproduction: failing more than 10 consecutive tests should trigger the warning to the console

Solution: I resolved it by using a helper function to ensure only one listener per channel is registered.

If you'd like, I can submit a pull request on it from this branch: https://github.com/JM-Mendez/jest-electron-runner/tree/event-listener-memory-leak

the relevant changes are: line #55 lines 111-120

aaronabramov commented 5 years ago

@JM-Mendez pull request would be awesome! (you'll need to sign a CLA though)

also instead of checking for listenerCount we should probably keep some local state like processEventRegistered, because other parts of the test runner hook into process as well and can confuse the counters

JM-Mendez commented 5 years ago

Ok, did as you suggested, signed the CLA, and submitted the pull request. Let me know if there's anything else I overlooked.

aaronabramov commented 5 years ago

closed via https://github.com/facebook-atom/jest-electron-runner/pull/15