[android] Instumentation tests determine whether to install test apk based on Md5Sum.

[android] Instumentation tests determine whether to install the test apk based on Md5Sum.

This deprecates the -I switch to instrumention tests.

BUG=260084
TEST=None
NOTRY=True

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213956

[frankf, 2013-07-19 21:53:54]

https://codereview.chromium.org/19799003/diff/2001/build/android/pylib/instru...
File build/android/pylib/instrumentation/test_runner.py (right):

https://codereview.chromium.org/19799003/diff/2001/build/android/pylib/instru...
build/android/pylib/instrumentation/test_runner.py:93: if not
self.adb.CheckMd5Sum(self.test_pkg.GetApkPath(), installed_apk_path):
This seems generic enough to be part of *Install methods in android_commands.py
as opposed to something the caller have to explicitly do.

https://codereview.chromium.org/19799003/diff/2001/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/19799003/diff/2001/build/android/test_runner....
build/android/test_runner.py:280: help='This switch has no effect.')
Follow the DEPRECATED used elsewhere in this file.

[craigdh, 2013-07-19 23:38:37]

https://codereview.chromium.org/19799003/diff/2001/build/android/pylib/instru...
File build/android/pylib/instrumentation/test_runner.py (right):

https://codereview.chromium.org/19799003/diff/2001/build/android/pylib/instru...
build/android/pylib/instrumentation/test_runner.py:93: if not
self.adb.CheckMd5Sum(self.test_pkg.GetApkPath(), installed_apk_path):
On 2013/07/19 21:53:54, frankf wrote:
> This seems generic enough to be part of *Install methods in
android_commands.py
> as opposed to something the caller have to explicitly do.

Done.

https://codereview.chromium.org/19799003/diff/2001/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/19799003/diff/2001/build/android/test_runner....
build/android/test_runner.py:280: help='This switch has no effect.')
On 2013/07/19 21:53:54, frankf wrote:
> Follow the DEPRECATED used elsewhere in this file.

Done.

[frankf, 2013-07-20 02:18:35]

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/andro...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/andro...
build/android/pylib/android_commands.py:421: return 'Success: identical %s apk
already installed' % package_name
Why do we return a string in this method?

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_package.py (right):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:43: # Install apk to be
tested.
a debugging session?

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_runner.py (left):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_runner.py:89: self.install_apk =
options.install_apk
Rebase

[craigdh, 2013-07-22 19:03:14]

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/andro...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/andro...
build/android/pylib/android_commands.py:421: return 'Success: identical %s apk
already installed' % package_name
On 2013/07/20 02:18:35, frankf wrote:
> Why do we return a string in this method?

Yeah, I thought that was odd too. Doesn't look like anyone uses it so removed.

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_package.py (right):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:43: # Install apk to be
tested.
On 2013/07/20 02:18:35, frankf wrote:
> a debugging session?

This installs the actual apk to be tested by removing Test from the apk name and
.tests from the package name. It uses the _rreplace() because the standard
library doesn't include a function to replace one occurrence from the end of the
string (just the beginning).

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_runner.py (left):

https://codereview.chromium.org/19799003/diff/11001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_runner.py:89: self.install_apk =
options.install_apk
On 2013/07/20 02:18:35, frankf wrote:
> Rebase

Done.

[frankf, 2013-07-22 20:27:04]

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/andro...
File build/android/pylib/android_commands.py (left):

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/andro...
build/android/pylib/android_commands.py:429: return install_status
Make sure callers don't depend on the return value

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_package.py (right):

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:14: def _rreplace(s, old,
new, occurrence=1):
Name style issue

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:17: 
2 Blank lines

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:44: apk_path =
_rreplace(self.GetApkPath(), 'Test.apk', '.apk')
This is not in the CL description. You also need to update the buildbot code not
to install the app separately. Perhaps do this in a separate CL.

[craigdh, 2013-07-22 23:50:24]

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/andro...
File build/android/pylib/android_commands.py (left):

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/andro...
build/android/pylib/android_commands.py:429: return install_status
On 2013/07/22 20:27:04, frankf wrote:
> Make sure callers don't depend on the return value

Already checked, they don't.

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_package.py (right):

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:14: def _rreplace(s, old,
new, occurrence=1):
On 2013/07/22 20:27:04, frankf wrote:
> Name style issue

Done.

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:17: 
On 2013/07/22 20:27:04, frankf wrote:
> 2 Blank lines

Done.

https://codereview.chromium.org/19799003/diff/17001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:44: apk_path =
_rreplace(self.GetApkPath(), 'Test.apk', '.apk')
On 2013/07/22 20:27:04, frankf wrote:
> This is not in the CL description. You also need to update the buildbot code
not
> to install the app separately. Perhaps do this in a separate CL.

Yep I was planning to do that separately. It's not a huge issue as the md5sum
will prevent it from being pushed twice in the meantime.

[craigdh, 2013-07-25 18:16:58]

ptal.

[frankf, 2013-07-25 18:31:02]

lgtm

https://codereview.chromium.org/19799003/diff/42001/build/android/pylib/instr...
File build/android/pylib/instrumentation/test_package.py (right):

https://codereview.chromium.org/19799003/diff/42001/build/android/pylib/instr...
build/android/pylib/instrumentation/test_package.py:8: import re
remove this

https://codereview.chromium.org/19799003/diff/42001/build/android/test_runner.py
File build/android/test_runner.py (right):

https://codereview.chromium.org/19799003/diff/42001/build/android/test_runner...
build/android/test_runner.py:279: help='(DEPRECATED) Install the test apk.')
Make sure to update the docs

[craigdh, 2013-07-25 18:37:48]

+bulach for buildbot OWNERS

[bulach, 2013-07-26 08:57:05]

lgtm, very nice, glad to see this going!

my impression is that we've just switched downstream to use bb_device_steps.py,
so you can probably remove the todo and the option right now. siva, could you
confirm?

[Siva Chandra, 2013-07-26 13:36:16]

On 2013/07/26 08:57:05, bulach wrote:
> lgtm, very nice, glad to see this going!
> 
> my impression is that we've just switched downstream to use
bb_device_steps.py,
> so you can probably remove the todo and the option right now. siva, could you
> confirm?

All official builders downstream bots still use the old scripts. I am working on
moving them to the new scripts. We will need to keep the option until then, or
remove from the old scripts and then remove it from here.

[commit-bot: I haz the power, 2013-07-26 17:47:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/craigdh@chromium.org/19799003/52001

[commit-bot: I haz the power, 2013-07-26 18:31:21]

Change committed as 213956