Skip to content

Only add process handlers if graceful cleanup requested#125

Closed
chrisprice wants to merge 1 commit intoraszi:masterfrom
chrisprice:patch-1
Closed

Only add process handlers if graceful cleanup requested#125
chrisprice wants to merge 1 commit intoraszi:masterfrom
chrisprice:patch-1

Conversation

@chrisprice
Copy link
Copy Markdown

I was running into MaxListenersExceededWarning when running Jest tests which referenced this module. I'm new to the project but after a quick look I think the change is safe.

@chrisprice
Copy link
Copy Markdown
Author

Just realised I misinterpreted the expected functionality, I don't think the warning I'm seeing is fixable by this module.

@chrisprice chrisprice closed this Jun 22, 2017
@silkentrance
Copy link
Copy Markdown
Collaborator

@chrisprice I am curious how this error came to. Can you provide me with a link to the sources?

@chrisprice
Copy link
Copy Markdown
Author

@silkentrance
Copy link
Copy Markdown
Collaborator

silkentrance commented Jun 26, 2017

@chrisprice Thanks. I just tried this out and at first, I did see the same error when running Jest in watch mode. After the fourth run of all test suites, the error would show up.
For this, I removed the work around from e2e/helpers.js.

I then modified node_modules/selenium-webdriver/node_modules/tmp to log some output to the console, including the array of exit listeners.

Turns out that this array is always empty before tmp installs its listeners, so this does not seem to be related to or even caused by tmp.

I then removed the node_modules directory and ran npm install again.
After this the warning message was gone, even with the work around in e2e/helpers.js commented out.

@chrisprice
Copy link
Copy Markdown
Author

Thanks for having a play around with this. I must admit, I'm now more confused!

My understanding was that Jest runs each spec file in it's own module context such that any local state in modules is lost between different spec files and different runs of the same spec file. However, listeners added to global modules are outside the scope of the module context and are therefore carried across between different spec files and different runs of the same spec file.

Based on that model, your first 3 paragraphs make sense but I'm very confused by why it worked in the last paragraph...

FWIW I'm happy with the fix that I have for now, if it causes a problem in the future I'll be sure to ping this thread with the solution or at least a more detailed description of the problem...

Thanks again!

@silkentrance
Copy link
Copy Markdown
Collaborator

@chrisprice jest uses child processes to run each test suite in parallel unless you run them "in band".

As for the module context, I think that tmp should know when it already installed its handlers so that it will not install them multiple times. I am currenty trying to think up a possible way to accomplish this.

@silkentrance
Copy link
Copy Markdown
Collaborator

@chrisprice found a solution to the problem. However, this requires that dependent other projects must update their package.json in order to make use of the new functionality and prevent tmp from installing its handlers multiple times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants