add support to set UiAutomator Congfigurator values #153

truebit · 2016-06-08T07:21:06Z

jlipps · 2016-06-08T09:52:03Z

lib/driver.js

@@ -42,8 +42,11 @@ class AndroidDriver extends BaseDriver {
    this.sessionChromedrivers = {};
    this.jwpProxyActive = false;
    this.jwpProxyAvoid = _.clone(NO_PROXY);
-    this.settings = new DeviceSettings({ignoreUnimportantViews: false},
-                                       this.onSettingsUpdate.bind(this));
+    this.settings = new DeviceSettings({ignoreUnimportantViews: false,


this needs a little formatting work. first, let's do one key/value per line. second, we do key: value (with a space after colon. third, we do:

foo({ new: 'line', before: 'keys/values' });

to save horizontal space :-)

jlipps · 2016-06-08T09:59:39Z

couple comments, then again a test would be nice!

jlipps · 2016-06-08T14:12:45Z

OK, I've had more time to grok what you are trying to do here, and I'd like to suggest a change in the approach.

Currently, you're asking the client to do something like:

POST /wd/hub/session/<sessionId/appium/settings

with a request body of:

{"settings": {"configurator": "{\"setKeyIntervalDelay\": 1000}"}}

In other words, you're creating a "configurator" group of settings that have their own internal API. Instead what I'd like to see is simply all the configurator options as top-level settings. So the client request body would look like:

{"settings": {"keyIntervalDelay": 1000}}

(Notice also no need to put set in keyIntervalDelay)

Then you'll need to change the way the code here works. Instead of:

if (key === "configurator") {
  await this.setConfiguratorConfig(value);
}

You'd have:

if (key === "keyIntervalDelay") {
  await this.setConfiguratorConfig({"setKeyIntervalDelay": value});
}

And so on with all the configurator config options you want to expose to the clients via this settings API.

(And you'd no longer need to JSON.parse in setConfiguratorConfig)

Does all this make sense? I think this is a great addition to the driver, thanks!

truebit · 2016-06-09T03:51:53Z

ok, I am now on vacation until next Monday. Will get on it after vocation. Thanks for the advice:)

Concerning about the test, maybe we should create more methods to implement getting these configurations from Configurator? So tests could be done by setting firstly and then verifying via getting

Sent from my mobile device.

在 2016年6月8日，22:12，Jonathan Lipps notifications@github.com 写道：

OK, I've had more time to grok what you are trying to do here, and I'd like to suggest a change in the approach.

Currently, you're asking the client to do something like:

POST /wd/hub/session/<sessionId/appium/settings
with a request body of:

{"settings": {"configurator": "{"setKeyIntervalDelay": 1000}"}}
In other words, you're creating a "configurator" group of settings that have their own internal API. Instead what I'd like to see is simply all the configurator options as top-level settings. So the client request body would look like:

{"settings": {"keyIntervalDelay": 1000}}
(Notice also no need to put set in keyIntervalDelay)

Then you'll need to change the way the code here works. Instead of:

if (key === "configurator") {
await this.setConfiguratorConfig(value);
}
You'd have:

if (key === "keyIntervalDelay") {
await this.setConfiguratorConfig({"setKeyIntervalDelay": value});
}
And so on with all the configurator config options you want to expose to the clients via this settings API.

(And you'd no longer need to JSON.parse in setConfiguratorConfig)

Does all this make sense? I think this is a great addition to the driver, thanks!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

truebit · 2016-06-25T08:28:53Z

@jlipps Sorry for the late reply. I tried to refactor as per your last comment using top level setting for all Configurator related configurations, it seems not making it less complex:

first change onSettingsUpdate as you said:

  async onSettingsUpdate (key, value) {
    if (key === "ignoreUnimportantViews") {
      await this.setCompressedLayoutHierarchy(value);
    }
    else if (key === "waitForIdleTimeout") {
      await this.setConfiguratorConfig({"waitForIdleTimeout": value});
    }
    else if (key === "waitForSelectorTimeout") {
      await this.setConfiguratorConfig({"waitForSelectorTimeout": value});
    }
    else if (key === "keyInjectionDelay") {
      await this.setConfiguratorConfig({"keyInjectionDelay": value});
    }
    else if (key === "actionAcknowledgmentTimeout") {
      await this.setConfiguratorConfig({"actionAcknowledgmentTimeout": value});
    }
    else if (key === "scrollAcknowledgmentTimeout") {
      await this.setConfiguratorConfig({"scrollAcknowledgmentTimeout": value});
    }
  }

Then change setConfiguratorConfig like below:

      // Set UiAutomator Configurator related configurations
      // value sould be like {"waitForIdleTimeout":5000}
      async setConfiguratorConfig (value) {
        for (let k in value) { // maybe need to check value object is a dictionary?
          await this.bootstrap.sendAction(k, value);
        }
      }

and change appium-android-bootstrap to handle all these settings

    map.put("waitForIdleTimeout", new ConfiguratorHandler());
    map.put("waitForSelectorTimeout", new ConfiguratorHandler());
    map.put("keyInjectionDelay", new ConfiguratorHandler());
    map.put("actionAcknowledgmentTimeout", new ConfiguratorHandler());
    map.put("scrollAcknowledgmentTimeout", new ConfiguratorHandler());

Is that all you expected?

jlipps · 2016-06-27T17:15:20Z

Hi @truebit the point wasn't necessarily to make the code less complex, it was to make it more compatible and user-friendly. But some improvements I could make to your examples:

  async onSettingsUpdate (key, value) {
    if (key === "ignoreUnimportantViews") {
      await this.setCompressedLayoutHierarchy(value);
    } else {
      await this.setConfiguratorConfig({key, value});
    }
  }

and

  // Set UiAutomator Configurator related configurations
  // value sould be like {"waitForIdleTimeout":5000}
  async setConfiguratorConfig (config) {
    await this.bootstrap.sendAction(config.key, config.value);
  }

but yeah, this is generally the right idea!

truebit · 2016-06-29T08:51:15Z

but the second param of this.bootstrap.sendAction must be a key-value object.
I use await this.bootstrap.sendAction("configurator", {config: config.key, value: config.value});, so that in appium-android-bootstrap, we could use just ONE class ConfiguatorHandler to handle all configurator settings

truebit · 2016-06-29T09:47:56Z

Concerning about the tests, after update settings, how should we get the real Configurator result to verify it?

jlipps · 2016-06-29T16:15:56Z

I use await this.bootstrap.sendAction("configurator", {config: config.key, value: config.value});, so that in appium-android-bootstrap, we could use just ONE class ConfiguatorHandler to handle all configurator settings

Sure that makes sense!

jlipps · 2016-06-29T16:16:27Z

Concerning about the tests, after update settings, how should we get the real Configurator result to verify it?

How would you verify that it is working as a user?

truebit · 2016-07-02T10:53:13Z

How would you verify that it is working as a user?

IMO this could hardly verify from a user's perspective in code. I could think the only verification method by using corresponding Configurator's get methods.
Does appium settings api has method to handlingGET method? like onSettingsUpdate being done in POST method? If so, we could override that method to get coresponding Congurator values to do the verification.

jlipps · 2016-07-05T16:28:49Z

Yeah that's the trick, it doesn't have such a method. Hmm. I suppose you could expose one from the bootstrap just for testing...

imurchie · 2016-07-28T16:48:24Z

Where are we on this PR?

truebit · 2016-07-29T03:14:03Z

@imurchie
As @jlipps mentioned, creating another method internally to get the Configurator values to verify this PR is too complex and there is no API in appium to support passing this value through. I have no idea how to do this.

Recently I found that uiObject.click() would wait for Configurator.getwaitForSelectorTimeout() timeout to find an element. But I do not know how to trigger that in Appium.

jlipps · 2017-08-03T23:07:14Z

@truebit what about putting a value on the result of the GET /session/:id call?

jlipps · 2017-08-18T16:41:59Z

@truebit what do you think about that idea?

3a4oT · 2018-04-03T11:34:18Z

@jlipps not sure if you are the right person to ping but can it be related to https://stackoverflow.com/questions/49628336/appium-android-set-custom-timeout-between-click-tap ?

mykola-mokhnach · 2019-01-28T13:44:21Z

Closed as obsolete

add support to set UiAutomator Congfigurator values

026c39c

truebit mentioned this pull request Jun 8, 2016

add support to set UiAutomator Congfigurator values appium/appium#6561

Merged

9 tasks

add semicolon

644d122

jlipps reviewed Jun 8, 2016
View reviewed changes

汪啸 added 2 commits June 29, 2016 17:24

modify methods to make configurator settings in top level

2a30312

correct wrong comment description

35d9a88

fix code style

5c13baa

truebit mentioned this pull request Sep 26, 2016

Android setting engine appium/java-client#477

Merged

3 tasks

jlipps mentioned this pull request Aug 3, 2017

Make UiAutomator configurations customizable appium/appium#6547

Open

KazuCocoa mentioned this pull request Apr 4, 2018

Add uiautomator configurations appium/appium-uiautomator2-server#131

Merged

mykola-mokhnach closed this Jan 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support to set UiAutomator Congfigurator values #153

add support to set UiAutomator Congfigurator values #153

truebit commented Jun 8, 2016

jlipps Jun 8, 2016

jlipps commented Jun 8, 2016

jlipps commented Jun 8, 2016

truebit commented Jun 9, 2016

truebit commented Jun 25, 2016 •

edited

Loading

jlipps commented Jun 27, 2016

truebit commented Jun 29, 2016 •

edited

Loading

truebit commented Jun 29, 2016

jlipps commented Jun 29, 2016

jlipps commented Jun 29, 2016

truebit commented Jul 2, 2016 •

edited

Loading

jlipps commented Jul 5, 2016

imurchie commented Jul 28, 2016

truebit commented Jul 29, 2016 •

edited

Loading

jlipps commented Aug 3, 2017

jlipps commented Aug 18, 2017

3a4oT commented Apr 3, 2018 •

edited

Loading

mykola-mokhnach commented Jan 28, 2019

add support to set UiAutomator Congfigurator values #153

add support to set UiAutomator Congfigurator values #153

Conversation

truebit commented Jun 8, 2016

jlipps Jun 8, 2016

Choose a reason for hiding this comment

jlipps commented Jun 8, 2016

jlipps commented Jun 8, 2016

truebit commented Jun 9, 2016

truebit commented Jun 25, 2016 • edited Loading

jlipps commented Jun 27, 2016

truebit commented Jun 29, 2016 • edited Loading

truebit commented Jun 29, 2016

jlipps commented Jun 29, 2016

jlipps commented Jun 29, 2016

truebit commented Jul 2, 2016 • edited Loading

jlipps commented Jul 5, 2016

imurchie commented Jul 28, 2016

truebit commented Jul 29, 2016 • edited Loading

jlipps commented Aug 3, 2017

jlipps commented Aug 18, 2017

3a4oT commented Apr 3, 2018 • edited Loading

mykola-mokhnach commented Jan 28, 2019

truebit commented Jun 25, 2016 •

edited

Loading

truebit commented Jun 29, 2016 •

edited

Loading

truebit commented Jul 2, 2016 •

edited

Loading

truebit commented Jul 29, 2016 •

edited

Loading

3a4oT commented Apr 3, 2018 •

edited

Loading