feat: Add setting geolocation by set-simulator-location as the first method #249

KazuCocoa · 2019-12-06T05:56:06Z

https://github.com/lyft/set-simulator-location

This works on Xcode 11.2 environment on my env. (against iOS 12 and iOS 13)
Current idb does not work well on Xcode 11 env, so, let's try another way before idb try.

ok

[HTTP] --> POST /wd/hub/session/857ac20d-beac-4409-ac0a-75a37aa833c9/location
[HTTP] {"location":{"latitude":34.18583,"longitude":131.47139,"altitude":0}}
[debug] [W3C (857ac20d)] Calling AppiumDriver.setGeoLocation() with args: [{"latitude":34.18583,"longitude":131.47139,"altitude":0},"857ac20d-beac-4409-ac0a-75a37aa833c9"]
[debug] [XCUITest] Executing command 'setGeoLocation'
[debug] [W3C (857ac20d)] Responding to client with driver.setGeoLocation() result: null
[HTTP] <-- POST /wd/hub/session/857ac20d-beac-4409-ac0a-75a37aa833c9/location 200 279 ms - 14
[HTTP]

fail

[HTTP] --> POST /wd/hub/session/ae50e600-8efd-440a-8a92-89f50018ff24/location
[HTTP] {"location":{"latitude":34.18583,"longitude":131.47139,"altitude":0}}
[debug] [W3C (ae50e600)] Calling AppiumDriver.setGeoLocation() with args: [{"latitude":34.18583,"longitude":131.47139,"altitude":0},"ae50e600-8efd-440a-8a92-89f50018ff24"]
[debug] [XCUITest] Executing command 'setGeoLocation'
[iOSSim] Cannot set geolocation with set-simulator-location, because it is not installed or failed to set.
[iOSSim] Cannot set geolocation with idb, because it is not installed. Defaulting to AppleScript
[iOSSim] Cannot focus Simulator window with idb. Defaulting to AppleScript
[debug] [iOSSim] Executing UI Apple Script on Simulator with UDID 3709FCFA-C989-437A-9929-615944085CB8:
[debug] [iOSSim]       tell application "System Events"
[debug] [iOSSim]         tell process "Simulator"
[debug] [iOSSim]           set frontmost to false
[debug] [iOSSim]           set frontmost to true
[debug] [iOSSim]           click (menu item 1 where (its name contains "iPhone 11 " and its name contains "13.2")) of menu 1 of menu bar item "Window" of menu bar 1
[debug] [iOSSim]         end tell
[debug] [iOSSim]       end tell
[debug] [iOSSim]
[debug] [iOSSim]
[debug] [iOSSim]       tell application "System Events"
[debug] [iOSSim]         tell process "Simulator"
[debug] [iOSSim]           set featureName to "Custom Location"
[debug] [iOSSim]           set dstMenuItem to menu item (featureName & "…") of menu 1 of menu item "Location" of menu 1 of menu bar item "Debug" of menu bar 1
[debug] [iOSSim]           click dstMenuItem
[debug] [iOSSim]           delay 1
[debug] [iOSSim]           set value of text field 1 of window featureName to 34.18583 as string
[debug] [iOSSim]           delay 0.5
[debug] [iOSSim]           set value of text field 2 of window featureName to 131.47139 as string
[debug] [iOSSim]           delay 0.5
[debug] [iOSSim]           click button "OK" of window featureName
[debug] [iOSSim]           delay 0.5
[debug] [iOSSim]           set isInvisible to (not (exists (window featureName)))
[debug] [iOSSim]         end tell
[debug] [iOSSim]       end tell
[debug] [iOSSim]
[debug] [iOSSim] Geolocation parameters dialog accepted: true
[debug] [iOSSim]
[debug] [W3C (ae50e600)] Responding to client with driver.setGeoLocation() result: null
[HTTP] <-- POST /wd/hub/session/ae50e6

mykola-mokhnach · 2019-12-06T06:26:50Z

lib/simulator-xcode-8.js

+      ]);
+      return true;
+    } catch (ign) {}
+    log.warn(`Cannot set geolocation with ${LYFT_SET_LOCATION}, because it is not installed or failed to set.`);


I'd rather log the warning inside catch block and additionally print the stderr output. fs.which should be in separate block, so we could supply more detailed instructions on how to install the tool if it is not installed

it would be also more logical to extract the whole stuff to a separate method/helper function

mykola-mokhnach · 2019-12-06T14:12:08Z

lib/geolocation.js

+ *   into the corresponding edit field, for example '19,0068'.
+ * @return {boolean} True if the given geolocation was able to set.
+ */
+async function execLyftSetLocation (udid, latitude, longitude) {


I would prefer this function to throw an exception in case of failure rather then returning a boolean result

lib/geolocation.js

mykola-mokhnach · 2019-12-06T14:14:09Z

lib/geolocation.js

+ *   into the corresponding edit field, for example '19,0068'.
+ * @return {boolean} True if the given geolocation was able to set.
+ */
+async function execIbdSetLocation (idb, latitude, longitude) {


I'd rather rename the functions to setLocationWith...

mykola-mokhnach · 2019-12-06T14:15:34Z

lib/simulator-xcode-8.js

-    `);
-    log.debug(`Geolocation parameters dialog accepted: ${output}`);
-    return _.isString(output) && output.trim() === 'true';
+    return await execLyftSetLocation(this.udid, latitude, longitude)


The method should throw an error if all the possible set location methods failed

mykola-mokhnach · 2019-12-06T16:51:17Z

lib/geolocation.js

 */
-async function execLyftSetLocation (udid, latitude, longitude) {
+async function setLocationWithLyftSetLocation (udid, latitude, longitude) {


setLocationWithLyft should be enough

mykola-mokhnach · 2019-12-06T16:51:41Z

lib/geolocation.js

 */
-async function execLyftSetLocation (udid, latitude, longitude) {
+async function setLocationWithLyftSetLocation (udid, latitude, longitude) {
  try {
    await fs.which(LYFT_SET_LOCATION);
  } catch (e) {
    log.info(`'${LYFT_SET_LOCATION}' binary has not been found in your PATH. ` +


an exception should be throw here

mykola-mokhnach · 2019-12-06T16:51:53Z

lib/geolocation.js

  } catch (e) {
-    log.warn(`Failed to set geolocation with '${LYFT_SET_LOCATION}'. ` +
+    throw new Error(`Failed to set geolocation with '${LYFT_SET_LOCATION}'. ` +
      `Original error: ${e.message}`);


e.stderr || e.message

mykola-mokhnach · 2019-12-06T16:52:19Z

lib/geolocation.js

  }
-  log.info(`Cannot set geolocation with idb, because it is not installed`);
-  return false;
+  throw new Error('Failed to set geolocation with idb. because it is not installed');


...with idb because...

mykola-mokhnach · 2019-12-06T16:54:06Z

lib/geolocation.js

@@ -91,9 +87,11 @@ async function execAppleScript (sim, latitude, longitude) {
    end tell
  `);
  log.debug(`Geolocation parameters dialog accepted: ${output}`);
-  return _.isString(output) && output.trim() === 'true';
+  if (_.isString(output) && output.trim() !== 'true') {


if (_.trim(output) !== 'true') { throw new Error('Failed to set geolocation with AppleScript. Original error: ${output}'); }

mykola-mokhnach · 2019-12-06T16:54:49Z

lib/geolocation.js

 */
-async function execIbdSetLocation (idb, latitude, longitude) {
+async function setLocationWithIbdSetLocation (idb, latitude, longitude) {


setLocationWithIdb should be fine (also there's a typo ;) )

mykola-mokhnach · 2019-12-06T16:57:40Z

lib/simulator-xcode-8.js

-    return await execLyftSetLocation(this.udid, latitude, longitude)
-      || await execIbdSetLocation(this.idb, latitude, longitude)
-      || await execAppleScript(this, latitude, longitude);
+    try {


const locationSetters = [ async () => await setLocationWithLyft(this.udid, latitude, longitude), async () => await setLocationWithIbdSetLocation(this.idb, latitude, longitude), ... ]; let lastError; for (const setter of locationSetters) { try { await setter(); return true; } catch (e) { log.info(e.message); lastError = e; } } throw lastError;

feat: Add set geolocation by set-simulator-location

b3fde0c

KazuCocoa requested a review from mykola-mokhnach December 6, 2019 05:56

KazuCocoa changed the title ~~feat: Add set geolocation by set-simulator-location as the first method~~ feat: Add setting geolocation by set-simulator-location as the first method Dec 6, 2019

mykola-mokhnach reviewed Dec 6, 2019

View reviewed changes

KazuCocoa added 3 commits December 6, 2019 18:29

extract geolocation stuff

5e39986

tweak messages

d2353ff

Add return comment

8a100fa

mykola-mokhnach reviewed Dec 6, 2019

View reviewed changes

lib/geolocation.js Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Dec 6, 2019

View reviewed changes

KazuCocoa added 3 commits December 7, 2019 00:36

rename method name, add brew install

78a73e7

try/catch in setGeolocation

8c73ce5

tweak error message

1758aac

mykola-mokhnach reviewed Dec 6, 2019

View reviewed changes

KazuCocoa added 2 commits December 7, 2019 02:25

tweak error, method names

d3c2229

Make three set location as an array exec

f530c79

KazuCocoa mentioned this pull request Dec 6, 2019

feat: Add set-simulator-location check appium/appium-doctor#87

Merged

mykola-mokhnach approved these changes Dec 6, 2019

View reviewed changes

KazuCocoa merged commit 7d11c85 into appium:master Dec 7, 2019

KazuCocoa deleted the km/add-set-geolocation branch December 7, 2019 01:11

imurchie mentioned this pull request Dec 11, 2019

[WIP] Appium 1.16.0 appium/appium#13634

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add setting geolocation by set-simulator-location as the first method #249

feat: Add setting geolocation by set-simulator-location as the first method #249

KazuCocoa commented Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019

mykola-mokhnach Dec 6, 2019 •

edited

Loading

KazuCocoa Dec 6, 2019

feat: Add setting geolocation by set-simulator-location as the first method #249

feat: Add setting geolocation by set-simulator-location as the first method #249

Conversation

KazuCocoa commented Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mykola-mokhnach Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

mykola-mokhnach Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mykola-mokhnach Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KazuCocoa commented Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019 •

edited

Loading

mykola-mokhnach Dec 6, 2019 •

edited

Loading