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

Jest tests time out since 5.1.0 #45

Closed
cegerard opened this issue Jan 14, 2020 · 3 comments · Fixed by #68
Closed

Jest tests time out since 5.1.0 #45

cegerard opened this issue Jan 14, 2020 · 3 comments · Fixed by #68

Comments

@cegerard
Copy link

cegerard commented Jan 14, 2020

Hi.
I use get-port in my jest test and it works perfectly since the 5.1.0 recent update.

Now my jest tests timeout and jest warns about some open handles from get-port.
Jest warn about this line https://github.com/sindresorhus/get-port/blob/master/index.js#L51 in particular.

My solution to make my test back to work is to downgrade get-port to 5.0.0.

Has anyone encountered this issue ?

@gzaripov
Copy link

gzaripov commented Mar 25, 2020

Yes, I did.
Problem is in setInterval inside the library, I think it's the right decision to check that timer wasn't in use, but logic with two generations of times looks overcomplicated for me. On a machine we have 65,535 ports, do we really need two generations? We can add ports to Set usedPorts and never use them except when a user specifies that port. In case when we want to clear ports anyway we can just write them in map "port to time" when we returned them, then in case when we cant get port, we can iterate through map and clear ports those are likely unused

@dobesv
Copy link

dobesv commented May 22, 2020

I think this interval thing is a bit weird and should probably be disabled by default. We're only calling get-port once on startup, we never call it again. We don't need a timer running forever.

@sindresorhus
Copy link
Owner

Yeah, I think it should use setTimeout instead of setInterval.

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

Successfully merging a pull request may close this issue.

4 participants