[Android] Make generated test scripts pass apk paths.

[Android] Make generated test scripts pass apk paths.

Platform mode uses explicit paths to APKs rather than the current system
of the name + the output directory. This is also a prelude to installing
an additional apk for instrumentation tests that will use the embedded
test server.

BUG=

Committed: https://crrev.com/7873c6dcee70af978571d9bdf12cf650ef296d57
Cr-Commit-Position: refs/heads/master@{#360922}

[jbudorick, 2015-11-19 17:32:42]

jbudorick@chromium.org changed reviewers:
+ agrieve@chromium.org

[agrieve, 2015-11-19 19:50:51]

just nits. lgtm

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/in...
build/config/android/internal_rules.gni:457: rebased_apk_path =
rebase_path(invoker.apk_path, root_build_dir)
nit: rebased_apk_path -> _rebased_apk_path

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:1832: build_config =
"${target_gen_dir}/${target_name}.build_config"
nit: call this _build_config here, and do an explicit build_config =
_build_config within test_runner_script so that it's obvious which template it
applies to.

Actually - might be better to have instrumentation_test derive this from
android_apk. Maybe pass android_apk_target as a parameter and use
get_label_info() to build its build_config path?

https://codereview.chromium.org/1465523002/diff/20001/chrome/chrome_public_te...
File chrome/chrome_public_test_apk.isolate (left):

https://codereview.chromium.org/1465523002/diff/20001/chrome/chrome_public_te...
chrome/chrome_public_test_apk.isolate:10:
'<(DEPTH)/chrome/test/data/android/about.html',
is this change relevant to the CL?

[jbudorick, 2015-11-20 03:16:23]

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/in...
build/config/android/internal_rules.gni:457: rebased_apk_path =
rebase_path(invoker.apk_path, root_build_dir)
On 2015/11/19 19:50:51, agrieve wrote:
> nit: rebased_apk_path -> _rebased_apk_path 

Done.

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:1832: build_config =
"${target_gen_dir}/${target_name}.build_config"
On 2015/11/19 19:50:51, agrieve wrote:
> nit: call this _build_config here, and do an explicit build_config =
> _build_config within test_runner_script so that it's obvious which template it
> applies to.

Done.

> 
> Actually - might be better to have instrumentation_test derive this from
> android_apk. Maybe pass android_apk_target as a parameter and use
> get_label_info() to build its build_config path?

I'm not sure I follow how that would work, and even after playing around with
what I thought you meant I still felt a bit lost... I'd like to pull the
build_config up from the android_apk invocation below, but I don't see how
get_label_info (and its restricted set of whats) would help here. Care to
elaborate?

https://codereview.chromium.org/1465523002/diff/20001/chrome/chrome_public_te...
File chrome/chrome_public_test_apk.isolate (left):

https://codereview.chromium.org/1465523002/diff/20001/chrome/chrome_public_te...
chrome/chrome_public_test_apk.isolate:10:
'<(DEPTH)/chrome/test/data/android/about.html',
On 2015/11/19 19:50:51, agrieve wrote:
> is this change relevant to the CL? 

oops, not any more. I'll back this out, though I anticipate doing it in the
future when it's otherwise relevant.

[agrieve, 2015-11-20 03:50:49]

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:1832: build_config =
"${target_gen_dir}/${target_name}.build_config"
On 2015/11/20 03:16:23, jbudorick wrote:
> On 2015/11/19 19:50:51, agrieve wrote:
> > nit: call this _build_config here, and do an explicit build_config =
> > _build_config within test_runner_script so that it's obvious which template
it
> > applies to.
> 
> Done.
> 
> > 
> > Actually - might be better to have instrumentation_test derive this from
> > android_apk. Maybe pass android_apk_target as a parameter and use
> > get_label_info() to build its build_config path?
> 
> I'm not sure I follow how that would work, and even after playing around with
> what I thought you meant I still felt a bit lost... I'd like to pull the
> build_config up from the android_apk invocation below, but I don't see how
> get_label_info (and its restricted set of whats) would help here. Care to
> elaborate?

Feel free to submit as-is, and I can have a look tomorrow to see if this is
possible. (otherwise I'll have a look tomorrow anyways :P)

[agrieve, 2015-11-20 16:25:19]

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
build/config/android/rules.gni:1832: build_config =
"${target_gen_dir}/${target_name}.build_config"
On 2015/11/20 03:50:49, agrieve wrote:
> On 2015/11/20 03:16:23, jbudorick wrote:
> > On 2015/11/19 19:50:51, agrieve wrote:
> > > nit: call this _build_config here, and do an explicit build_config =
> > > _build_config within test_runner_script so that it's obvious which
template
> it
> > > applies to.
> > 
> > Done.
> > 
> > > 
> > > Actually - might be better to have instrumentation_test derive this from
> > > android_apk. Maybe pass android_apk_target as a parameter and use
> > > get_label_info() to build its build_config path?
> > 
> > I'm not sure I follow how that would work, and even after playing around
with
> > what I thought you meant I still felt a bit lost... I'd like to pull the
> > build_config up from the android_apk invocation below, but I don't see how
> > get_label_info (and its restricted set of whats) would help here. Care to
> > elaborate?
> 
> Feel free to submit as-is, and I can have a look tomorrow to see if this is
> possible. (otherwise I'll have a look tomorrow anyways :P)

Okay, have a look at the instrumentation_test_apk and test_runner_script changes
here:
https://codereview.chromium.org/1462363002

(I patched yours in, made the changes, then uploaded)

[jbudorick, 2015-11-20 18:15:49]

On 2015/11/20 16:25:19, agrieve wrote:
>
https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
> File build/config/android/rules.gni (right):
> 
>
https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
> build/config/android/rules.gni:1832: build_config =
> "${target_gen_dir}/${target_name}.build_config"
> On 2015/11/20 03:50:49, agrieve wrote:
> > On 2015/11/20 03:16:23, jbudorick wrote:
> > > On 2015/11/19 19:50:51, agrieve wrote:
> > > > nit: call this _build_config here, and do an explicit build_config =
> > > > _build_config within test_runner_script so that it's obvious which
> template
> > it
> > > > applies to.
> > > 
> > > Done.
> > > 
> > > > 
> > > > Actually - might be better to have instrumentation_test derive this from
> > > > android_apk. Maybe pass android_apk_target as a parameter and use
> > > > get_label_info() to build its build_config path?
> > > 
> > > I'm not sure I follow how that would work, and even after playing around
> with
> > > what I thought you meant I still felt a bit lost... I'd like to pull the
> > > build_config up from the android_apk invocation below, but I don't see how
> > > get_label_info (and its restricted set of whats) would help here. Care to
> > > elaborate?
> > 
> > Feel free to submit as-is, and I can have a look tomorrow to see if this is
> > possible. (otherwise I'll have a look tomorrow anyways :P)
> 
> Okay, have a look at the instrumentation_test_apk and test_runner_script
changes
> here:
> https://codereview.chromium.org/1462363002
> 
> (I patched yours in, made the changes, then uploaded)

oh, ok. I didn't realize you meant doing this in the test_runner_script
template.

Merged in your changes in the latest patchset.

[agrieve, 2015-11-20 18:26:01]

On 2015/11/20 18:15:49, jbudorick wrote:
> On 2015/11/20 16:25:19, agrieve wrote:
> >
>
https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
> > File build/config/android/rules.gni (right):
> > 
> >
>
https://codereview.chromium.org/1465523002/diff/20001/build/config/android/ru...
> > build/config/android/rules.gni:1832: build_config =
> > "${target_gen_dir}/${target_name}.build_config"
> > On 2015/11/20 03:50:49, agrieve wrote:
> > > On 2015/11/20 03:16:23, jbudorick wrote:
> > > > On 2015/11/19 19:50:51, agrieve wrote:
> > > > > nit: call this _build_config here, and do an explicit build_config =
> > > > > _build_config within test_runner_script so that it's obvious which
> > template
> > > it
> > > > > applies to.
> > > > 
> > > > Done.
> > > > 
> > > > > 
> > > > > Actually - might be better to have instrumentation_test derive this
from
> > > > > android_apk. Maybe pass android_apk_target as a parameter and use
> > > > > get_label_info() to build its build_config path?
> > > > 
> > > > I'm not sure I follow how that would work, and even after playing around
> > with
> > > > what I thought you meant I still felt a bit lost... I'd like to pull the
> > > > build_config up from the android_apk invocation below, but I don't see
how
> > > > get_label_info (and its restricted set of whats) would help here. Care
to
> > > > elaborate?
> > > 
> > > Feel free to submit as-is, and I can have a look tomorrow to see if this
is
> > > possible. (otherwise I'll have a look tomorrow anyways :P)
> > 
> > Okay, have a look at the instrumentation_test_apk and test_runner_script
> changes
> > here:
> > https://codereview.chromium.org/1462363002
> > 
> > (I patched yours in, made the changes, then uploaded)
> 
> oh, ok. I didn't realize you meant doing this in the test_runner_script
> template.
> 
> Merged in your changes in the latest patchset.

lgtm

[jbudorick, 2015-11-20 21:14:53]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-11-20 21:14:53]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/1465523002/#ps100001
(title: "rebase on pkotwicz's emma change")

[commit-bot: I haz the power, 2015-11-20 21:15:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1465523002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1465523002/100001

[commit-bot: I haz the power, 2015-11-20 21:47:30]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-11-20 21:48:11]

Patchset 6 (id:??) landed as
https://crrev.com/7873c6dcee70af978571d9bdf12cf650ef296d57
Cr-Commit-Position: refs/heads/master@{#360922}