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

Improve test performance #2377

Closed
FERNman opened this issue Jan 25, 2020 · 12 comments · Fixed by #3766
Closed

Improve test performance #2377

FERNman opened this issue Jan 25, 2020 · 12 comments · Fixed by #3766
Labels
outdated scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx type: feature

Comments

@FERNman
Copy link

FERNman commented Jan 25, 2020

Hey there,

We've been using this package for quite some time now and it has improved the development experience at our company a lot!
However, as our project gets larger and larger, one thing that bothers us is the performance of unit tests. Our project is medium-sized with about 75 libraries and 3 apps (Angular and NestJS) and about 350 test suites.

It takes about 12 minutes on my machine (windows, very decent specs) to run all tests using nx affected:test --all --parallel. In our CI environment (Linux) and on laptops this can take quite a bit longer, around 20 to 25 minutes. By using nx affected, we can run the unit tests for PRs in a few minutes, but for bigger PRs with more files, the time it takes to run the tests increases rapidly.

NOTE: If this is something nobody else is experiencing, feel free to close this issue right away (please comment on how you achieved good test performance).

After looking into a lot of issues, we're pretty confident that ts-jest and typescript compilation time are the main contributing factors. It takes between 10-20 seconds for each library just to get jest to start running the tests. Once jest is running, the tests are fast.
Using the fix from #1299 improved performance quite a bit (around 20-30%), but this still is slow for this amount of tests. kulshekhar/ts-jest#1115 is probably part of the reason.

We are still very confident that re-compiling all necessary files for each library individually is not a good approach for unit testing.
That's why we decided to try to run all our unit tests together. We altered the global jest.config.js and are now able to run all unit tests with the Jest CLI in a single test run.
The time it takes to run all those tests is about 60 seconds on my pc and 90-100 seconds on a laptop. I did not yet test the speed in our CI environment.
That's a massive speedup (over 10x faster), but could still be improved upon, and that's what this issue is actually about.

My suggestion is to alter nx affected:test to collect all the projects that have changed, then divide them by their needed jest setup (jest-preset-angular with JSDOM, just JSDOM or Node) and run those tests at once. This could be done by adding an entry to the testMatch array in the jest config dynamically for every library and would result in a maximum of three test runs.

EDIT:
After investigating a bit further, Jest supports a config entry called projects since Jest v20. This is a perfect fit for this problem.
If the global jest config was altered for every new project generated (just like tsconfig.json and nx.json), it would be possible to run jest from the root directly through the CLI. This would also allow configuring each project individually as needed through its own jest.config.js and to run ng test <library> just like now.
In addition, I would remove the hack to alter the jest config in a script and instead change the schematic that is used to generate a new angular library to add the config needed for 'jest-preset-angular' to the libraries' jest.config.js.

nx affected:test would then be even easier to implement then in my previous suggestion. Just replace the projects entry in the global jest config by an array of changed project paths.

Pros:

  • Unit tests run a lot faster
  • Jest config is cleaner and more exposed
  • Jest CLI can be used to run unit tests
  • VSCode-Jest can be used to run unit tests
  • The coverage report shows the total coverage

Cons:

  • Parsing failed projects is probably a bit more difficult to implement

Expected Behavior

Be able to run unit tests in 1-2 minutes for small and medium-sized projects and even less for PRs.

Current Behavior

It takes about 12 minutes on my machine (windows, very decent specs) to run all tests. In our CI environment (Linux) and on laptops this can take quite a bit longer, around 20 to 25 minutes.

Context

Please provide any relevant information about your setup:

@nrwl/angular : 8.11.0
@nrwl/cli : 8.11.0
@nrwl/cypress : 8.11.0
@nrwl/jest : 8.11.0
@nrwl/linter : 8.11.0
@nrwl/nest : 8.11.0
@nrwl/node : 8.11.0
@nrwl/tao : 8.11.0
@nrwl/workspace : 8.11.0
typescript : 3.4.5

jest-preset-angular: 7.1.1
jest: 24.8.0
ts-jest: 24.3.0

@vsavkin vsavkin added scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx type: feature labels Jan 26, 2020
@vsavkin
Copy link
Member

vsavkin commented Jan 26, 2020

Thank you for submitting the issue.

We definitely need to explore projects or TS project references to speed this use case up.

There are several things we'd like to preserve:

  • We only test what is affected.
  • If projects A and B are affected, tests for A should not in any way affected project B. So tests are isolated.
  • We should be able to distribute tests across multiple machines.

Running Jest multiple times is the easiest way to achieve that. If the main cost of the test run is TypeScript compilation, TS project references might be a better solution because they preserve the properties. But we will explore using projects.

@luchsamapparat
Copy link
Contributor

luchsamapparat commented Mar 31, 2020

I'd just like to confirm your observations not only for Jest but also for those projects, that use Karma/Jasmine. In the last months, I worked on several projects using Nx Workspace both with Jest and Karma/Jasmine. While the actual tests for each library are being executed within a few seconds, the startup time for each library is several times longer than the execution itself.

I'd like to use libraries even more than I currently do to structure my projects and set up strict boundaries (along the lines of @angular-architects/ddd), but the price for additional (smaller) libraries is just too high at the moment.

@vsavkin are there plans to evolve nx-enforce-module-boundaries to define dependency rules within a library? I'm thinking of something like what Spring is currently doing with Moduliths.

@luchsamapparat
Copy link
Contributor

luchsamapparat commented Mar 31, 2020

That's why we decided to try to run all our unit tests together. We altered the global jest.config.js and are now able to run all unit tests with the Jest CLI in a single test run.

@FERNman could you provide an example for this configuration?

@FERNman
Copy link
Author

FERNman commented Mar 31, 2020

Hi @luchsamapparat,

We now have two test configs:

jest-preset.js, which is used as a preset by each jest config in a library. This one is pretty much just the original jest.config.js generated by the CLI.

And then we have a global jest.config.js, which is needed in order to be able to use the jest CLI (and for example vscode-jest.

module.exports = {
  testPathIgnorePatterns: ['<rootDir>'], // We only want to run tests in the projects
  projects: [
     '<rootDir>/libs/library-one',
     '<rootDir>/libs/library-two',
     '<rootDir>/apps/app-one',
  ]
}

One more thing that's important to know is that in each of the angular libraries we are adding all fields needed by jest-preset-angular manually after creating the library. These fields are usually added by the NX CLI when running the tests. This is not necessary when not using Angular.

Example library config:

module.exports = {
  name: 'some-lib',
  preset: '../../jest-preset.js',
  coverageDirectory: '../../coverage/libs/some-lib',
  globals: {
    'ts-jest': {
      stringifyContentPathRegex: '\\.html$',
      astTransformers: ['jest-preset-angular/build/InlineFilesTransformer', 'jest-preset-angular/build/StripStylesTransformer']
    }
  },
  setupFilesAfterEnv: ['<rootDir>/src/test-setup.ts'],
  snapshotSerializers: [
    'jest-preset-angular/build/AngularNoNgAttributesSnapshotSerializer.js',
    'jest-preset-angular/build/AngularSnapshotSerializer.js',
    'jest-preset-angular/build/HTMLCommentSerializer.js'
  ]
};

Another approach if you don't want/need to use the Jest CLI is to create a custom script similar to nx affected:test calling the jest CLI in code. This way, no separate jest-preset.js is needed and some advanced things like only testing affected projects can be included. If you need help regarding such a script, feel free to ask.

Disclaimer
After using this approach for more than two months now, I can say it has its downsides as well. Jest is leaking a lot of memory (at least in our setup), which leads to the unit tests taking up a lot of RAM. To prevent this, we are now executing our tests in batches of 100 tests suites per batch.

If you can live without type checking when running tests, I suggest using isolatedModules. This greatly improves test speed and drastically reduces memory usage.

@luchsamapparat
Copy link
Contributor

luchsamapparat commented Apr 3, 2020

Thanks @FERNman for your detailed response. I didn't have time to test it on one of my own apps, but I recently updated Angular as well as Nx to the latest version and noticed a huge performance improvement when running all tests (at least on consecutive executions)

I suspect that this is because of Nx' Computation Caching which is enabled now by default:
https://blog.nrwl.io/computation-caching-out-of-the-box-revamped-docs-community-plugins-and-more-in-nx-9-2-e97801116e02#146b

@FERNman
Copy link
Author

FERNman commented Apr 3, 2020

@luchsamapparat I can completely confirm what you are describing, we are also thinking about going back to the plain nx affected:test script.

@ahnpnl
Copy link
Contributor

ahnpnl commented Apr 20, 2020

Thank you for submitting the issue.

We definitely need to explore projects or TS project references to speed this use case up.

There are several things we'd like to preserve:

  • We only test what is affected.
  • If projects A and B are affected, tests for A should not in any way affected project B. So tests are isolated.
  • We should be able to distribute tests across multiple machines.

Running Jest multiple times is the easiest way to achieve that. If the main cost of the test run is TypeScript compilation, TS project references might be a better solution because they preserve the properties. But we will explore using projects.

@vsavkin FYI the next version of ts-jest will support projectReferences, see kulshekhar/ts-jest#1541 and kulshekhar/ts-jest#1527

@ahnpnl
Copy link
Contributor

ahnpnl commented Apr 21, 2020

FYI: kulshekhar/ts-jest#1549 will be in alpha version of ts-jest (possibly today). Anyone who is using ts-jest please help to test the alpha version and give us some feedbacks for kulshekhar/ts-jest#1115

@dogmatico
Copy link

Thanks @FERNman for your detailed response. I didn't have time to test it on one of my own apps, but I recently updated Angular as well as Nx to the latest version and noticed a huge performance improvement when running all tests (at least on consecutive executions)

I suspect that this is because of Nx' Computation Caching which is enabled now by default:
https://blog.nrwl.io/computation-caching-out-of-the-box-revamped-docs-community-plugins-and-more-in-nx-9-2-e97801116e02#146b

We observe huge performance hits if both tasksRunnerOptions' cache and parallel are active when testing: like 10x time to complete compared to running in parallel with cache disabled. Maybe the I/O operations required to keep the cache functionality are a bottleneck

@luchsamapparat
Copy link
Contributor

I started running the tests with the --run-in-band flag a while ago. At least under Windows that was way faster. However, this was already the case before caching was introduced.

@cvocvo
Copy link

cvocvo commented May 27, 2020

Interestingly we're just rolling out nx on our monorepo project comprising multiple apps and libraries. We've been tearing our hair out trying to figure out the reason why this is slower as you described. In some instances I'm seeing nx affected:test take anywhere from 30% longer to 3-4 times longer than simply jest --noStackTrace. When we run jest --noStackTrace, I see processor utilization hit 100% across our threadripper PC's, but nx affected:test (even with an extra large number for parallel processing such as nx affected:test --parallel --maxParallel=12 --runInBand --maxWorkers=100 yields slower results and the CPU only hits 50% utilization total and then trails off between 10-20% for the remainder of the tests running.

Has anyone figured out a solution? By design it it seems like nx affected:test should perform much better than jest --noStackTrace.

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: testing tools Issues related to Cypress / Jest / Playwright / Vitest support in Nx type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants