Switch test runner from Jest to Vitest #1609
Conversation
🦋 Changeset detectedLatest commit: 5aacf14 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Instead of importing from vitest, you can use globals. |
this way you pollute the global namespace with vitest types though |
|
Which is what jest did before, so it's not too much of an issue, is it? |
|
I am happy to switch to using globals if we can come to a consensus on it! The globals were used before, yes. Vitest by default has globals off, but they can be enabled in the config easily. I will be resolving the merge conflicts related to changes upstream shortly. |
|
I'm for switching them on to keep the impact of this PR low. If at some point we should decide to remove globals again, the PR for that will be simpler. |
|
That sounds like a great idea for me. I've went ahead and pushed out commits to enable globals and removed all of the vitest imports. This PR is a bit more manageable now. |
|
Hmm, I see my changes for Vitest globals broke type testing. I will take a look. Thanks for letting the check run! |
|
This has been fixed - I believe the checks should pass this time around. |
|
I added the @vitest/coverage-istanbul package and ran the coverage command locally, and it seems to be working now. I picked istanbul to mirror the same coverage provider Jest uses by default, and made sure to enable the lcov reporter. Hopefully, I've gotten everything right this time, haha. EDIT: The code coverage percentage is really off in Vitest compared to the results from Jest. Will take a look later and see why that is. |
|
Good news! Switching to c8 instead of instanbul has actually fixed the coverage issues I was seeing! We are getting 95.79% |
|
Just switching from cjs to esm changes the coverage slightly, so that looks okay to me. |
Pull Request Test Coverage Report for Build 4522497044Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Thank you so much. Awesome work. |
|
Hmm.. merging into next branch all the solid-js/web tests are failing. Any ideas? I guess it pulls in babel-preset-solid externally. So when I'm working on new stuff this is sort of awkward. EDIT: It looks to be related to the external source test actually. |
|
I spent some time playing with this issue this morning and I think it might have something to do with vite-plugin-solid bringing in babel-preset-solid: I haven't identified a solution yet, but later today I'm going to try to see if I can get rid of that external dependency somehow and just have it use the babel-preset-solid package in the repository to take care of things. |
|
I'm at a bit of a loss with this issue at the moment. I tried as a test to add configuration to the package.json to override the babel-preset-solid dependency with the one from the workspace, like so: And yet this didn't actually do anything to help resolve this issue. Still doing a bit of digging here, but just wanted to give a quick update. Such an odd issue. |
|
Amazing thank you. |
|
This is fixed in this PR: #1654 Let me know if you have any questions! |
Summary
This PR switches the test runner from Jest to Vitest. By implementing this change, I was hoping to see some speed improvements in the time it takes to run tests, especially when it comes to tests running in CI. On my MacBook Pro with an M2 Max, I saw a bit of an improvement running the following command before and after this change:
pnpm turbo run test --forceThis is one of my first PRs to an open source project, so please let me know if there is anything I can do or any changes I can make to help with this. I know it's a bit much to digest due to a lot of the changes seen here, and there are some interesting things I did that we might need to work around, like the usage of vite-plugin-solid as a dependency. I am open to making any changes as needed to help increase the viability of this PR. There are a couple of things we can tweak when it comes to running tests in CI as well, especially the threading and isolation settings in Vitest to see if it would speed up a CI run compared to my own computer locally, where it didn't seem to help.
How did you test this change?
I tested this change by making sure all of the tests passed and that Solid still built as expected. I compared the distributable files before and after my changes and made sure that they matched exactly too, to make sure nothing weird was affected when I was doing this migration.
SHA sums for distributable files before changes
SHA sums for distributable files after changes
Expand me to see shell output from running the tests