Conversation
… in separate vitest project
|
@ntatoud is attempting to deploy a commit to the 47ng Team on Vercel. A member of the Team first needs to authorize it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
franky47
left a comment
There was a problem hiding this comment.
Thank you!
We can test only in chromium for those tests, there are some things which may be useful to test in Safari (detecting the default throttle rate), but we can leave that out of scope for now.
.github/workflows/ci-cd.yml
Outdated
| - name: Install dependencies | ||
| run: pnpm install --ignore-scripts --frozen-lockfile --filter nuqs... | ||
| - name: Install Playwright Chromium | ||
| run: pnpm exec playwright install chromium --with-deps |
There was a problem hiding this comment.
note: It looks like this step fails:
pnpm exec playwright install chromium --with-deps
shell: /usr/bin/bash -e {0}
env:
FORCE_COLOR: 3
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
undefined
ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "playwright" not found
Error: Process completed with exit code 254.
Do we need to add playwright as a devDependency?
There was a problem hiding this comment.
Ah yes we do, I'ma fix that
There was a problem hiding this comment.
I added it in the pnpm e2e catalog like cypress since you intend to migrate to it anyway.
I also removed jsdom that was still hanging there
|
We have a different error now, on the install step: And when the tests run (expectedly, it doesn't find the binary): |
Ah my bad, it should have been |
|
It does, but it takes ~40s to install playwright 😭 I wonder if we could cache it, it's not recommended but surely restoring from cache would be faster (and more robust) than going through the network? Do we also need system dependencies? GHA images contain a lot of things built-in. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The build fails on Vercel but the error seems completely unrelated (unless it's a lockfile update cascading). I'll review over the weekend, thanks again! |
|
🎉 This PR is included in version 2.8.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Main goal
This PR aims to move away from JSdom for tests in a browser environment and use vitest browser mode instead.
Additional notes
On top of moving tests needing a browser environment away from jsdom, I splitted existing tests in 3 vitest projects.
browser=> Self explanatory (matches all test names*.browser.test.*)For this, I had to replace the usage of some node apis in the tests since they are not polyfilled in the browser.
unit=> For all tests that should pass in a non-browser environment. (matches all*.test.*except*.browser.test.*types=> For all type testingspackage.jsonscripts to keep consistent behavior.Pending questions:
playwright(Webkit, Firefox, Chromium) or keep only Chromium as it is right now ?