Add required deps to third_party/android_support_test_runner

Add required deps to third_party/android_support_test_runner

BUG=627916
Committed: https://crrev.com/24da3c36fd807a0003bece9c352567ad637f810e
Cr-Commit-Position: refs/heads/master@{#414150}

[aluo, 2016-07-13 19:15:52]

aluo@chromium.org changed reviewers:
+ yolandyan@chromium.org

[aluo, 2016-07-13 19:15:52]

Fixed android support test runner to work with tests that use
AndroidJUnitRunner.

[the real yoland, 2016-07-13 19:24:55]

On 2016/07/13 at 19:15:52, aluo wrote:
> Fixed android support test runner to work with tests that use
AndroidJUnitRunner.

Hmm, the reason we don't use AJunitRunner is because we have stuff needed to
inherit from BaseInstrumentationTestRunner (e.g. things to recognize annotations
like @DisableIf)
So I added this CL (https://codereview.chromium.org/2146713003) to support
keeping track of Activity life cycler for Espresso tests

[aluo, 2016-07-15 01:55:34]

On 2016/07/13 19:24:55, the real yoland wrote:
> On 2016/07/13 at 19:15:52, aluo wrote:
> > Fixed android support test runner to work with tests that use
> AndroidJUnitRunner.
> 
> Hmm, the reason we don't use AJunitRunner is because we have stuff needed to
> inherit from BaseInstrumentationTestRunner (e.g. things to recognize
annotations
> like @DisableIf)
> So I added this CL (https://codereview.chromium.org/2146713003) to support
> keeping track of Activity life cycler for Espresso tests

After playing around with the runner in
(https://codereview.chromium.org/2146713003) for a while, I still couldn't get
it to work properly for tests that use espresso-intent.  This is due to missing
IntentMonitorRegistry logic (it's more involved to hook it up correctly, see
MonitoringInstrumentation.java in android.support.test.runner).  I believe it'd
be simpler to use the AndroidJUnitRunner for espresso tests.

[the real yoland, 2016-08-03 23:05:09]

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
File third_party/android_support_test_runner/BUILD.gn (right):

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:8: jar_path =
"lib/runner-0.5-release-no-dep.jar"
I don't think I was able to find 0.5 here

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:20:
android_java_prebuilt("junit_java") {
+mikecase is adding junit4.12 to //third_party/junit, so we can just junit from
soon

[aluo, 2016-08-05 22:00:16]

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
File third_party/android_support_test_runner/BUILD.gn (right):

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:8: jar_path =
"lib/runner-0.5-release-no-dep.jar"
On 2016/08/03 23:05:09, the real yoland wrote:
> I don't think I was able to find 0.5 here

Done.  I uploaded these to the bucket

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:20:
android_java_prebuilt("junit_java") {
On 2016/08/03 23:05:09, the real yoland wrote:
> +mikecase is adding junit4.12 to //third_party/junit, so we can just junit
from
> soon

I'll wait for junit to be updated and modify this.

[the real yoland, 2016-08-05 22:16:58]

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
File third_party/android_support_test_runner/BUILD.gn (right):

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:8: jar_path =
"lib/runner-0.5-release-no-dep.jar"
On 2016/08/05 at 22:00:16, aluo wrote:
> On 2016/08/03 23:05:09, the real yoland wrote:
> > I don't think I was able to find 0.5 here
> 
> Done.  I uploaded these to the bucket

Got it, in that case, you want to also add the change to the sha1 file (and the
new sha1 file for exposed_instrumentation_api_publish_java jar)

[aluo, 2016-08-05 22:23:05]

On 2016/08/05 22:16:58, the real yoland wrote:
>
https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
> File third_party/android_support_test_runner/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
> third_party/android_support_test_runner/BUILD.gn:8: jar_path =
> "lib/runner-0.5-release-no-dep.jar"
> On 2016/08/05 at 22:00:16, aluo wrote:
> > On 2016/08/03 23:05:09, the real yoland wrote:
> > > I don't think I was able to find 0.5 here
> > 
> > Done.  I uploaded these to the bucket
> 
> Got it, in that case, you want to also add the change to the sha1 file (and
the
> new sha1 file for exposed_instrumentation_api_publish_java jar)

ok, I uploaded the exposed instrumentation api jar+sha1 too.

[mikecase (-- gone --), 2016-08-19 20:36:21]

mikecase@chromium.org changed reviewers:
+ mikecase@chromium.org

[mikecase (-- gone --), 2016-08-19 20:36:21]

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
File third_party/android_support_test_runner/BUILD.gn (right):

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:8: jar_path =
"lib/runner-0.5-release-no-dep.jar"
On 2016/08/05 at 22:16:57, the real yoland wrote:
> On 2016/08/05 at 22:00:16, aluo wrote:
> > On 2016/08/03 23:05:09, the real yoland wrote:
> > > I don't think I was able to find 0.5 here
> > 
> > Done.  I uploaded these to the bucket
> 
> Got it, in that case, you want to also add the change to the sha1 file (and
the new sha1 file for exposed_instrumentation_api_publish_java jar)

Yeah, still need to update sha1 hash
Add file...

lib/runner-0.5-release-no-dep.jar.sha1

and I guess remove

lib/runner-0.3-release-no-dep.jar.sha1

https://codereview.chromium.org/2142413004/diff/1/third_party/android_support...
third_party/android_support_test_runner/BUILD.gn:20:
android_java_prebuilt("junit_java") {
On 2016/08/05 at 22:00:16, aluo wrote:
> On 2016/08/03 23:05:09, the real yoland wrote:
> > +mikecase is adding junit4.12 to //third_party/junit, so we can just junit
from
> > soon
> 
> I'll wait for junit to be updated and modify this.

junit 4.12 should be available now via //third_party/junit:junit I think
https://codereview.chromium.org/2076783004

[aluo, 2016-08-22 23:35:20]

The CQ bit was checked by aluo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-22 23:36:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mikecase (-- gone --), 2016-08-22 23:42:18]

ty for updating CL :D

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:259: parser.add_option(
If you take my suggestions to internal_rules.gni, you shouldn't have to change
this file at all.

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:260: '--use_host_bootclasspath',
--use-host-bootclasspath

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:261: type='int',
Remove type='int'

Instead just action="store_true". Then if --use-host-bootclasspath is specified
the varible args.use_host_bootclasspath will just be true.

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:263: 'specified.')
indenting (should be aligned with text above)

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

https://codereview.chromium.org/2142413004/diff/20001/build/config/android/in...
build/config/android/internal_rules.gni:2033: args += [
"--bootclasspath=$_rebased_android_sdk_ijar" ]
I would probably just add a flag that causes this not to be set.

if not android_use_host_jdk:
   args += [ "--bootclasspath=$_rebased_android_sdk_ijar" ]


 or something

https://codereview.chromium.org/2142413004/diff/20001/third_party/android_sup...
File
third_party/android_support_test_runner/lib/runner-0.5-release-no-dep.jar.sha1
(right):

https://codereview.chromium.org/2142413004/diff/20001/third_party/android_sup...
third_party/android_support_test_runner/lib/runner-0.5-release-no-dep.jar.sha1:1:
796f9229948e907471c5595fbe94655ccf03ff73
You can delete the old sha1 file right?

[aluo, 2016-08-23 00:37:56]

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

[aluo, 2016-08-23 00:37:57]

Addressed Mike's comments, also adding agrieve@ as owner reviewer for
internal_rules.gni

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:259: parser.add_option(
On 2016/08/22 23:42:17, mikecase wrote:
> If you take my suggestions to internal_rules.gni, you shouldn't have to change
> this file at all.

There's logic in this script to set -src and -target to java 1.7 instead of
whatever is on the host if a --bootclasspath is present.  We still need this to
be set otherwise on my machine which is using java 1.8, I get errors related to
javac versions when creating the dex file.  So that's why I made it a separate
option.

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:260: '--use_host_bootclasspath',
On 2016/08/22 23:42:17, mikecase wrote:
> --use-host-bootclasspath

Acknowledged.

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:261: type='int',
On 2016/08/22 23:42:17, mikecase wrote:
> Remove type='int'
> 
> Instead just action="store_true". Then if --use-host-bootclasspath is
specified
> the varible args.use_host_bootclasspath will just be true.

Acknowledged.

https://codereview.chromium.org/2142413004/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:263: 'specified.')
On 2016/08/22 23:42:17, mikecase wrote:
> indenting (should be aligned with text above)

Acknowledged.

[agrieve, 2016-08-23 03:52:16]

https://codereview.chromium.org/2142413004/diff/40001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/40001/build/android/gyp/javac...
build/android/gyp/javac.py:262: help='Do not use custom bootclasspath even if
bootclasspath is '
This is pretty weird. Why not just don't pass --bootclasspath? If it's for
-source and -target, then I think either just set those always, or put them
behind a separate flag.

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

https://codereview.chromium.org/2142413004/diff/40001/build/config/android/in...
build/config/android/internal_rules.gni:2022: if (_supports_android) {
Rather than adding use_host_bootclasspath, would it be sufficient to change this
condition to:
if (invoker.requires_android)?

[aluo, 2016-08-24 01:23:38]

https://codereview.chromium.org/2142413004/diff/40001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/40001/build/android/gyp/javac...
build/android/gyp/javac.py:262: help='Do not use custom bootclasspath even if
bootclasspath is '
On 2016/08/23 03:52:16, agrieve wrote:
> This is pretty weird. Why not just don't pass --bootclasspath? If it's for
> -source and -target, then I think either just set those always, or put them
> behind a separate flag.

Acknowledged.

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

https://codereview.chromium.org/2142413004/diff/40001/build/config/android/in...
build/config/android/internal_rules.gni:2022: if (_supports_android) {
On 2016/08/23 03:52:16, agrieve wrote:
> Rather than adding use_host_bootclasspath, would it be sufficient to change
this
> condition to:
> if (invoker.requires_android)?

Acknowledged.

[agrieve, 2016-08-24 01:42:30]

lgtm /w one small nit

https://codereview.chromium.org/2142413004/diff/60001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/60001/build/android/gyp/javac...
build/android/gyp/javac.py:261: default='',
nit: no need to use '' as the default. just remove this arg so that None is the
default.

[mikecase (-- gone --), 2016-08-24 15:22:36]

Before committing, talk to Yoland on whether we want to update the runner/rules
jars from v0.3 to v0.5. see: https://codereview.chromium.org/2270583003/

Otherwise lgtm

[aluo, 2016-08-24 17:13:52]

On 2016/08/24 15:22:36, mikecase wrote:
> Before committing, talk to Yoland on whether we want to update the
runner/rules
> jars from v0.3 to v0.5. see: https://codereview.chromium.org/2270583003/
> 
> Otherwise lgtm

Thanks, I pinged him.

[aluo, 2016-08-24 17:14:49]

https://codereview.chromium.org/2142413004/diff/60001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/2142413004/diff/60001/build/android/gyp/javac...
build/android/gyp/javac.py:261: default='',
On 2016/08/24 01:42:30, agrieve wrote:
> nit: no need to use '' as the default. just remove this arg so that None is
the
> default.

Acknowledged.

[the real yoland, 2016-08-24 18:47:39]

lgtm

[aluo, 2016-08-24 18:51:55]

The CQ bit was checked by aluo@chromium.org

[aluo, 2016-08-24 18:51:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
mikecase@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2142413004/#ps80001
(title: "Remove redundant default setting for java-version in javac.py")

[commit-bot: I haz the power, 2016-08-24 18:52:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 21:28:42]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-24 21:30:32]

Description was changed from

==========
Add required deps to third_party/android_support_test_runner

BUG=627916
==========

to

==========
Add required deps to third_party/android_support_test_runner

BUG=627916

Committed: https://crrev.com/24da3c36fd807a0003bece9c352567ad637f810e
Cr-Commit-Position: refs/heads/master@{#414150}
==========

[commit-bot: I haz the power, 2016-08-24 21:30:33]

Patchset 5 (id:??) landed as
https://crrev.com/24da3c36fd807a0003bece9c352567ad637f810e
Cr-Commit-Position: refs/heads/master@{#414150}