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

dispose Mocha Runner after use to avoid MaxlistenersExceededWarning #41403

Merged
merged 2 commits into from
Nov 11, 2020

Conversation

boneskull
Copy link
Contributor

@boneskull boneskull commented Nov 4, 2020

This change eliminates the frequent MaxListenersExceededWarnings emitted when running gulp runtests-parallel. The Runner#dispose method removes all EE listeners the Runner instance created. When using the Runner manually like this, you'll want to call this function. It is a relatively new, and the Mocha typings have not been updated to reflect its presence (hence the @ts-ignore).

Apologies, I could not find an issue here associated with this change, but I'm sure there is one somewhere. I'll leave this as a draft until I find it. Created #41404.

Fixes #41404, mochajs/mocha#4329 reported by @weswigham


Commit summary:

  • call runner.dispose() on listener for end event
  • removed manual unhandledRejection listener as Mocha v8.2.0 now has one

- removed manual `unhandledRejection` listener as Mocha v8.2.0 now has one
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Nov 4, 2020
@typescript-bot
Copy link
Collaborator

The PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@DanielRosenwasser
Copy link
Member

I've got a DefinitelyTyped PR over at DefinitelyTyped/DefinitelyTyped#49382 if you'd like to take a look so we can get rid of the // @ts-ignore comment.

@boneskull
Copy link
Contributor Author

@DanielRosenwasser not sure if you're asking me to review the typings. if it works for you, it works for me... I haven't figured out how to consume them when working in Mocha itself

@boneskull
Copy link
Contributor Author

but happy to update this PR when those changes land

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 11, 2020

@types/mocha@8.0.4 should have the right changes.

@DanielRosenwasser
Copy link
Member

I've updated the PR, let's see if it works.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 11, 2020

@amcasey are the build failures just a matter of updating the lockfile?

@amcasey
Copy link
Member

amcasey commented Nov 11, 2020

@DanielRosenwasser I think I'm missing a step. Was dispose added recently? When I check out this branch, it builds locally, so I'm missing something if it's a package-lock.json issue.

Edit: I wasn't building the tests locally...

@amcasey
Copy link
Member

amcasey commented Nov 11, 2020

Okay, I'm finally caught up. Yes, this PR branch is using @types/mocha@8.0.3, whereas master is on @types/mocha@8.0.4. I haven't seen many such issues, so I'd suggest we treat it as a one-off and just merge/rebase this PR to pick up the new types.

@DanielRosenwasser DanielRosenwasser merged commit 40adb27 into microsoft:master Nov 11, 2020
@DanielRosenwasser
Copy link
Member

Thanks @boneskull!

@boneskull boneskull deleted the fix-mocha-leak branch November 11, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

frequent MaxListenersExceededWarnings when running test suite
5 participants