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 detects 1 open handle after test run #31

Closed
roberttaylor426 opened this issue Sep 10, 2019 · 6 comments
Closed

Jest detects 1 open handle after test run #31

roberttaylor426 opened this issue Sep 10, 2019 · 6 comments

Comments

@roberttaylor426
Copy link

roberttaylor426 commented Sep 10, 2019

We're using Mockttp with Jest, with the --detectOpenHandles option set, and after our tests complete we're seeing the following warning:

Jest has detected the following 1 open handle potentially keeping Jest from exiting:
  ●  Timeout
      at Object.<anonymous> (node_modules/mockttp/src/util/socket-util.ts:68:5)

Taking a look at the code in question it seems clear why this is happening, but is there a suggested workaround? We'd rather avoid the false-negative.

Happy to raise a PR if needed.

@pimterry
Copy link
Member

If you run the tests without --detectOpenHandles, does it exit successfully, or is this actually blocking your tests from exiting for real?

As far as I know, the interval there shouldn't block anything, as it calls .unref(), which explicitly tells node that this timer isn't important and shouldn't keep the process alive.

My best guess here is that this is indeed a false herring, and this is really a bug in Jest's open handle detection, since they should ignore anything that has been unref'd. Does that sound plausible?

If so, I'm afraid options to fix this on my end nicely are limited, but it should be very easy to fix in Jest for Node 11+: they simply need to ignore open handles for timeouts where resource.hasRef() is false.

Unfortunately hasRef was only added in v11, so for older versions that doesn't help, but you could polyfill that (e.g. by patching unref).

Anyway, for you in the meantime, if you want to fix this right now, I would monkeypatch setInterval to drop the interval entirely. Wherever you first require('mockttp'), I think you can do something like:

const setInterval = global.setInterval;
global.setInterval = () => {}; // Ignore all intervals
const { getLocal } = require('mockttp');
global.setInterval = setInterval; // Start listening to intervals again

As far as I'm aware, there's no strictly necessary intervals that Mockttp uses anywhere, it's just this one that keeps the interfaces list up to date and you'll survive without that.

There are potentially other things we could do here (add an option/env var to disable the interval for example), but given that it looks a lot like a Jest bug I'd rather fix it properly there instead.

@roberttaylor426
Copy link
Author

Thanks for the quick response and the various suggestions. The tests do indeed exit successfully, it's just an undesirable warning message displaying at the end of the test run.

I take your point about the .unref(); I'll have a closer look at what Jest is doing and try to better understand their intent. As you say, it might be something that could be fixed / refined on their side.

@pimterry
Copy link
Member

I did some testing and I'm confident this is both a Jest bug, and an easy to fix bug (for Node 11+ at least).

I've filed an issue over there to get that sorted: jestjs/jest#8939. I'll leave this open until that's resolved though.

@roberttaylor426
Copy link
Author

You beat me to it :-)

Thanks!

@pimterry
Copy link
Member

pimterry commented Oct 7, 2019

That Jest PR still hasn't been released yet, but Mockttp conveniently no longer uses that setInterval (as of 0.18.0), so I'm going to close this regardless. Thanks for the report!

@pimterry pimterry closed this as completed Oct 7, 2019
@roberttaylor426
Copy link
Author

Thanks very much for your help!

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

No branches or pull requests

2 participants