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

--detectOpenHandles does not detect handles opened in test function with done callback #11377

Closed
Mr0grog opened this issue May 6, 2021 · 2 comments · Fixed by #11382
Closed

Comments

@Mr0grog
Copy link
Contributor

Mr0grog commented May 6, 2021

🐛 Bug Report

The --detectOpenHandles option fails to detect/warn about handles that were opened in test functions with a done callback. (It seems to work fine for promise-returning or async test functions.) I noticed this while looking into issues with Supertest in #8554 (comment)

To Reproduce

Given the following test file:

const http = require("http");

describe("detectOpenHandles", () => {
  it("should detect an open server when using a `done` function", (done) => {
    const server = http.createServer((_request, response) => response.end("ok"));
    server.listen(0, () => done());
  });
});

Running Jest with --detectOpenHandles hangs, but does not print any information about the open TCPSERVERWRAP handle:

$ jest --detectOpenHandles open-handles.test.js
 PASS  ./open-handles.test.js
  detectOpenHandles
    ✓ should detect an open server when using a `done` function (7 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.297 s
Ran all test suites matching /open-handles.test.js/i

Expected Behavior

In contrast, using an async function or returning a promise works as expected. Given the following test file:

const http = require("http");

describe("detectOpenHandles", () => {
  it("should detect an open server when using a promise", () => {
    const server = http.createServer((_request, response) => response.end("ok"));
    return new Promise(resolve => server.listen(0, resolve));
  });
});

Running it successfully detects and prints information about the open server:

$ jest --detectOpenHandles open-handles.test.js
 PASS  ./open-handles.test.js
  detectOpenHandles
    ✓ should detect an open server when using a promise (6 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.956 s
Ran all test suites matching /open-handles.test.js/i.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  TCPSERVERWRAP

      89 |   it("should detect an open server when using a promise", () => {
      90 |     const server = http.createServer((_request, response) => response.end("ok"));
    > 91 |     return new Promise(resolve => server.listen(0, resolve));
         |                                          ^
      92 |   });
      93 | });

      at open-handles.test.js:91:42
      at Object.<anonymous> (open-handles.test.js:91:12)

Cause

It looks like the issue is in how Jest decides which handles to track. In jest-core/src/collectHandles.ts, handles are only tracked if stackIsFromUser() returns true. It checks for the names of wrappers that run the test functions: https://github.com/facebook/jest/blob/ba84480a5603aeeb94184ffc26f9b39024cdcd6c/packages/jest-core/src/collectHandles.ts#L18-L38

That works great for functions that don’t take a done callback. They get wrapped with a function named asyncJestTest, which stackIsFromUser() looks for: https://github.com/facebook/jest/blob/ba84480a5603aeeb94184ffc26f9b39024cdcd6c/packages/jest-jasmine2/src/jasmineAsyncInstall.ts#L123-L153

But functions that take a done callback don’t get wrapped at all, and are anonymous or have no name that stackIsFromUser() knows to look for: https://github.com/facebook/jest/blob/ba84480a5603aeeb94184ffc26f9b39024cdcd6c/packages/jest-jasmine2/src/jasmineAsyncInstall.ts#L111-L113

It looks like lifecycle functions will experience the same issue (although I haven’t tested), given the similar code in promisifyLifeCycleFunction().

The simplest fix here might be to simply wrap functions that take a done callback with a function, too (e.g. asyncJestTestWithCallback) and add that to the list of things to watch for in stackIsFromUser().

envinfo

  System:
    OS: macOS 10.15.7
    CPU: (4) x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
  Binaries:
    Node: 16.1.0 - ~/.nvm/versions/node/v16.1.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.11.2 - ~/.nvm/versions/node/v16.1.0/bin/npm
  npmPackages:
    jest: ^26.6.3 => 26.6.3
@Mr0grog
Copy link
Contributor Author

Mr0grog commented May 6, 2021

Note: this only happens with the Jasmine test runner; Circus [which I just learned about!] does not seem to exhibit the issue.

Mr0grog added a commit to Mr0grog/jest that referenced this issue May 7, 2021
Wrap test and lifecycle functions that take a `done` callback in a named function so that they can be detected as user code in the call stack. This lets the `collectHandles` module in jest-core know to track async resources created in those functions.

Fixes jestjs#11377.
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant