async-wait-until icon indicating copy to clipboard operation
async-wait-until copied to clipboard

[Bug Report] WaitUntil does not close handlers so jest stuck and waits

Open npwork opened this issue 3 years ago • 8 comments

Describe the bug waitUntil does not close all open handlers so jest sits and waits

To Reproduce Steps to reproduce the behavior:

  1. Create jest test like
const end = Date.now() + 1000
await waitUntil(() => Date.now() < end)
  1. Run test

You will see

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

Expected behavior All handlers are closed

Screenshots If you run with --detectOpenHandles you will see

Screen Shot 2022-02-09 at 6 55 06 PM

Additional context Add any other context about the problem here.

npwork avatar Feb 09 '22 17:02 npwork

Workaround is to disable timing out:

{
  timeout: Number.POSITIVE_INFINITY,
}

To fix this, the promise returned by delay() would need to be somehow cancellable based on the other promise resolving, but the way the code is currently written this would be a fairly big rewrite.

aldeed avatar Jun 12 '22 18:06 aldeed

@aldeed the problem's never-ending predicate support – and not the delay itself, since it's not called when the specified timeout is Infinity. I'll remove this test and infinite wait times from the documentation (but not from the implementation, to keep any existing consumers working).

devlato avatar Jun 13 '22 07:06 devlato

@devlato Maybe I'm not understanding you. I meant that adding an infinite timeout solved this for me because it then doesn't call delay() due to a specific check I noticed in the code. It's only an infinite timeout that can work around this issue because of that explicit check for it. Any actual timeout value causes creation of the delay promise, which is still unresolved at the time that the code exits.

The way I would normally fix this in the code is something like:

let otherPromiseResolved = false;

const delay = (delayMs) => {
  const start = Date.now();
  return new Promise((resolve) => {
    const interval = setInterval(() => {
      if (Date.now() - start > delayMs || otherPromiseResolved) {
        clearInterval(interval);
        resolve();
      }
    }, 500);
  )
}

// When the promise that `delay` is racing against resolves, set otherPromiseResolved = true.

The key point being to somehow inform delay that the promise it is racing against has resolved so that it can immediately resolve. But because there are several layers in this codebase, I wasn't sure how best to implement this pattern here.

aldeed avatar Jun 14 '22 20:06 aldeed

@npwork Very much interested in a solution here or an alternative. This is blocking our test suite as well.

mgagliardo91 avatar Aug 10 '22 21:08 mgagliardo91

@npwork @devlato This is been kept open for long, any progress made on this as its blocking us too. Thanks in advance

NerdishShah avatar Jan 16 '23 04:01 NerdishShah

@NerdishShah will try and fix it this weekend - thanks for reminding me!

devlato avatar Jan 26 '23 02:01 devlato

@NerdishShah will try and fix it this weekend - thanks for reminding me!

May I ask if there's any update on this?

NerdishShah avatar Feb 23 '23 05:02 NerdishShah

A gentle reminder

NerdishShah avatar Apr 01 '23 09:04 NerdishShah