[Android] Create a dummy app to upload to Appurify for gtests.

[Android] Create a dummy app to upload to Appurify for gtests.

BUG=428729
Committed: https://crrev.com/335cbe5177b3d0ec5d5564b646cb1a7b814c26d2
Cr-Commit-Position: refs/heads/master@{#310159}

[jbudorick, 2015-01-06 15:27:59]

jbudorick@chromium.org changed reviewers:
+ cjhopman@chromium.org, klundberg@chromium.org, rnephew@chromium.org

[jbudorick, 2015-01-06 15:28:00]

rnephew: b/a/pylib/remote/device/*.py
klundberg: general
cjhopman: gyp + whatever else you'd like to comment on

[klundberg, 2015-01-06 18:17:31]

My only concern is about using "robotium" as we do not actually run Robotium
tests at all. I'm sorry for having missed this in previous reviews.

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/dummy/dummy.gyp (right):

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/dummy/dummy.gyp:5: # Running Robotium tests on
AMP requires both an "app" APK and a "test" APK
"Robotium" can be a little confusing as we won't actually run Robotium tests at
all. I know that is what AMP uses but maybe you could make this comment a little
less "implementation specific" to avoid confusion?

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_gtest_run.py (right):

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_gtest_run.py:47: if runner_type
== 'robotium':
Similar comment about using "robotium" here. Seems confusing.

And I'm sorry I seemed to have missed this in a previous review as this file is
already using robotium.

[jbudorick, 2015-01-06 18:25:57]

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/dummy/dummy.gyp (right):

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/dummy/dummy.gyp:5: # Running Robotium tests on
AMP requires both an "app" APK and a "test" APK
On 2015/01/06 18:17:31, klundberg wrote:
> "Robotium" can be a little confusing as we won't actually run Robotium tests
at
> all. I know that is what AMP uses but maybe you could make this comment a
little
> less "implementation specific" to avoid confusion?

The issue with that is that the entire reason for this target _is_
implementation specific - AMP's "robotium" test type _requires_ an app APK and a
test APK.

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_gtest_run.py (right):

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_gtest_run.py:47: if runner_type
== 'robotium':
On 2015/01/06 18:17:31, klundberg wrote:
> Similar comment about using "robotium" here. Seems confusing.
> 
> And I'm sorry I seemed to have missed this in a previous review as this file
is
> already using robotium.

The implementation _is_ specific to "robotium" tests on AMP. While it's true
that these aren't robotium tests, they are running in AMP's robotium mechanism,
and this script code is specifically written for that. Naming it this way seemed
the least confusing way of doing things.

I understand that running gtests via a "robotium" mechanism is a little
confusing, though, and we should probably clarify that in comments.

[jbudorick, 2015-01-06 18:29:40]

On 2015/01/06 18:25:57, jbudorick wrote:
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> File build/android/pylib/remote/device/dummy/dummy.gyp (right):
> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> build/android/pylib/remote/device/dummy/dummy.gyp:5: # Running Robotium tests
on
> AMP requires both an "app" APK and a "test" APK
> On 2015/01/06 18:17:31, klundberg wrote:
> > "Robotium" can be a little confusing as we won't actually run Robotium tests
> at
> > all. I know that is what AMP uses but maybe you could make this comment a
> little
> > less "implementation specific" to avoid confusion?
> 
> The issue with that is that the entire reason for this target _is_
> implementation specific - AMP's "robotium" test type _requires_ an app APK and
a
> test APK.

(to clarify here: we're currently uploading the lone gtest APK as both the app
and the test, but it turns out that AMP doesn't really like that for reasons I
can discuss in further depth if you're curious)

> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> File build/android/pylib/remote/device/remote_device_gtest_run.py (right):
> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> build/android/pylib/remote/device/remote_device_gtest_run.py:47: if
runner_type
> == 'robotium':
> On 2015/01/06 18:17:31, klundberg wrote:
> > Similar comment about using "robotium" here. Seems confusing.
> > 
> > And I'm sorry I seemed to have missed this in a previous review as this file
> is
> > already using robotium.
> 
> The implementation _is_ specific to "robotium" tests on AMP. While it's true
> that these aren't robotium tests, they are running in AMP's robotium
mechanism,
> and this script code is specifically written for that. Naming it this way
seemed
> the least confusing way of doing things.
> 
> I understand that running gtests via a "robotium" mechanism is a little
> confusing, though, and we should probably clarify that in comments.

[klundberg, 2015-01-06 18:30:21]

On 2015/01/06 18:25:57, jbudorick wrote:
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> File build/android/pylib/remote/device/dummy/dummy.gyp (right):
> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> build/android/pylib/remote/device/dummy/dummy.gyp:5: # Running Robotium tests
on
> AMP requires both an "app" APK and a "test" APK
> On 2015/01/06 18:17:31, klundberg wrote:
> > "Robotium" can be a little confusing as we won't actually run Robotium tests
> at
> > all. I know that is what AMP uses but maybe you could make this comment a
> little
> > less "implementation specific" to avoid confusion?
> 
> The issue with that is that the entire reason for this target _is_
> implementation specific - AMP's "robotium" test type _requires_ an app APK and
a
> test APK.
> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> File build/android/pylib/remote/device/remote_device_gtest_run.py (right):
> 
>
https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
> build/android/pylib/remote/device/remote_device_gtest_run.py:47: if
runner_type
> == 'robotium':
> On 2015/01/06 18:17:31, klundberg wrote:
> > Similar comment about using "robotium" here. Seems confusing.
> > 
> > And I'm sorry I seemed to have missed this in a previous review as this file
> is
> > already using robotium.
> 
> The implementation _is_ specific to "robotium" tests on AMP. While it's true
> that these aren't robotium tests, they are running in AMP's robotium
mechanism,
> and this script code is specifically written for that. Naming it this way
seemed
> the least confusing way of doing things.
> 
> I understand that running gtests via a "robotium" mechanism is a little
> confusing, though, and we should probably clarify that in comments.

I understand that the reasoning for this is implementation specific but we could
use another term to make this less implementation specific in our code.
As I understand it, AMP only support monkey and robotium for Android?
In that case, we could replace robotium with something like "amp" or
"amp_functional" or something.

The concerns with using robotium are 1. that we have no specified anywhere what
robotium is at all and 2. Robotium is an Android test framework that we are not
using at all.
1. can be solved by comments but I'm more concerned about 2 and would prefer to
use another term if possible.

[jbudorick, 2015-01-06 18:41:26]

Hm. The relevant part of the robotium test type, as far as we're concerned, is
that it starts tests via am instrument. What if I renamed everything that
doesn't explicitly _have_ to be robotium to something that implies that am
instrument is the relevant part?

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_test_run.py (right):

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:145: self._test_id =
self._UploadTestToDevice('robotium', test_path)
We have to explicitly use 'robotium' both here, when we upload the test...

https://codereview.chromium.org/833403002/diff/20001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:148:
self._SetTestConfig('robotium', config)
... and here, when we upload the config.

[jbudorick, 2015-01-06 19:24:39]

Removed robotium references where possible. wdyt?

[rnephew (Wrong account), 2015-01-06 19:30:40]

rnephew@google.com changed reviewers:
+ rnephew@google.com

[rnephew (Wrong account), 2015-01-06 19:30:41]

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_gtest_run.py (right):

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_gtest_run.py:40:
self.DEFAULT_RUNNER_TYPE)
I know this isn't your mistake, but I just noticed this is using
DEFAULT_RUNNER_TYPE when it should be using DEFAULT_RUNNER_PACKAGE and since you
got rid of runner_type this could cause issues (other than the fact that its
just wrong).

[klundberg, 2015-01-06 19:33:02]

Minor thing that Randy already commented on. Otherwise LGTM

Thanks for making the changes, John!!

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_gtest_run.py (right):

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_gtest_run.py:40:
self.DEFAULT_RUNNER_TYPE)
On 2015/01/06 19:30:41, rnephew wrote:
> I know this isn't your mistake, but I just noticed this is using
> DEFAULT_RUNNER_TYPE when it should be using DEFAULT_RUNNER_PACKAGE and since
you
> got rid of runner_type this could cause issues (other than the fact that its
> just wrong).

I was just about to comment on this too :-)

[cjhopman, 2015-01-06 19:34:27]

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/dummy/dummy.gyp (right):

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/dummy/dummy.gyp:12: 'target_name':
'remote_device_dummy_apk',
Please also add the corresponding GN build stuff for this (ping me if any part
of doing that isn't obvious).

You should be able to test with something like:

gn gen '--args=os="android" use_goma=true is_debug=true' out-gn/mobile
ninja -C out-gn/mobile base_unittests_apk

[jbudorick, 2015-01-06 20:23:53]

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/dummy/dummy.gyp (right):

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/dummy/dummy.gyp:12: 'target_name':
'remote_device_dummy_apk',
On 2015/01/06 19:34:27, cjhopman wrote:
> Please also add the corresponding GN build stuff for this (ping me if any part
> of doing that isn't obvious).
> 
> You should be able to test with something like:
> 
> gn gen '--args=os="android" use_goma=true is_debug=true' out-gn/mobile
> ninja -C out-gn/mobile base_unittests_apk

Done.

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_gtest_run.py (right):

https://codereview.chromium.org/833403002/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_gtest_run.py:40:
self.DEFAULT_RUNNER_TYPE)
On 2015/01/06 19:33:02, klundberg wrote:
> On 2015/01/06 19:30:41, rnephew wrote:
> > I know this isn't your mistake, but I just noticed this is using
> > DEFAULT_RUNNER_TYPE when it should be using DEFAULT_RUNNER_PACKAGE and since
> you
> > got rid of runner_type this could cause issues (other than the fact that its
> > just wrong).
> 
> I was just about to comment on this too :-)

Done.

[cjhopman, 2015-01-06 20:58:34]

lgtm

[jbudorick, 2015-01-06 21:22:44]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-01-06 21:23:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/833403002/60001

[commit-bot: I haz the power, 2015-01-06 22:27:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-06 22:28:09]

Patchset 4 (id:??) landed as
https://crrev.com/335cbe5177b3d0ec5d5564b646cb1a7b814c26d2
Cr-Commit-Position: refs/heads/master@{#310159}