-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(cli): parse --browser=<name>
correctly
#5179
fix(cli): parse --browser=<name>
correctly
#5179
Conversation
✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.
|
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
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.
I wonder how these case are handled now (and also before).
--browser=false
--browser false
--browser some-file.test.ts
Can you include these in tests?
75be929
to
fa65648
Compare
Added test cases for the |
Just to clarify, I wasn't saying to support something like I would imagine it would be tricky to support that, so it's probably okay to treat it as |
Looks like it doesn't work with the latest release either. Let's not do changes related to that for now. I'll revert that test case. $ vitest --browser app.spec.ts
DEV v1.2.2 /x/y/z/browser-simple
Browser runner started at http://localhost:5173/
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: [] Browser "app.spec.ts" is not supported by the browser provider "playwright". Supported browsers: firefox, webkit, chromium. |
fa65648
to
4e6ba96
Compare
Maybe revert |
It was clearly a bug, and one of the reasons we are making changes to the CLI parsing is to fix those edge cases. |
Description
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:
.