angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
96.29k stars 25.52k forks source link

Jest detects open handles in getTestBed after upgrade from angular v12 to v13 #45776

Closed SimonMueller closed 2 years ago

SimonMueller commented 2 years ago

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

Yes

Description

Expected behavior

No open handles after tests are done executing with angular v13.

Actual behavior

Two open handles after tests are done executing in angular v13 which were not present in angular v12. See screenshot:

Original Issue: https://github.com/thymikee/jest-preset-angular/issues/1426

We saw that jest-preset-angular is only an indirection and the same error occurs when we inline the setup done by jest-preset-angular (see the error output below)

Please provide a link to a minimal reproduction of the bug

https://github.com/SimonMueller/jest-preset-angular-open-handle-reproduction

Please provide the exception or error you saw

Jest has detected the following 2 open handles potentially keeping Jest from exiting:

  ●  MESSAGEPORT

      13 |   require('../zone-patch');
      14 | }
    > 15 | const getTestBed = require('@angular/core/testing').getTestBed;
         |                    ^
      16 | const BrowserDynamicTestingModule = require('@angular/platform-browser-dynamic/testing').BrowserDynamicTestingModule;
      17 | const platformBrowserDynamicTesting = require('@angular/platform-browser-dynamic/testing')
      18 |   .platformBrowserDynamicTesting;

      at startWorkerThreadService (node_modules/jest-preset-angular/node_modules/esbuild/lib/main.js:2045:48)
      at Object.<anonymous> (setupJest.ts:15:20)

  ●  WORKER

      13 |   require('../zone-patch');
      14 | }
    > 15 | const getTestBed = require('@angular/core/testing').getTestBed;
         |                    ^
      16 | const BrowserDynamicTestingModule = require('@angular/platform-browser-dynamic/testing').BrowserDynamicTestingModule;
      17 | const platformBrowserDynamicTesting = require('@angular/platform-browser-dynamic/testing')
      18 |   .platformBrowserDynamicTesting;

      at startWorkerThreadService (node_modules/jest-preset-angular/node_modules/esbuild/lib/main.js:2046:16)
      at Object.<anonymous> (setupJest.ts:15:20)

Please provide the environment you discovered this bug in (run ng version)

System:
    OS: Linux 5.15 Ubuntu 22.04 LTS 22.04 (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
  Binaries:
    Node: 14.19.1 - ~/.nvm/versions/node/v14.19.1/bin/node
    npm: 6.14.16 - ~/.nvm/versions/node/v14.19.1/bin/npm
  npmPackages:
    jest: 27.5.1 => 27.5.1

Anything else?

No response

EricDitp commented 2 years ago

thanks for the issue, any news?

AndrewKushnir commented 2 years ago

Thanks for providing a repro, it was very helpful. I've performed the necessary investigation and found out that the problem started to happen between version 13.0.0-next.9 and 13.0.0-next.10 - this is where the ESM support was introduced. I've went to the jest-preset-angular package docs and was able to configure Jest for Angular (using the latest 12.0.1 version of the package) with a newly generated app (created via ng new for version 14.0.0-rc.0). The only extra config that I had to make is to add the "src/polyfills.ts" and "src/test.ts" into the "include" section of the "tsconfig.spec.json" file.

The jest-preset-angular also contains some extra information on how to configure it for Angular v13 and above and how to resolve common issues.

I'm closing this issue, since the problem doesn't seem to exist for the latest version of the jest-preset-angular package.

SimonMueller commented 2 years ago

Hi @AndrewKushnir

Thanks for your response. 👍

Unfortunately i cannot make it work on in my reproduction example even with angular 14.0.0-rc.0 and jest-preset-angular 12.0.1. I have added the "src/polyfills.ts" to the include section but the error is still the same. With the "src/test.ts" i always get an error in my example.

Then i tried it with a newly generated app with the same versions as above and i have the same problems as in my example. So i cannot confirm that it is fixed or maybe i am doing something wrong.

Can you please elaborate on how you got the problem fixed or maybe link to your minimal example?

AndrewKushnir commented 2 years ago

@SimonMueller here are the steps that I took:

  1. Ran ng new to create a new Angular app
  2. Ran npm install -D jest jest-preset-angular @types/jest as recommended in docs
  3. Updated the tsconfig.spec.json and jest.config.js files.
  4. Ran npm t to run tests.

Here is the content of the "jest.config.js" file that I have in my test app:

const { pathsToModuleNameMapper } = require('ts-jest');
const { paths } = require('./tsconfig.json').compilerOptions;

module.exports = {
  preset: 'jest-preset-angular',
  setupFilesAfterEnv: ['<rootDir>/setup-jest.ts'],
  moduleNameMapper: pathsToModuleNameMapper(paths, { prefix: '<rootDir>' }),
  globalSetup: 'jest-preset-angular/global-setup',
  moduleFileExtensions: ['ts', 'html', 'js', 'json', 'mjs'],
};

and also the content of the tsconfig.spec.json file:

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "./out-tsc/spec",
    "module": "CommonJs",
    "types": ["jest"]
  },
  "include": ["src/polyfills.ts", "src/test.ts", "src/**/*.spec.ts", "src/**/*.d.ts"]
}

Also the diff of the package.json file:

diff --git a/package.json b/package.json
index a3bbeb4..995b68b 100644
--- a/package.json
+++ b/package.json
@@ -27,7 +27,10 @@
     "@angular/cli": "~14.0.0-next.13",
     "@angular/compiler-cli": "^14.0.0-next.0",
     "@types/jasmine": "~4.0.0",
+    "@types/jest": "^27.5.1",
     "jasmine-core": "~4.1.0",
+    "jest": "^28.1.0",
+    "jest-preset-angular": "^12.0.1",
     "karma": "~6.3.0",
     "karma-chrome-launcher": "~3.1.0",
     "karma-coverage": "~2.2.0",

There are no other changes that I made to the newly generated Angular app.

Please let me know if it helps.

Thank you, Andrew

SimonMueller commented 2 years ago

Hi @AndrewKushnir

I set up my newly created app with your exact configs and still have issues. The "test.ts" file seems to have issues with jest-preset-angular and it is never mentioned in their docs or examples as far as i can tell. I always get the following error: image

After i removed this file i still get the same error about the open handles.

Do you run your test with the following command "jest --detectOpenHandles --no-cache"? As the --no-cache flag seems quite important in reproducing the issue consistently. If i remove this the test are green after a first run with errors.

Then i got curious and ran their official examples for a v13 and v14 app (e.g. https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v14) with the "--detectOpenHandles --no-cache" flag and the error also popped up.

So for me it seems that the issue still persists.

Regards Simon

AndrewKushnir commented 2 years ago

@SimonMueller I've just tested the example app that you mentioned (https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v14) and got it working by running the npm t command (mentioned in their docs), here is the output:

$ npm t

> example-app-v14@14.0.0 test
> jest --no-cache

Determining test suites to run...
ngcc-jest-processor: running ngcc
 PASS  src/app/model/testing/http-client.spec.ts (27.376 s)
 PASS  src/app/demo/async-helper.spec.ts (27.477 s)
 PASS  src/app/banner/banner-initial.component.spec.ts (27.816 s)
 PASS  src/app/welcome/welcome.component.spec.ts (27.931 s)
 PASS  src/app/shared/highlight.directive.spec.ts (28.012 s)
 PASS  src/app/dashboard/dashboard-hero.component.spec.ts (28.519 s)
 PASS  src/app/model/hero.service.spec.ts (28.537 s)
 PASS  src/app/twain/twain.component.spec.ts (28.581 s)
 PASS  src/app/demo/demo.spec.ts (28.68 s)
 PASS  src/app/banner/banner.component.detect-changes.spec.ts
 PASS  src/app/app-initial.component.spec.ts
 PASS  src/app/banner/banner.component.spec.ts
 PASS  src/app/banner/banner-external.component.spec.ts
 PASS  src/app/shared/title-case.pipe.spec.ts
 PASS  src/app/dashboard/dashboard.component.spec.ts (29.528 s)
 PASS  src/app/shared/canvas.component.spec.ts
 PASS  src/app/hero/hero-list.component.spec.ts (29.717 s)
 PASS  src/app/about/about.component.spec.ts
 PASS  src/app/dashboard/dashboard.component.no-testbed.spec.ts
 PASS  src/app/app.component.spec.ts (29.929 s)
 PASS  src/app/demo/demo.testbed.spec.ts (29.966 s)
 PASS  src/app/app.component.router.spec.ts (30.036 s)
 PASS  src/app/hero/hero-detail.component.spec.ts (30.235 s)

Test Suites: 2 skipped, 23 passed, 23 of 25 total
Tests:       2 skipped, 191 passed, 193 total
Snapshots:   0 total
Time:        32.26 s
Ran all test suites.

If you are still seeing the issue, could you please create a repro in a form of a GitHub repository with a minimal setup required to replicate the issue, so that we can look at it?

Thank you, Andrew

SimonMueller commented 2 years ago

Hi @AndrewKushnir

The command command line option from jest that detects the open handles which are the source of this issue (see original issue description) is "--detectOpenHandles"

What i did to get to the error was to add the option to the test script of https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v14.

The command i used for the test script "npm t" was : "jest --detectOpenHandles --no-cache".

SimonMueller commented 2 years ago

@AndrewKushnir Are there any news on this as this issue is not closed for me? It seems your reproduction to close the issue did not include the jest " --detectOpenHandles" flag.

AndrewKushnir commented 2 years ago

@SimonMueller sorry, I don't have any additional updates at this moment. Do you see the error on each run or just occasionally? I can debug further, just wanted to see if the problem can be reproduced consistently.

MichaelKaaden commented 2 years ago

@AndrewKushnir I'm able to reproduce this issue using the steps given by @SimonMueller.

  1. Clone https://github.com/thymikee/jest-preset-angular.git
  2. cd to jest-preset-angular/examples/example-app-v14
  3. run yarn to install the necessary NPM packages
  4. edit the test script in the package.json file to be "jest --detectOpenHandles --no-cache"
  5. run yarn test

The result is the same for all five test runs I did consecutively: grafik

Hope that helps.

Fylipp commented 2 years ago

@MichaelKaaden @AndrewKushnir My team is experiencing the same problem in our Angular 13 project with Nx. Inlining the side-effects of import 'jest-preset-angular/setup-jest' (importing zone.js directly and initializing the test environment) leads to the open handles being detected on import { getTestBed } from '@angular/core/testing' instead (also MESSAGEPORT and WORKER)

AndrewKushnir commented 2 years ago

@MichaelKaaden thanks for additional information. I was able to reproduce the problem by adding the --detectOpenHandles flag as you mentioned. Looking at the package.json file in the examples/example-app-v14 folder, I also noticed that there is a esm-specific command called yarn test-esm. I tried adding the --detectOpenHandles flag there (to the very end of the command) and tests passed without issues and errors at the end.

I did further debugging and added some logs to the code that throws the "open handles" error, in node_modules/@jest/core/build/cli/index.js after this line:

const {openHandles} = results;

and got the following output:

 ErrorWithStack: MESSAGEPORT
      at MessagePort.emitInitNative (node:internal/async_hooks:201:43)
      at new Worker (node:internal/worker:223:30)
      at startWorkerThreadService (jest-preset-angular/examples/example-app-v14/node_modules/esbuild-wasm/lib/main.js:2167:12)
      at Object.transformSync (jest-preset-angular/examples/example-app-v14/node_modules/esbuild-wasm/lib/main.js:1957:29)
      at NgJestTransformer.process (jest-preset-angular/examples/example-app-v14/node_modules/jest-preset-angular/build/ng-jest-transformer.js:51:114)
      at ScriptTransformer.transformSource (jest-preset-angular/examples/example-app-v14/node_modules/@jest/transform/build/ScriptTransformer.js:619:31)
      at ScriptTransformer._transformAndBuildScript (jest-preset-angular/examples/example-app-v14/node_modules/@jest/transform/build/ScriptTransformer.js:765:40)
      at ScriptTransformer.transform (jest-preset-angular/examples/example-app-v14/node_modules/@jest/transform/build/ScriptTransformer.js:822:19)
      at Runtime.transformFile (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1722:47)
      at Runtime._execModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1639:34)
      at Runtime._loadModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1222:12)
      at Runtime.requireModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1046:12)
      at Runtime.requireModuleOrMock (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1247:21)
      at Object.<anonymous> (jest-preset-angular/examples/example-app-v14/setup-jest.ts:3:1)
      at Runtime._execModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1691:24)
      at Runtime._loadModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1222:12)
      at Runtime.requireModule (jest-preset-angular/examples/example-app-v14/node_modules/jest-runtime/build/index.js:1046:12)
      at jestAdapter (jest-preset-angular/examples/example-app-v14/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:80:15)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at runTestInternal (jest-preset-angular/examples/example-app-v14/node_modules/jest-runner/build/runTest.js:411:16)
      at runTest (jest-preset-angular/examples/example-app-v14/node_modules/jest-runner/build/runTest.js:499:34)

It looks like the error originates from the jest-preset-angular code (the NgJestTransformer class) that tries to process the source code via esbuild. It doesn't seem like the TestBed or the framework code is incorrect, the problem seems to be related to the setup and the way ES modules are processed (likely by the jest-preset-angular code).

We'll discuss this issue with the team further and I will update this thread once we have any news.

alan-agius4 commented 2 years ago

This morning I looked into this and I started by reading the detectOpenHandles https://jestjs.io/docs/cli#--detectopenhandles description, which mentions the following:

Attempt to collect and print open handles preventing Jest from exiting cleanly. Use this in cases where you need to use --forceExit in order for Jest to exit to potentially track down the reason

Okay, but in this case Jest is exiting properly, hence it's unlikely that there are actually any open handles. So I started digging in the Jest code, specifically the handle collection logic.

The interesting part of the code is https://github.com/facebook/jest/blob/6a68a13cd476746d590eabda15ad30a590896822/packages/jest-core/src/collectHandles.ts#L98-L111

if ('hasRef' in resource) {
  if (hasWeakRef) {
    // @ts-expect-error: doesn't exist in v12 typings
    const ref = new WeakRef(resource);
    isActive = () => {
      return ref.deref()?.hasRef() ?? false;
    };
  } else {
    isActive = resource.hasRef.bind(resource);
  }
} else {
  // Handle that doesn't support hasRef
  isActive = alwaysActive;
}

hasRef on ports has only been added in Node.js 18+ and this API is still experimental (https://nodejs.org/api/worker_threads.html#porthasref). When this API doesn't exist Jest has a fallback logic and keeps the handle marked as active even when it might have been closed. https://github.com/facebook/jest/blob/6a68a13cd476746d590eabda15ad30a590896822/packages/jest-core/src/collectHandles.ts#L41

const alwaysActive = () => true;

Reading between the lines of the detectOpenHandles error message ie: Jest has detected the following 2 potentially keeping Jest from exiting it does mention the word potentially which does indicate that it might not be a problem at all.

Closing as there is nothing actionable from our end here.

angular-automatic-lock-bot[bot] commented 2 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.