-
-
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
feat(browser): add support for custom browser capabilities #3758
feat(browser): add support for custom browser capabilities #3758
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.
|
f9bd487
to
d3fa570
Compare
c9b15da
to
d8b2994
Compare
Pushed changed in the spirit of the requested modifications 👍 |
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 don't see any changes to config types? And how do you pass down these new provider options?
9917ccd
to
76ba7fc
Compare
8145b58
to
dfc9894
Compare
packages/vitest/src/types/browser.ts
Outdated
@@ -2,14 +2,23 @@ import type { Awaitable } from '@vitest/utils' | |||
import type { WorkspaceProject } from '../node/workspace' | |||
import type { ApiConfig } from './config' | |||
|
|||
export interface ProviderSpecificOptions { | |||
webdriverio?: WebDriver.Capabilities | WebDriver.DesiredCapabilities | |||
playwright?: any |
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.
Couldn't find a good way here to import playwright types only
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 don't think this approach will work, because webdriverio is optional dependency. We shouldn't rely on types
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.
What would be your preferred way? Would removing added types from package.json and using unknown
for both be acceptable?
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.
Should be acceptable for now
packages/vitest/src/types/browser.ts
Outdated
@@ -2,14 +2,23 @@ import type { Awaitable } from '@vitest/utils' | |||
import type { WorkspaceProject } from '../node/workspace' | |||
import type { ApiConfig } from './config' | |||
|
|||
export interface ProviderSpecificOptions { | |||
webdriverio?: WebDriver.Capabilities | WebDriver.DesiredCapabilities | |||
playwright?: any |
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 don't think this approach will work, because webdriverio is optional dependency. We shouldn't rely on types
dfc9894
to
0ebfedd
Compare
ebf9469
to
2051f33
Compare
@@ -57,8 +60,9 @@ export class WebdriverBrowserProvider implements BrowserProvider { | |||
this.cachedBrowser = await remote({ | |||
logLevel: 'error', | |||
capabilities: { |
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.
Why are passing down only capabilities? Shouldn't it be remote
options?
Will take another look in the coming two weeks |
The aim of this PR is to add support for passing custom browser capability arguments for WebdriverIO. This was done rather quickly so improvements and suggestions are welcome, I only saw tests for playwiright.