-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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(appium): prepare setup subcommand as shortcut for drivers/plugins installation #20102
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.
like this idea!
packages/appium/lib/cli/parser.js
Outdated
static _addSetupToParser(subParser) { | ||
const setupParser = subParser.add_parser('setup', { | ||
add_help: true, | ||
help: 'Install latest uiautomator2 and xcuitest driver if no APPIUM_HOME was empty', |
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 does if no APPIUM_HOME was empty
mean? Do you mean to say unless APPIUM_HOME isn't empty
, of if APPIUM_HOME is empty
?
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.
yea, that was my bad. Right now is:
Install latest preset drivers and plugins if APPIUM_HOME has no drivers and plugins
@@ -276,6 +278,9 @@ async function init(args) { | |||
pluginConfig, | |||
appiumHome, | |||
}); | |||
} else if (isSetupCommandArgs(preConfigArgs)) { | |||
await setupCommand(appiumHome, preConfigArgs, driverConfig, pluginConfig); | |||
return /** @type {InitResult<Cmd>} */ ({}); |
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.
maybe the return value should be the merge of the return values of all the install commands, in case this is run by script?
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.
Meaning by appium setup
? The current behavior is if the command succeeds, the existing status will be 0
. Otherwise non-zero, so probably users can catch the error if an error occurred.
current e.g.
% ./node_modules/.bin/appium setup
dbug Appium No manifest file found at /Users/kazu/.appium/node_modules/.cache/appium/extensions.yaml; creating
dbug Appium Discovering newly installed extensions...
✔ Checking if 'appium-uiautomator2-driver' is compatible
✔ Installing 'uiautomator2' using NPM install spec 'appium-uiautomator2-driver'
ℹ Driver uiautomator2@3.5.1 successfully installed
- automationName: UiAutomator2
- platformNames: ["Android"]
✔ Checking if 'appium-xcuitest-driver' is compatible
✔ Installing 'xcuitest' using NPM install spec 'appium-xcuitest-driver'
ℹ Driver xcuitest@7.15.3 successfully installed
- automationName: XCUITest
- platformNames: ["iOS","tvOS"]
✔ Checking if '@appium/images-plugin' is compatible
✔ Installing 'images' using NPM install spec '@appium/images-plugin'
ℹ Plugin images@3.0.6 successfully installed
% echo $?
0
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 meant, in --json
mode it can return a nice json object which is the merge of all the install operations.
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.
-> #20130 I'll check and update the --json
mode
Removed uninstallation/reset stuff to not include such discussion in this PR. it will be in #20117 |
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.
looks good to me. Just not quite sure about which items should be included into presets and which not
Proposed changes
#20097
This idea is that the
setup
is a shortcut of multiple preset drivers/plugins installation.e.g.
help
This PR does not include documentation update. I'll prepare documentation when this change will be released to reflect this
setup
command.Types of changes
What types of changes does your code introduce to Appium?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...