added startActivity function #101

0x1mason · 2014-09-05T21:12:37Z

Related to appium/appium #2969. Added startActivity api that maps to new appium feature.

Jonahss · 2014-09-05T23:56:52Z

src/main/java/io/appium/java_client/AppiumDriver.java

+   * @param options 'appPackage' and 'appActivity' are required;
+   *                'appWaitPackage' and 'appWaitActivity' are optional
+   */
+  public void startActivity(Map<String, String> options){


Since this is a client, we try to be convenient :) Instead of an 'options' object, which can have any keys, and putting the key names in the comments, can we have a method that just accepts 4 arguments? Since two are optional, we can have two methods, one calling the other.

Are appWaitPkg and appWaitAct dependent on the other? IE can you have just
one of them?

On Friday, September 5, 2014, Jonah [email protected] wrote:

In src/main/java/io/appium/java_client/AppiumDriver.java:

@@ -168,6 +169,17 @@ public String currentActivity() {
Response response = execute(CURRENT_ACTIVITY);
return response.getValue().toString();
}
+

/**

* Launches an arbitrary activity during a test. If the activity belongs to

* another application, that application is started and the activity is opened.

* This is an Android only method.

* @param options 'appPackage' and 'appActivity' are required;

* 'appWaitPackage' and 'appWaitActivity' are optional

*/

public void startActivity(Map<String, String> options){

Since this is a client, we try to be convenient :) Instead of an 'options'
object, which can have any keys, and putting the key names in the comments,
can we have a method that just accepts 4 arguments? Since two are optional,
we can have two methods, one calling the other.

—
Reply to this email directly or view it on GitHub
https://github.com/appium/java-client/pull/101/files#r17204294.

Yes, you can have just one of them.

In that case, we can't have overloads to handle all the cases. That's why I
went with the map. We can have:

startAct(appPkg, appAct)
startAct(appPkg, appAct, appWaitPkg OR -Act) <= can't have overloads for
both cases, won't compile
startAct(appPkg, appAct, appWaitPkg, appWaitAct)

I say we either just go for signature #3, in which case users will have to
pass null for whichever of the last 2 they don't care about, or we stick
with the hash map.

On Saturday, September 6, 2014, Jonathan Lipps [email protected]
wrote:

In src/main/java/io/appium/java_client/AppiumDriver.java:

@@ -168,6 +169,17 @@ public String currentActivity() {
Response response = execute(CURRENT_ACTIVITY);
return response.getValue().toString();
}
+

/**

* Launches an arbitrary activity during a test. If the activity belongs to

* another application, that application is started and the activity is opened.

* This is an Android only method.

* @param options 'appPackage' and 'appActivity' are required;

* 'appWaitPackage' and 'appWaitActivity' are optional

*/

public void startActivity(Map<String, String> options){

Yes, you can have just one of them.

—
Reply to this email directly or view it on GitHub
https://github.com/appium/java-client/pull/101/files#r17211060.

0x1mason · 2014-09-08T18:18:21Z

@jlipps @Jonahss Updated the code with parameter validation and new startActivity signature. Went with one sig that takes null args, since this is what other functions do, eg sendKeyEvent.

bootstraponline · 2014-09-08T18:19:51Z

Is the java-client.iml change intentional?

bootstraponline · 2014-09-08T18:23:41Z

src/main/java/io/appium/java_client/remote/MobileCapabilityType.java

@@ -19,4 +19,5 @@
 	String APP_PACKAGE = "appPackage";
 	String APP_ACTIVITY = "appActivity";
 	String APP_WAIT_ACTIVITY = "appWaitActivity";
+    String APP_WAIT_PACKAGE = "appWaitPackage";


spacing seems incorrect

0x1mason · 2014-09-08T18:56:48Z

K, hopefully it's fixed.

bootstraponline · 2014-09-08T18:57:51Z

src/main/java/io/appium/java_client/AppiumDriver.java

@@ -17,9 +17,11 @@

 package io.appium.java_client;

+import com.google.common.base.Preconditions.*;


I don't think you need this? The IDE has an option called optimize imports which will resolve any problems.

bootstraponline · 2014-09-08T19:06:24Z

src/main/java/io/appium/java_client/remote/MobileCapabilityType.java

@@ -19,4 +19,5 @@
 	String APP_PACKAGE = "appPackage";
 	String APP_ACTIVITY = "appActivity";
 	String APP_WAIT_ACTIVITY = "appWaitActivity";
+  String APP_WAIT_PACKAGE = "appWaitPackage";


spacing on this still seems wrong

bootstraponline · 2014-09-08T19:17:32Z

looks good to me. 👍

Jonahss · 2014-09-08T23:06:20Z

Tests don't pass, is that expected? Or am I running it wrong?

0x1mason · 2014-09-08T23:08:44Z

They pass for me, so no, it's not expected.

On Mon, Sep 8, 2014 at 7:06 PM, Jonah [email protected] wrote:

Tests don't pass, is that expected? Or am I running it wrong?

—
Reply to this email directly or view it on GitHub
#101 (comment).

jlipps · 2014-09-08T23:27:01Z

@Jonahss did you run reset on master?

Jonahss · 2014-09-08T23:59:49Z

Hmmm. thought I did. Re-running

Jonahss · 2014-09-09T00:17:15Z

I pulled @0x1mason's fork of appium and checked out the 2969 branch. Ran reset.sh --dev
Test fails:

@Test
  public void startActivityInThisAppTest(){
    driver.startActivity("io.appium.android.apis", ".accessibility.AccessibilityNodeProviderActivity", null, null);
    String activity = driver.currentActivity();
    System.out.println(activity);                    // .ApiDemos
    assertTrue(activity.contains("Node"));   // .ApiDemos doesn't contain Node so this fails
  }

Jonahss · 2014-09-09T00:17:58Z

Are you running just that test or the whole suite? I'm running the test individually.

0x1mason · 2014-09-09T00:45:48Z

Just the individual test. Could be a race condition. Maybe my emulator
runs faster? Do you see the Accessibility Node activity open?

On 9/8/14, Jonah [email protected] wrote:

Are you running just that test or the whole suite? I'm running the test
individually.

Reply to this email directly or view it on GitHub:
#101 (comment)

Jonahss · 2014-09-09T01:07:34Z

Nope, don't see it open.

I added in a 20second wait and I still don't see it actually open O.o

info: --> POST /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/start_activity {"appWaitPackage":"","appActivity":".accessibility.AccessibilityNodeProviderActivity","appWaitActivity":"","appPackage":"io.appium.android.apis"}
info: [debug] Getting device API level
info: [debug] executing: "/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb" -s emulator-5554 shell "getprop ro.build.version.sdk"
info: [debug] Device is at API Level 19
info: [debug] executing: "/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb" -s emulator-5554 shell "am start -S -a undefined -c undefined -f undefined -n io.appium.android.apis/.accessibility.AccessibilityNodeProviderActivity"
info: [debug] Responding to client with success: {"status":0,"value":"Successfully launched the app.","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: <-- POST /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/start_activity 200 4280.604 ms - 104 {"status":0,"value":"Successfully launched the app.","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: --> GET /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/current_activity {}
info: [debug] Getting focused package and activity
info: [debug] executing: "/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb" -s emulator-5554 shell "dumpsys window windows"
info: [debug] Responding to client with success: {"status":0,"value":".ApiDemos","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: <-- GET /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/current_activity 200 671.921 ms - 83 {"status":0,"value":".ApiDemos","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: --> DELETE /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e {}
info: Shutting down appium session

0x1mason · 2014-09-09T01:24:31Z

I see the problem. You need 2969 branch of appium-adb.

On 9/8/14, Jonah [email protected] wrote:

Nope, don't see it open.

I added in a 20second wait and I still don't see it actually open O.o

info: --> POST
/wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/start_activity
{"appWaitPackage":"","appActivity":".accessibility.AccessibilityNodeProviderActivity","appWaitActivity":"","appPackage":"io.appium.android.apis"}
info: [debug] Getting device API level
info: [debug] executing:
"/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb"
-s emulator-5554 shell "getprop ro.build.version.sdk"
info: [debug] Device is at API Level 19
info: [debug] executing:
"/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb"
-s emulator-5554 shell "am start -S -a undefined -c undefined -f undefined
-n io.appium.android.apis/.accessibility.AccessibilityNodeProviderActivity"
info: [debug] Responding to client with success:
{"status":0,"value":"Successfully launched the
app.","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: <-- POST
/wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/start_activity
200 4280.604 ms - 104 {"status":0,"value":"Successfully launched the
app.","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: --> GET
/wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/current_activity
{}
info: [debug] Getting focused package and activity
info: [debug] executing:
"/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb"
-s emulator-5554 shell "dumpsys window windows"
info: [debug] Responding to client with success:
{"status":0,"value":".ApiDemos","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: <-- GET
/wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e/appium/device/current_activity
200 671.921 ms - 83
{"status":0,"value":".ApiDemos","sessionId":"4db4fef4-2c45-4701-bc6e-6e73b349a52e"}
info: --> DELETE /wd/hub/session/4db4fef4-2c45-4701-bc6e-6e73b349a52e {}
info: Shutting down appium session

Reply to this email directly or view it on GitHub:
#101 (comment)

bootstraponline · 2014-09-09T01:26:04Z

wasn't that branch merged into master?

0x1mason · 2014-09-09T01:44:57Z

Looks like it. Something's broken how the adb cmd line is
constructed. Checkout all the undefined adb args in Jonahs logs.

On 9/8/14, bootstraponline [email protected] wrote:

wasn't that branch merged into
master?

Reply to this email directly or view it on GitHub:
#101 (comment)

Jonahss · 2014-09-09T02:14:32Z

Oh yeah, that is pretty undefined. Wonder why no errors were thrown.

0x1mason · 2014-09-09T11:48:27Z

Dunno. That shouldn't happen if your appium-adb branch is at HEAD or
you have 2969 checked out. Undefined params won't get added to the cmd
line.

On 9/8/14, Jonah [email protected] wrote:

Oh yeah, that is pretty undefined. Wonder why no errors were thrown.

Reply to this email directly or view it on GitHub:
#101 (comment)

jlipps · 2014-09-09T16:34:49Z

Is a version of appium-adb with the appropriate commit included in appium's submodules/package.json?

If not, I think that needed to be part of the appium PR to keep this in sync.

If so, I leave the investigation to you guys :-)=

bootstraponline · 2014-09-09T16:36:21Z

@Jonahss try a fresh clone of appium into a new directory and then invoke reset.sh.

0x1mason · 2014-09-09T16:49:55Z

@jlipps: appium-adb commit is probably wrong in appium branch 2969.

On Tue, Sep 9, 2014 at 12:36 PM, bootstraponline [email protected]
wrote:

@Jonahss https://github.com/Jonahss try a fresh clone of appium into a
new directory and then invoke reset.

—
Reply to this email directly or view it on GitHub
#101 (comment).

Jonahss · 2014-09-09T18:28:22Z

What's holding back the merge of 2969 into appium master? That can happen prior to the java-client being updated, can't it?

0x1mason · 2014-09-09T18:58:20Z

I guess it could, but the docs have examples from the client libraries, so
the signatures and args in client and server docs need to match.

On Tue, Sep 9, 2014 at 2:28 PM, Jonah [email protected] wrote:

What's holding back the merge of 2969 into appium master? That can happen
prior to the java-client being updated, can't it?

—
Reply to this email directly or view it on GitHub
#101 (comment).

Jonahss · 2014-09-09T20:28:12Z

Oh I see.
How do we want to go forward? I like merging appium master, then having the
clients update, but maybe it's not the best way to go about it?

On Tue, Sep 9, 2014 at 11:58 AM, Eric Millin [email protected]
wrote:

I guess it could, but the docs have examples from the client libraries, so
the signatures and args in client and server docs need to match.

On Tue, Sep 9, 2014 at 2:28 PM, Jonah [email protected] wrote:

What's holding back the merge of 2969 into appium master? That can
happen
prior to the java-client being updated, can't it?

—
Reply to this email directly or view it on GitHub
#101 (comment).

—
Reply to this email directly or view it on GitHub
#101 (comment).

bootstraponline · 2014-09-09T20:30:27Z

I like merging appium master, then having the
clients update, but maybe it's not the best way to go about it?

I'm pretty sure that's the best way. It's also how we've done things so far. The clients are expected to pass against appium master when they add the new method.

Jonahss · 2014-09-09T21:01:00Z

In this case though, I could just merge the PR because the client is
sending the right requests, it's just my appium setup which is faulty...

On Tue, Sep 9, 2014 at 1:30 PM, bootstraponline [email protected]
wrote:

I like merging appium master, then having the
clients update, but maybe it's not the best way to go about it?

I'm pretty sure that's the best way. It's also how we've done things so
far. The clients are expected to pass against appium master when they add
the new method.

—
Reply to this email directly or view it on GitHub
#101 (comment).

bootstraponline · 2014-09-09T21:22:39Z

I guess, as long as you're okay with testing against a faulty setup.

0x1mason · 2014-09-09T22:15:31Z

@Jonahss You could also just pull the most recent version of appium-adb and run the tests.

Jonahss · 2014-09-10T18:16:32Z

hmm. Now I get this error:

info: --> POST /wd/hub/session/62badc94-08f9-4853-91ab-68b6a00ac77e/appium/device/start_activity {"appWaitPackage":"","appActivity":".accessibility.AccessibilityNodeProviderActivity","appWaitActivity":"","appPackage":"io.appium.android.apis"}
info: [debug] Getting device API level
info: [debug] executing: "/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb" -s emulator-5554 shell "getprop ro.build.version.sdk"
info: [debug] Device is at API Level 19
info: [debug] executing: "/Users/jonahss/Android/adt-bundle-mac-x86_64-20131030/sdk/platform-tools/adb" -s emulator-5554 shell "am start -S -n io.appium.android.apis/.accessibility.AccessibilityNodeProviderActivity"
error: Activity used to start app doesn't exist or cannot be launched! Make sure it exists and is a launchable activity
info: [debug] Responding to client with error: {"status":1,"value":{"message":"Unable to launch the app: Error: Activity used to start app doesn't exist or cannot be launched! Make sure it exists and is a launchable activity"},"sessionId":"62badc94-08f9-4853-91ab-68b6a00ac77e"}
info: <-- POST /wd/hub/session/62badc94-08f9-4853-91ab-68b6a00ac77e/appium/device/start_activity 500 6617.593 ms - 231

jlipps · 2014-09-10T18:32:18Z

I think merging into master before the clients is a good idea. Just waiting on the package.json update and conflict resolution.

Jonahss · 2014-09-10T18:50:38Z

My bad, still hadn't gotten the right version of appium-adb. tests pass locally, merging.

Jonahss · 2014-09-10T21:05:17Z

src/main/java/io/appium/java_client/AppiumDriver.java

@@ -574,7 +617,7 @@ public void setNetworkConnection(NetworkConnectionSetting connection) {

  @Override
  public WebDriver context(String name) {
-    if (name == null) {
+    if (_isNotNullOrEmpty(name)) {


whoops, accidental bug which escaped our code review. yay for tests.
changed to !_isNotNullOrEmpty

Jonahss reviewed Sep 5, 2014
View reviewed changes

0x1mason force-pushed the 2969 branch 2 times, most recently from 047bf83 to 3aa020c Compare September 8, 2014 18:16

bootstraponline reviewed Sep 8, 2014
View reviewed changes

0x1mason force-pushed the 2969 branch 2 times, most recently from cf3f639 to 967c24b Compare September 8, 2014 18:56

bootstraponline reviewed Sep 8, 2014
View reviewed changes

0x1mason force-pushed the 2969 branch from 967c24b to 50ed9ce Compare September 8, 2014 19:05

bootstraponline reviewed Sep 8, 2014
View reviewed changes

added startActivity function

409c344

0x1mason force-pushed the 2969 branch from 50ed9ce to 409c344 Compare September 8, 2014 19:16

Jonahss merged commit 409c344 into appium:master Sep 10, 2014

Jonahss added a commit that referenced this pull request Sep 10, 2014

fix tests. fixed a bug in context() introduced in #101

06a2c34

Jonahss reviewed Sep 10, 2014
View reviewed changes

		@@ -17,9 +17,11 @@

		package io.appium.java_client;

		import com.google.common.base.Preconditions.*;

added startActivity function #101

added startActivity function #101

Conversation

0x1mason commented Sep 5, 2014

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

0x1mason commented Sep 8, 2014

bootstraponline commented Sep 8, 2014

Choose a reason for hiding this comment

0x1mason commented Sep 8, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bootstraponline commented Sep 8, 2014

Jonahss commented Sep 8, 2014

0x1mason commented Sep 8, 2014

jlipps commented Sep 8, 2014

Jonahss commented Sep 8, 2014

Jonahss commented Sep 9, 2014

Jonahss commented Sep 9, 2014

0x1mason commented Sep 9, 2014

Jonahss commented Sep 9, 2014

0x1mason commented Sep 9, 2014

bootstraponline commented Sep 9, 2014

0x1mason commented Sep 9, 2014

Jonahss commented Sep 9, 2014

0x1mason commented Sep 9, 2014

jlipps commented Sep 9, 2014

bootstraponline commented Sep 9, 2014

0x1mason commented Sep 9, 2014

Jonahss commented Sep 9, 2014

0x1mason commented Sep 9, 2014

Jonahss commented Sep 9, 2014

bootstraponline commented Sep 9, 2014

Jonahss commented Sep 9, 2014

bootstraponline commented Sep 9, 2014

0x1mason commented Sep 9, 2014

Jonahss commented Sep 10, 2014

jlipps commented Sep 10, 2014

Jonahss commented Sep 10, 2014

Choose a reason for hiding this comment