Skip to content
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

Allow to use any browser with BrowserStack and Appium. #3634

Merged
merged 6 commits into from
Mar 10, 2023

Conversation

garg3133
Copy link
Member

@garg3133 garg3133 commented Mar 8, 2023

This PR allows users to use any browser with BrowserStack, as well as while creating a session with Appium server.

This is because with BrowserStack, we can send any browser name and instead of returning an error it creates a session with Chrome browser as default. And with Appium server, accepted values of browserName depends on the driver being used and hence cannot be pre-defined.

With selenium server, because we use Selenium's Builder class which supports only a few browsers, it will throw an appropriate error for other unsupported browsers instead of just some random error.

@swrdfish
Copy link
Member

swrdfish commented Mar 9, 2023

Can you add a test for this and fix the current tests.

Copy link
Member

@gravityvi gravityvi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keep the changes here instead:

if ((usingAppium || usingBrowserStack) && !browserName) {
return browserName;
}
.

we can remove the check for browserName from the condition

@garg3133
Copy link
Member Author

garg3133 commented Mar 9, 2023

@gravityvi That would work if for Appium and BrowserStack we do not want to execute this portion of code:

image

@gravityvi
Copy link
Member

@gravityvi That would work if for Appium and BrowserStack we do not want to execute this portion of code:

image

Yes. We want to achieve the same thing. According to the issue we need to allow any browser name.

@swrdfish
Copy link
Member

@gravityvi That would work if for Appium and BrowserStack we do not want to execute this portion of code:
image

Yes. We want to achieve the same thing. According to the issue we need to allow any browser name.

@garg3133 is correct, we should keep that part of the code so that command line arguments can be used.

@beatfactor beatfactor merged commit 24158bb into nightwatchjs:main Mar 10, 2023
harshit-bs pushed a commit to harshit-bs/nightwatch that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error while running test in Samsung Browser
4 participants