Adds AndroidWebView and ChromiumTestShell tests to the Android FYI bot.

Adds AndroidWebView and ChromiumTestShell tests to the Android FYI bot.

TEST=Ran the changes through the Android FYI (android_fyi_dbg) trybot.
BUG=152492
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=159768

[klundberg, 2012-10-02 04:10:58]

Ran this through the trybot:
http://build.chromium.org/p/tryserver.chromium/builders/android_fyi_dbg_trigg...

And in general, everything seems to work although some of the AndroidWebView
tests are failing (but that is somewhat expected).

As mentioned in an offline thread, I think we should mark the failing test flaky
and mark the bot red whenever any instrumentation tests are failing. 
This means that we should probably not submit this not until the tests have been
marked flaky. 

Martin and Marcus, please feel free to patch this in, make any necessary changes
or submit directly if you want...although Isaac probably has some input although
I discussed some of this with him. 

Thanks!
Karin

[Isaac (away), 2012-10-02 05:13:37]

This looks generally good.  But do we really need 12 separate instrumentation
test steps?  What do they all do?

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
File build/android/buildbot/buildbot_functions.sh (right):

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:224: if (( $TEST_RUN_EXIT_CODE != 0
)); then
use a [[ ]] test here

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:229: return 0
return is not technically necessary here but it's fine for clarity.

[bulach, 2012-10-02 10:40:46]

some drive-by comments:

http://codereview.chromium.org/10993069/diff/30005/build/android/adb_install_...
File build/android/adb_install_apk.py (right):

http://codereview.chromium.org/10993069/diff/30005/build/android/adb_install_...
build/android/adb_install_apk.py:48: p.join()
would something like this work?

all_args = [(device, options) for device in devices]
p = multiprocessing.Pool(len(devices))
p.map(InstallApk, all_args)

I reckon it can replace 40-48..

http://codereview.chromium.org/10993069/diff/30005/build/android/adb_install_...
build/android/adb_install_apk.py:49: 
nit: need an extra \n here

http://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bui...
File build/android/buildbot/buildbot_functions.sh (right):

http://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bui...
build/android/buildbot/buildbot_functions.sh:221: set +e
I'm not terribly familiar with bash, but is a "set -e" needed for symmetry?

http://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bui...
build/android/buildbot/buildbot_functions.sh:236: #   $2: EXTRA_FLAGS to be
passed to run_instrumentation_tests.py.
nit: $3: ... ? :)

http://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bui...
build/android/buildbot/buildbot_functions.sh:254: function
bb_run_instrumentation_tests {
hmm, these three methods sound too similar..
would these be any clearer?
bb_run_instrumentation_test_internal
bb_run_all_instrumentation_tests_for_apk
bb_run_all_instrumentation_tests

[klundberg, 2012-10-02 14:10:30]

Please hold off reviewing this. Looks like Martin is trying to get his changes
in.

[klundberg, 2012-10-02 18:40:06]

I talked to Martin this morning and we agreed to try to get this in since his
patch required several changes and this might be closer to what we want.

I have addressed people's comments, including some comments from an offline
discussion with Isaac.

Please review as we would like to get this in today and start running these
tests upstream.

FYI, my most recent try job failed for the AndroidWebView tests but it didn't
pick up Martin's recent change to mark these flaky. I'm sending another try job
with the needed revision to pick up Martin's change just to make sure that
everything will work once this is submitted.

https://codereview.chromium.org/10993069/diff/30005/build/android/adb_install...
File build/android/adb_install_apk.py (right):

https://codereview.chromium.org/10993069/diff/30005/build/android/adb_install...
build/android/adb_install_apk.py:48: p.join()
On 2012/10/02 10:40:46, bulach wrote:
> would something like this work?
> 
> all_args = [(device, options) for device in devices]
> p = multiprocessing.Pool(len(devices))
> p.map(InstallApk, all_args)
> 
> I reckon it can replace 40-48..

Done.

https://codereview.chromium.org/10993069/diff/30005/build/android/adb_install...
build/android/adb_install_apk.py:49: 
On 2012/10/02 10:40:46, bulach wrote:
> nit: need an extra \n here

Done.

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
File build/android/buildbot/buildbot_functions.sh (right):

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:221: set +e
On 2012/10/02 10:40:46, bulach wrote:
> I'm not terribly familiar with bash, but is a "set -e" needed for symmetry?

I discussed this with Isaac and we don't need "set -e" since this is now a
separate process and "set +e" only affects what is inside the parentheses.

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:224: if (( $TEST_RUN_EXIT_CODE != 0
)); then
On 2012/10/02 05:13:37, Isaac wrote:
> use a [[ ]] test here

Done.

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:229: return 0
On 2012/10/02 05:13:37, Isaac wrote:
> return is not technically necessary here but it's fine for clarity.

Done.

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:236: #   $2: EXTRA_FLAGS to be
passed to run_instrumentation_tests.py.
On 2012/10/02 10:40:46, bulach wrote:
> nit: $3: ... ? :)

Done.

https://codereview.chromium.org/10993069/diff/30005/build/android/buildbot/bu...
build/android/buildbot/buildbot_functions.sh:254: function
bb_run_instrumentation_tests {
On 2012/10/02 10:40:46, bulach wrote:
> hmm, these three methods sound too similar..
> would these be any clearer?
> bb_run_instrumentation_test_internal
> bb_run_all_instrumentation_tests_for_apk
> bb_run_all_instrumentation_tests

I discussed this with Isaac and since we don't care about test sizes anymore, I
have simplified the scripts.

[Isaac (away), 2012-10-02 19:29:59]

lgtm w/ minor nits on python script

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
File build/android/adb_install_apk.py (right):

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
build/android/adb_install_apk.py:17: def InstallApk(args):
suggest changing this to:

def InstallApk(options, device)

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
build/android/adb_install_apk.py:44: pool.map(InstallApk, zip([options] *
len(devices), devices))
change this to pool.imap and you can eliminate the zip.

[klundberg, 2012-10-02 21:01:02]

Will submit shortly so last chance to object :-)

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
File build/android/adb_install_apk.py (right):

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
build/android/adb_install_apk.py:17: def InstallApk(args):
On 2012/10/02 19:29:59, Isaac wrote:
> suggest changing this to:
> 
> def InstallApk(options, device)

As discussed in person, just changing the line below.

https://codereview.chromium.org/10993069/diff/21010/build/android/adb_install...
build/android/adb_install_apk.py:44: pool.map(InstallApk, zip([options] *
len(devices), devices))
On 2012/10/02 19:29:59, Isaac wrote:
> change this to pool.imap and you can eliminate the zip.

As discussed (and tried) in person, this doesn't work.

[Isaac (away), 2012-10-02 21:03:42]

objection!  Please get to patchset numbers >30

Still lgtm :-)