- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2
FEC-259: Vitest PoC #2 #951
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
Conversation
| 
 | 
| resolve: { | ||
| alias: { | ||
| '@/core': path.resolve(__dirname, './standalone/src/core'), | ||
| '@/core/test-utils': path.resolve( | ||
| __dirname, | ||
| './standalone/src/core/test-utils' | ||
| ), | ||
| '@/graphql-types': path.resolve( | ||
| __dirname, | ||
| './standalone/src/graphql-types' | ||
| ), | ||
| '@/models': path.resolve(__dirname, './standalone/src/models'), | ||
| '@/utils': path.resolve(__dirname, './standalone/src/utils'), | ||
| }, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If comparing to the original PoC, the release of Vitest 3.x became a little more strict about path resolution & now requires explicit aliases.
| "devDependencies": { | ||
| "@vitest/coverage-v8": "^3.2.4", | ||
| "@vitest/ui": "^3.2.4", | ||
| "vitest": "^3.2.4" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if comparing to the original PoC, devDependency [email protected] was causing several issues and determined to be outdated/incompatible at this point. No longer needed.
| // Add custom stringFromArray matcher | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (expect as any).stringFromArray = (allowed: string[]) => ({ | ||
| asymmetricMatch: (value: unknown) => | ||
| typeof value === 'string' && allowed.includes(value), | ||
| getExpectedType: () => 'string', | ||
| }); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| targets: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| addressKey: expect.any(String), | ||
| quantity: expect.any(Number), | ||
| shippingMethodKey: null, | ||
| }), | ||
| ]), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full disclosure, I do not know why this wasn't needed in the original PoC, or if vitest just became a little more strict since then to require us to explicitly define the expected structure.
| }, | ||
| maxConcurrency: os.cpus().length, | ||
| // vmthreads for better isolation | ||
| pool: 'vmThreads', | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vmThreads reads as though it slows down the run, but it actually increases it in our case.
unclear if this is a viable option
Improving Vitest performance
| Based on the learnings of this PoC we decided to not migrate to vitest. | 
This PR revisits introducing vitest into our stack.
Please see relevent comments in the applicable files.
Refactored original PoC to align with updated dependencies & monorepo changes since initial spike.
References:
Overall status: The Vitest runner is allegedly faster than Jest. Where the actual tests do run faster, the overall process takes almost 2x as long. This exercise is to:
1.) Find out why
2.) How is Jest handling the task that takes longer for Vitest.
3.) How to sidestep this issue.
Of importance - Isolation must be
true. Though it does not impact this particular repo, it's mandatory for mc-fe, so the idea here is to keep that variable constant.Notes: Stats taken from 5a48e9e (includes vmThreads & file optimization)
Known - Collection time (specifically tied to presets) determined to be the most resource intensive.
jest - Time ~18s
vitest: Duration 10.69s (transform 6.65s, setup 506ms, collect 80.99s, tests 1.97s, environment 256ms, prepare 9.25s)
Collection is an aggregate CPU time across all threads. From this example, with 10 CPUs, this is about ~8.1s of work per CPU (~81% of the total work).
What happens during the collection phase:
Why are dependency graphs thought to be the culprit?
Where the current commit stands (how we got from ~27s to ~10s):
1.) Excludes preset implementation files
**/presets/**/*.ts- Excludes all .ts files in preset directories.preset.ts → @/core → @jackfranklin/test-data-bot → @faker-js/faker)2.) Re-includes preset spec files
(Note that by default, vitest includes/excludes certain files. Once you start to customize it though, it overrides those).
Why is this working so far? Now during the collection phase:
TL;DR - Functionality is preserved: All tests still run, but the collect phase is much faster. The preset files don't need to be processed during the collection phase - they only need to be available when the spec files actually run the tests.
Eg Before updated config when vitest processes all files:
Eg After updated config when vitest processes only spec files:
A visual example of a single dependency graph (whomever takes this over should check it out, its pretty hypnotic):

What is Jest doing that it handles this differently? read but not confirmed.