chore: migrate from jest to vitest #4396
Conversation
|
Thanks a lot for the contribution! To be clear: is this PR a work-in-progress, or is it ready for review? |
|
@nolanlawson its ready for review. I think tests not migrated here will require deeper refactoring. The jest tests are failing. I'm not sure if it's better to exclude the migrated ones from jest or to keep the the old test files alongside. |
|
I think ideally we'd like to do a clean break from Jest. It would be extra maintenance to have some tests in Jest and others in Vitest. |
|
@nolanlawson I'll give it a shot this weekend at fully migrating from jest. |
|
Current status: 34 out of 41 suites migrated. 1169 out of 1321 tests running. Jest (master branch): Vitest |
| @@ -77,7 +77,7 @@ describe('customRendererConfig normalization', () => { | |||
| }, | |||
| }) | |||
| ).toThrowErrorMatchingInlineSnapshot( | |||
| `"LWC1150: customRendererConfig contains duplicate entry for use element tag"` | |||
| `[Error: LWC1150: customRendererConfig contains duplicate entry for use element tag]` | |||
There was a problem hiding this comment.
It's much easier to change the test here than make it work with vitest hopefully this is fine.
| @@ -95,7 +95,7 @@ describe('customRendererConfig normalization', () => { | |||
| }, | |||
| }) | |||
| ).toThrowErrorMatchingInlineSnapshot( | |||
| `"LWC1151: customRendererConfig should not contain a custom element tag, but found lightning-input"` | |||
| `[Error: LWC1151: customRendererConfig should not contain a custom element tag, but found lightning-input]` | |||
|
Figured out the issue with coverage tests – you actually can't define them in
|
|
/nucleus test |
|
/nucleus test |
|
/nucleus test |
|
Both Jest and Vitest report 41 test suites and 1327 tests passed. |
|
/nucleus test |
| @@ -34,21 +34,21 @@ | |||
| "prettier": "v3 requires ESM, and we use prettier in our Jest tests. Jest does not support ESM yet." | |||
| }, | |||
| "devDependencies": { | |||
| "@babel/plugin-transform-modules-commonjs": "^7.24.8", | |||
There was a problem hiding this comment.
No longer needed, only needed for Jest
| // Workaround to fix `const enum`, which is required because we use e.g. `APIFeature` from `@lwc/shared` | ||
| // See https://github.com/vitest-dev/vitest/discussions/3964 | ||
| // Using `src` also ensures that the test coverage is accurately reported | ||
| alias: Object.fromEntries( | ||
| Object.keys(pkg.dependencies ?? {}) | ||
| .filter((dep) => dep.startsWith('@lwc/')) | ||
| .map((dep) => [dep, `${dep}/src`]) | ||
| ), |
There was a problem hiding this comment.
Does it matter if we define an alias for a package that's not actually used? Could we just move this to the root config and define aliases for all @lwc/ packages?
There was a problem hiding this comment.
I tried doing this, but it messed with the lwc tests because it tries to import from dist.
I can give this another shot, though.
|
/nucleus test |
|
Thanks a bunch @cardoso! 💪 |
Details
This PR aims to migrate a few packages to vitest, starting with the easier ones.
I'll try to have 1 commit per package so it's easy to follow and choose up to which one to merge.
I'm trying not to touch jest tests at first, but some will break:
usage of jest global namespace (eg:
jest.fn()->vi.fn()) and incompatible apis (eg:jest.setTimeout(n)->vi.setConfig({ testTimeout: n })).Closes #4153
Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?