-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Add support in App Automate for Appium #130
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds comprehensive support for Appium-based testing in the App Automate tool. It restructures the codebase to separate native execution (Espresso/XCUITest) from Appium SDK integration, enabling users to set up BrowserStack App Automate with Appium across multiple programming languages and testing frameworks.
Key Changes
- Restructures App Automate utilities into
native-execution
andappium-sdk
modules - Adds new
setupBrowserStackAppAutomateTests
tool for Appium SDK integration - Implements multi-language support for Appium (Java, C#, Python, Node.js, Ruby)
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
src/tools/appautomate.ts | Adds new setupBrowserStackAppAutomateTests tool and reorganizes imports |
src/tools/appautomate-utils/native-execution/types.ts | Moves and updates type definitions, removes APPIUM enum value |
src/tools/appautomate-utils/appium-sdk/ | New module with comprehensive Appium SDK setup utilities |
tests/tools/appautomate.test.ts | Updates import path to match new structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
`-DBROWSERSTACK_USERNAME="${process.env.BROWSERSTACK_USERNAME}" ` + | ||
`-DBROWSERSTACK_ACCESS_KEY="${process.env.BROWSERSTACK_ACCESS_KEY}" ` + |
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.
The Maven command construction uses template literals that reference process.env variables, but these will be evaluated at build time rather than runtime. This will likely result in 'undefined' values being inserted into the command string. The variables should be passed as parameters or handled differently.
Copilot uses AI. Check for mistakes.
`-DBROWSERSTACK_USERNAME="${process.env.BROWSERSTACK_USERNAME}" ` + | ||
`-DBROWSERSTACK_ACCESS_KEY="${process.env.BROWSERSTACK_ACCESS_KEY}" ` + |
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.
Same issue as in getMavenCommandForWindows - the process.env references will be evaluated at build time, not runtime, likely resulting in 'undefined' values in the Maven command.
Copilot uses AI. Check for mistakes.
No description provided.