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

Add support to upload app to app-automate. #3573

Merged
merged 9 commits into from Feb 9, 2023

Conversation

garg3133
Copy link
Member

To test an app on BrowserStack app-automate, we first need to upload that application to BrowserStack using BrowserStack's REST API: https://www.browserstack.com/docs/app-automate/api-reference/appium/apps#upload-an-app

The upload can be done either from a public URL or using a file from the local filesystem. This PR allows users to set appUploadPath or appUploadUrl as desired capability if they wish to upload the app they wish to test to BrowserStack.

Additionally, if they have already set appium:app desired capability to some BrowserStack URL (starts with bs://), it will be overwritten with the new URL. And if they have set appium:app to something else other than BrowserStack URL, that will be considered as the custom id while uploading the application, which then can be used as it is to run tests against the uploaded app.

A few test runs:

Without custom id:
image

With custom id:
image

@beatfactor
Copy link
Member

can you fix the "Using: undefined()" bit?

});

if (response.error) {
Logger.error(response.error);
Copy link
Member

Choose a reason for hiding this comment

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

We need a more descriptive error message here, like "Uploading the app the BrowSerstack has failed because of: ...". And we can also add some help text and a link for more info (if applicable), using the actionable error messages framework. Same goes for the error logging in the catch() block

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

image

Copy link
Member

Choose a reason for hiding this comment

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

the url in the screenshot contains undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 0a4ba71. We should only show the build URL if a session with BrowserStack was successfully created.

@garg3133 garg3133 force-pushed the app-automate-support branch 6 times, most recently from c86cee3 to 29ffd1b Compare January 23, 2023 13:33
@garg3133
Copy link
Member Author

This PR is ready to be merged now.

@@ -269,7 +269,7 @@ class Transport extends BaseTransport {
const startTime = new Date();
const {host, port, start_process} = this.settings.webdriver;
const portStr = port ? `port ${port}` : 'auto-generated port';
const options = this.createSessionOptions(argv);
const options = await this.createSessionOptions(argv);
Copy link
Member

Choose a reason for hiding this comment

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

createSessionOptions doesn't return a Promise.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does now, for AppAutomate, since I am uploading the app to BrowserStack inside this method only, after creating all the session options. It also made more sense to do it inside createSessionOptions since options are not final until the app is uploaded to BrowserStack. And for the other implementations of createSessionOptions, adding await here shouldn't have any effect.

I tried doing this inside this.createDriver also (just so that I don't have to add this await) but the problem there was that the spinner (Connecting to localhost on 4723...) was overwriting the console messages related to app upload (Uploading app to BrowserStack... and App upload successful!) and the spinner is called just after calling this.createSessionOptions method.

@beatfactor beatfactor merged commit d9e2bd6 into nightwatchjs:main Feb 9, 2023
7 checks passed
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.

None yet

2 participants