-
Notifications
You must be signed in to change notification settings - Fork 3
Set-up to run tests with vitest browser mode #1947
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1947 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 70 70
Lines 1316 1316
Branches 486 486
=========================================
Hits 1316 1316 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7c733ce
to
6850f6f
Compare
console.error(pendingErrorNotice); | ||
throw pendingError; | ||
} | ||
}); |
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.
This throws a lo of false positives when used with vitest, and I reckon it is no longer needed with karma either, so I'm removing it.
6850f6f
to
063dd1f
Compare
test/bootstrap.js
Outdated
@@ -12,31 +12,10 @@ sinon.assert.expose(assert, { prefix: null }); | |||
// karma-sinon plugins | |||
globalThis.assert = assert; | |||
globalThis.sinon = sinon; | |||
globalThis.context = globalThis.context ?? globalThis.describe; |
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.
Vitest does not have a global context
, so this simply creates an alias from describe
, which is basically what mocha does.
f9059fd
to
20fac69
Compare
ad961d9
to
2e57c32
Compare
2e57c32
to
5cca6d6
Compare
b1b9cf7
to
ffac4ac
Compare
721cdbf
to
5cdb120
Compare
yarn.lock
Outdated
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.
Most removals here are due to the execution of yarn dedupe
, which was needed to solve a weird error when trying to generate code coverage reports.
By googling the error I found many people recommending the removal of the yarn.lock
file, which did work, but introduced many more changes.
Based on that I assumed the issue was in the fact that istanbul was a dependency of multiple other dependencies, which was causing different conflicting versions to be installed.
5cdb120
to
ef06f47
Compare
Makefile
Outdated
@@ -65,4 +65,5 @@ build: node_modules/.uptodate | |||
|
|||
node_modules/.uptodate: package.json yarn.lock | |||
yarn install | |||
yarn playwright install |
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.
This has a low impact when the binaries are already up to date. In my machine it's around 0.6s, which is acceptable if we only run it when yarn install
is also needed.
e8eea74
to
24452f0
Compare
24452f0
to
8c11ffc
Compare
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.
Karma config file used to be in the src
dir, but I have put this in the root dir. If there was a good reason for the old location, I think it should be possible to move this there.
555afbd
to
1cfeef0
Compare
I see a message about
|
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.
The playwright install command can be optimized by installing only the browsers we are going to use, especially in CI. Otherwise this looks good.
1cfeef0
to
0e6f9c2
Compare
This is a POC which aims to replace karma as the test runner with vitest, using browser mode + playwright.
Karma has been deprecated and is no longer actively maintained, so we need to eventually find a replacement that allows us to continue writing our tests in the same way, and running a pre-bundled tests bundle.
This PR:
Todo
yarn.lock
is deleted and dependencies are cleanly installed.yarn.lock
and install dependencies again. Then coverage would be generated properly.I need to find what's the exact dependency conflict that causes this. My theory is that there are a bunch of instanbul instances that are conflicting, and may be fixed by simply uninstalling all karma stuff, including its istanbul plugin.EDIT: I just verified running
yarn dedupe
also fixes this issue. I assume it's because there are multiple instances of istanbul in probably incompatible versions, because multiple dependencies depend on it themeselves.yarn playwright install
every time we runyarn install
. That should keep playwright binaries up to date.Running that command when no updates are needed has a very low impact compared to the overall execution time of
yarn install
.--grep <some_pattern>
(Add support to run tests with vitest frontend-build#722)--live
is passed. (Add support to run tests with vitest frontend-build#722)