-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
test: split fixtures for browser test #4878
Conversation
✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.
|
8e21772
to
f1be906
Compare
const result = await execa('npx', [ | ||
'vitest', | ||
'run', |
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.
Was this using npx
before? Signal terminals are buggy when scripts are launched via npx
. I can't find the link for the issue or older references for these issues right now. Consider using startVitest
instead.
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.
pnpm exec
should also work correctly.
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.
Currently browser mode testing is written with execa
+ npx vitest
, so I was keeping the same structure from:
vitest/test/browser/specs/run-vitest.mjs
Lines 7 to 12 in c946b1c
const argv = ['vitest', '--run', `--browser.name=${browser}`] | |
if (browser !== 'safari') | |
argv.push('--browser.headless') | |
const { stderr, stdout } = await execa('npx', argv.concat(moreArgs), { |
The test runner used here is node --test
, so test utils like runVitest
are not available and runner.test.mjs
would probably be easier to written with execa
, so I'll keep that there for now.
This small test custom-base.test.mjs
should be fine with startVitest
API, so I'll rewrite that. Thanks for the suggestion!
Btw, I found things are already flaky without update-snapshot.test.mjs
from #4867, so I'm now testing with only runner.test.mjs
and custom-base.test.mjs
here #4879.
Closing this in favor of the other one |
Description
Base branch: #4867 (new change is only the last commit)
Testing to see if splitting the custom base test would affect test failure on CI.
I also rewrote an assertion to be based on stdout/stderr, so it might be easier to debug.
I'll retry CI for a few times by force-pushing the same commit.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.