Skip to content

chore: move all packages away from jest#156

Merged
sjwilczynski merged 16 commits intomicrosoft:mainfrom
sjwilczynski:sjwilczy/moveToVitest
Apr 17, 2025
Merged

chore: move all packages away from jest#156
sjwilczynski merged 16 commits intomicrosoft:mainfrom
sjwilczynski:sjwilczy/moveToVitest

Conversation

@sjwilczynski
Copy link
Copy Markdown
Contributor

@sjwilczynski sjwilczynski commented Apr 7, 2025

This PR is logical continuation of #150 - we in scope of this PR migrate away from using jest in the repository completely. Here are some performance numbers between jest and vitest, for script doing the benchmark check the gist. Pasting here for convenience:

Vitest:

Package Average Time (s) Std Deviation (s) Min Time (s) Max Time (s)
examples 18.96 5.26 16.84 38.57
examples (with swc) 16.10 2.46 14.70 25.22
nova-graphql-compiler 2.23 0.24 2.13 3.15
nova-react 4.40 0.10 4.33 4.77
nova-react-test-utils 2.61 0.16 2.54 3.20

Jest (before moving most tests in examples to stories):

Package Average Time (s) Std Deviation (s) Min Time (s) Max Time (s)
examples 19.53 6.64 16.15 44.31
nova-graphql-compiler 7.66 0.64 7.39 10.04
nova-react 16.14 1.62 15.38 22.15
nova-react-test-utils 6.92 0.57 6.60 8.97

Notes:

  • the SWC version is the one currently in the PR.
  • the max time refers to case when we just checked out the repo, ran yarn and immediately after started running tests

We can see 3-4 times fast perf on smaller packages and ~20% improvement faster in bigger much (with much higher amount of tests, because now all stories are included) and ~45% faster cold start, on newly checked out repo.

Some additional benefits:

  • we use Vitest browser mode, which means tests are run in headless browser instead of poor simulation like JSDOM in jest, no more cases of something working in browser but not when run in Jest
  • test verb no longer depends on build or types as vitest doesn't have limitations like ts-jest
  • Vitest has proper support for ESM so we no longer need to worry about complex and counterintuitive jest transforms
  • We share a lot of test config between unit tests and stories
  • We move away from testing-library in favor of vitest-browser-react with similar but a bit nicer API with playwright like capabilities

Some additional changes:

  • we now have share vitest config, the other packages extend or reuse
  • we simplified tsconfig to have shared options in root and only override on each package level
  • BREAKING CHANGE: in @nova/react-test-utils we make a breaking change as previously we hardcoded bubble and trigger to be instances of jest mock for variant of util exposed to unit tests. As we now want to support both, instead of having default mock, we just give an option to pass mock from your testing framework of choice. In 1JS we don't depend on that behavior but in semantic versioning this needs to be a major bump
  • We managed to get rid of vite-plugin-commonjs (cc @ansemb) by first replacing require statement when loading schema and then by removing vite-plugin-relay and related Babel plugin by replacing it with @swc/plugin-relay to align more with tools used in our outside repositories.

@sjwilczynski sjwilczynski force-pushed the sjwilczy/moveToVitest branch from cc0957e to c561cda Compare April 12, 2025 13:12
@sjwilczynski sjwilczynski changed the title [wip] move all tests to vitest chore: move all packages away from jest Apr 12, 2025
@sjwilczynski sjwilczynski marked this pull request as ready for review April 12, 2025 19:27
Copy link
Copy Markdown
Member

@Markionium Markionium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff! 🥳

argv,
parallel,
} from "just-scripts";
import { tscTask, esbuildTask, eslintTask, parallel } from "just-scripts";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any opinions on if we need/want a just-scripts task for vitest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need it. In general I am not the biggest fan of this abstraction layer especially that unless configured, it doesn't allow passing more arguments to a command. I personally find it confusing and it doesn't bring value, sharing config is anyway done through just by extending default config in each package.

However, if someone could show me some clear benefits, I am open to changing my mind

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with that, I phrased it as a question to not anchor the comment.

@@ -1,7 +1,14 @@
/**
* @vitest-environment node
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own curiosity and anyone reading next :)

https://vitest.dev/guide/environment.html#environments-for-specific-files

@sjwilczynski sjwilczynski merged commit 0d00c42 into microsoft:main Apr 17, 2025
2 checks passed
@sjwilczynski sjwilczynski deleted the sjwilczy/moveToVitest branch April 17, 2025 06:57
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

Successfully merging this pull request may close these issues.

2 participants