-
-
Notifications
You must be signed in to change notification settings - Fork 169
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 script param in setDeviceSysLocaleViaSettingApp #372
Conversation
lib/tools/adb-commands.js
Outdated
*/ | ||
methods.setDeviceSysLocaleViaSettingApp = async function (language, country) { | ||
await this.shell(['am', 'broadcast', '-a', LOCALE_SETTING_ACTION, | ||
methods.setDeviceSysLocaleViaSettingApp = async function (language, country, 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.
it might be more readable if we set script to null
by default (script = null
)
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 considered the way, too. I'll follow the way.
lib/tools/adb-commands.js
Outdated
'-n', LOCALE_SETTING_RECEIVER, | ||
'--es', 'lang', language.toLowerCase(), | ||
'--es', 'country', country.toUpperCase()]); | ||
'--es', 'country', country.toUpperCase() | ||
].concat(_.isString(script) ? ['--es', 'script', 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.
I'd rather prefer to use this style:
if (script) {
params.push('--es', 'script', script);
}
I'll publish a new version of appium settings soon |
ca6515d is a commit to run e2e test. |
gulpfile.js
Outdated
|
||
boilerplate({ | ||
build: 'appium-adb', | ||
jscs: false, | ||
e2eTest: { android: true } | ||
testTimeout: 120000, |
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 it be a global setting?
test/functional/setup.js
Outdated
}; | ||
|
||
const avdName = process.env.ANDROID_AVD || 'NEXUS_S_18_X86'; | ||
const platformVersion = process.env.PLATFORM_VERSION || '4.3'; |
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.
will back to ^
package.json
Outdated
@@ -10,7 +10,7 @@ | |||
"watch": "gulp watch", | |||
"build": "gulp transpile", | |||
"mocha": "mocha", | |||
"e2e-test": "gulp e2e-test", | |||
"e2e-test": "gulp transpile && mocha --timeout 600000 build/test/functional/", |
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.
This requires that mocha
be globally installed. It would be better to be
"e2e-test": "gulp transpile && npm run mocha -- -t 600000 --recursive build/test/functional/",
// zh-hans-cn : zh-cn | ||
const localeCode = script ? `${language}-${script.toLowerCase()}-${country}` : `${language}-${country}`; | ||
|
||
if (localeCode === curLocale) { |
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.
how about writing a small log message here?
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.
👍
@mykola-mokhnach |
related to: appium/io.appium.settings#23
After this PR, we can set
script
in setting locale.