-
-
Notifications
You must be signed in to change notification settings - Fork 208
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
tweak android date format #377
Conversation
In Ruby, both formats can parse as same, so this change doesn't affect Ruby client though. # "Sat Jun 9 10:31:40 JST 2018"
Date.parse "Sat Jun 9 10:31:40 JST 2018"
=> #<Date: 2018-06-09 ((2458279j,0s,0n),+0s,2299161j)>
# "Sat Jun 09 2018 10:30:13 GMT+0900 (JST)"
Date.parse "Sat Jun 09 2018 10:30:13 GMT+0900 (JST)"
=> #<Date: 2018-06-09 ((2458279j,0s,0n),+0s,2299161j)> In JavaScript, |
The initial proposal was to be able to provide a custom format string as function argument, so we could alter this is base driver first. Also, I'd love to see the default format as iso-8601, since its parsing is for sure supported by the majority of the corresponding clients. |
yeah. |
lib/commands/general.js
Outdated
@@ -28,8 +28,8 @@ commands.doSendKeys = async function (params) { | |||
commands.getDeviceTime = async function () { | |||
log.info('Attempting to capture android device date and time'); | |||
try { | |||
// format: Sat Jun 09 2018 11:17:42 GTM+0900 (JST) | |||
let out = await this.adb.shell(['date', '\"+%a\ %b\ %d\ %Y\ %X\ GTM%z\ \(%Z\)\"']); | |||
// format: 2018-06-09T16:21:54+0900 |
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 probably makes sense to mention this in the docstring, that the resulting date string format conforms to ISO-8601 by default.
@KazuCocoa Would you like to include the optional format argument into this PR or create a new one? |
I'd like to create another PR for the option to keep change small. And I'll add docstring later 👍 |
lib/commands/general.js
Outdated
/** | ||
* Get device time | ||
* @return {String} Return value following iso-8601 format like `2018-06-09T16:21:54+0900` | ||
* @throws {Error} If the adb command doesn't return device date, error throes |
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 is enough to keep "If the adb command doesn't return a device date" part
lib/commands/general.js
Outdated
commands.getDeviceTime = async function () { | ||
log.info('Attempting to capture android device date and time'); | ||
try { | ||
let out = await this.adb.shell(['date']); | ||
let out = await this.adb.shell(['date', '+%Y-%m-%dT%T%z']); |
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.
return (await this.adb.shell(['date', '+%Y-%m-%dT%T%z'])).trim()
thanks for the review! |
addressed : appium/appium#10832
I made sure the format worked with emulator OS 4, 5, 6, 7, 8 and real device with 6.
Wha do you think?
Date
(both real devices and simulators):https://github.com/appium/appium-ios-driver/blob/17ac721c3c1cbefce28445a036daf1a50f4c6d80/lib/commands/general.js#L20
shell date
can format.