Switch test runner from Jest to Vitest #1694
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1694 +/- ##
==========================================
+ Coverage 97.11% 97.55% +0.44%
==========================================
Files 21 20 -1
Lines 831 1513 +682
Branches 84 319 +235
==========================================
+ Hits 807 1476 +669
- Misses 23 37 +14
+ Partials 1 0 -1 ☔ View full report in Codecov by Sentry. |
|
Coverage is being calculated differently it seems. Vitest uses v8 coverage by default as opposed to Jest which uses Babel with Istanbul. Since the v8 one is part of the engine that parses JS code, it might make some shortcuts, optimize some code away, thus less perceived hits with no real impact otherwise, or so I assume. Here are some more details: jestjs/jest#11188 |
|
Related #1626 |
| @@ -12,8 +13,6 @@ const index = { | |||
| uid: 'movies_test', | |||
| }; | |||
|
|
|||
| jest.setTimeout(100 * 1000); | |||
There was a problem hiding this comment.
EXPLAINER: This is replaced by a global setting in vitest.config.js testTimeout: 100_000, // 100 seconds.
tests/client.test.ts
Outdated
| @@ -1,3 +1,4 @@ | |||
| import { afterAll, expect, test, describe, beforeEach } from 'vitest'; | |||
There was a problem hiding this comment.
EXPLAINER: Vitest default and recommended way is to have explicit imports for these methods.
|
The biggest drop in coverage is because |
flevi29
changed the title
|
bors cancel |
|
bors merge |
flevi29
force-pushed
the
jest-to-vitest
branch
from
dfc8688 to
ebe4cb9
Compare
flevi29
force-pushed
the
jest-to-vitest
branch
3 times, most recently
from
8c6d851 to
ad9ced0
Compare
flevi29
force-pushed
the
jest-to-vitest
branch
from
ad9ced0 to
97bf454
Compare
flevi29
added
the
skip-changelog
curquiza
removed
the
skip-changelog
|
@flevi29 can you fix git conflicts? |
flevi29
force-pushed
the
jest-to-vitest
branch
from
4a3a69c to
47096c4
Compare
|
@curquiza fixed. |
.github/workflows/tests.yml
Outdated
| @@ -45,6 +45,7 @@ jobs: | |||
| cache: 'yarn' | |||
| - name: Install dependencies | |||
| run: yarn --dev | |||
| # @TODO: Need to clean up test scripts, they're confusing, and base test is expected to run coverage | |||
There was a problem hiding this comment.
Can you remove this and open an issue instead?
|
@flevi29 sorry, git conflicts again 🙈 |
flevi29
force-pushed
the
jest-to-vitest
branch
from
47096c4 to
0500f2c
Compare
|
@curquiza fixed again! |
Pull Request
Why?
I am also getting rid of
jsdomtests, we were already skipping Node.js tests when usingjsdomand everything else is based on web standards so there isn't really much of a benefit tojsdom. This halves the time tests are running for. I am also planning to parallelize some tests by using multiple indexes on Meilisearch to further speed up tests.PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!