Android: makes "apk_package" optional in adb_install_apk.py

Android: makes "apk_package" optional in adb_install_apk.py

We can derive the package name from the mandatory apk.

BUG=
TEST=adb_install_apk.py --apk ContentShell.apk


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=165450

[bulach, 2012-10-31 13:47:08]

thanks to frank for touching the apk_info.py :) it reminded me we could simplify
the adb_install_apk.py..
quick look please?

[klundberg, 2012-10-31 15:45:36]

lgtm

[frankf, 2012-10-31 18:04:26]

LGTM with some nits. Thanks.

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
File build/android/adb_install_apk.py (right):

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:15: from pylib import constants
Move this up so it's alphabetical.

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:29:
test_options_parser.AddInstallAPKOption(parser)
Hmm. 'test_options_parser' has become a misnomer since this has nothing to do
with test :)

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:41: 'apks', options.apk)
optional: Maybe move this to function similar to ValidInstrumentationOptions in
test_options_parser (and allow absolute path like options.test_apk). Ideally the
assumption on directory structure should be centralized so if it changes, we
don't need to modify a lot of scripts.

[bulach, 2012-10-31 18:44:20]

thanks frank! comments addressed (except test_options_parser renaming), another
look please?

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
File build/android/adb_install_apk.py (right):

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:15: from pylib import constants
On 2012/10/31 18:04:26, frankf wrote:
> Move this up so it's alphabetical.

Done.

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:29:
test_options_parser.AddInstallAPKOption(parser)
On 2012/10/31 18:04:26, frankf wrote:
> Hmm. 'test_options_parser' has become a misnomer since this has nothing to do
> with test :)

indeed! :) too big a change for this patch, but I can try to do separately.

http://codereview.chromium.org/11365004/diff/2001/build/android/adb_install_a...
build/android/adb_install_apk.py:41: 'apks', options.apk)
On 2012/10/31 18:04:26, frankf wrote:
> optional: Maybe move this to function similar to ValidInstrumentationOptions
in
> test_options_parser (and allow absolute path like options.test_apk). Ideally
the
> assumption on directory structure should be centralized so if it changes, we
> don't need to modify a lot of scripts.

good idea! done..

[frankf, 2012-10-31 18:51:10]

http://codereview.chromium.org/11365004/diff/4002/build/android/pylib/test_op...
File build/android/pylib/test_options_parser.py (right):

http://codereview.chromium.org/11365004/diff/4002/build/android/pylib/test_op...
build/android/pylib/test_options_parser.py:40: if not
os.path.exists(options.apk):
Can you verify that options.apk and options.build_type are not None and throw an
optionparse error.

[bulach, 2012-10-31 19:00:08]

thanks frank! comment addressed, ptal

http://codereview.chromium.org/11365004/diff/4002/build/android/pylib/test_op...
File build/android/pylib/test_options_parser.py (right):

http://codereview.chromium.org/11365004/diff/4002/build/android/pylib/test_op...
build/android/pylib/test_options_parser.py:40: if not
os.path.exists(options.apk):
On 2012/10/31 18:51:10, frankf wrote:
> Can you verify that options.apk and options.build_type are not None and throw
an
> optionparse error.

good point. since there's only one user and it'd be required anyways, I called
AddBuildTypeOption() on AddInstallAPKOption() so that build_type is guaranteed
to be set..

added a test for the apk being None.

[frankf, 2012-10-31 19:04:12]

lgtm

Thanks!

[commit-bot: I haz the power, 2012-11-01 09:55:21]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[bulach, 2012-11-01 10:18:29]

+jknotten for full committer power :)

[John Knottenbelt, 2012-11-01 16:59:59]

lgtm

[commit-bot: I haz the power, 2012-11-01 17:00:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bulach@chromium.org/11365004/11001

[commit-bot: I haz the power, 2012-11-01 18:55:28]

Change committed as 165450