Add java-side support for _incremental instrumentation tests

Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
Committed: https://crrev.com/3bbba07e116e2c5a10303387de6bce832b191023
Cr-Commit-Position: refs/heads/master@{#374695}

[agrieve, 2016-02-09 16:58:07]

Description was changed from

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
==========

to

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
==========

[agrieve, 2016-02-09 16:58:07]

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

[agrieve, 2016-02-09 16:58:35]

On 2016/02/09 16:58:07, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:nyquist@chromium.org

Fingers cross this is the last change to this!

[nyquist, 2016-02-10 07:08:32]

lgtm

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File build/android/incremental_install/installer.py (right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/installer.py:38: return
os.path.join(constants.GetOutDirectory(), file_name)
Does constants.GetOutDirectory magically change depending on where the args.gn
file lives? Or is this something we could pass in? It seems like people using GN
tend to use a bunch of different out-directories.

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File
build/android/incremental_install/java/org/chromium/incrementalinstall/BootstrapApplication.java
(right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/BootstrapApplication.java:153:
private String getClassNameFromMetadata(String key, Context context)
Nit: Could this be static now?

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java
(right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:189:
private static boolean copyIfModified(File src, File dest) throws IOException {
Optional nit: Add a super-tiny comment as to what the return value means?

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:191:
if (!dest.exists() || dest.lastModified() != lastModified) {
Optional nit: Would it be easier to read with something like:
if (dest.exists() && dest.lastModified() == lastModified) return false;
?

[agrieve, 2016-02-10 16:47:16]

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File build/android/incremental_install/installer.py (right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/installer.py:38: return
os.path.join(constants.GetOutDirectory(), file_name)
On 2016/02/10 07:08:32, nyquist wrote:
> Does constants.GetOutDirectory magically change depending on where the args.gn
> file lives? Or is this something we could pass in? It seems like people using
GN
> tend to use a bunch of different out-directories.

It is somewhat comically named. There's a setter for it that is called based on
--output-directory, and it can also be changed with environment variables. Not
quite "constant" :P.

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File
build/android/incremental_install/java/org/chromium/incrementalinstall/BootstrapApplication.java
(right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/BootstrapApplication.java:153:
private String getClassNameFromMetadata(String key, Context context)
On 2016/02/10 07:08:32, nyquist wrote:
> Nit: Could this be static now?

Done.

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java
(right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:189:
private static boolean copyIfModified(File src, File dest) throws IOException {
On 2016/02/10 07:08:32, nyquist wrote:
> Optional nit: Add a super-tiny comment as to what the return value means?

Electing not to since it's private and is clear from the code.

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:191:
if (!dest.exists() || dest.lastModified() != lastModified) {
On 2016/02/10 07:08:32, nyquist wrote:
> Optional nit: Would it be easier to read with something like:
> if (dest.exists() && dest.lastModified() == lastModified) return false;
> ?

Done.

[jbudorick, 2016-02-10 16:53:14]

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

[jbudorick, 2016-02-10 16:53:14]

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
File build/android/incremental_install/installer.py (right):

https://codereview.chromium.org/1684583003/diff/20001/build/android/increment...
build/android/incremental_install/installer.py:38: return
os.path.join(constants.GetOutDirectory(), file_name)
On 2016/02/10 16:47:15, agrieve wrote:
> On 2016/02/10 07:08:32, nyquist wrote:
> > Does constants.GetOutDirectory magically change depending on where the
args.gn
> > file lives? Or is this something we could pass in? It seems like people
using
> GN
> > tend to use a bunch of different out-directories.
> 
> It is somewhat comically named. There's a setter for it that is called based
on
> --output-directory, and it can also be changed with environment variables. Not
> quite "constant" :P.

constants.GetOutDirectory() is one of my pet peeves that works well enough that
I haven't prioritized making it sane.

[agrieve, 2016-02-10 16:57:37]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-02-10 16:57:37]

The patchset sent to the CQ was uploaded after l-g-t-m from nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/1684583003/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2016-02-10 16:58:38]

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

[commit-bot: I haz the power, 2016-02-10 19:08:04]

Description was changed from

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
==========

to

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
==========

[commit-bot: I haz the power, 2016-02-10 19:08:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-16 22:30:19]

Description was changed from

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975
==========

to

==========
Add java-side support for _incremental instrumentation tests

Build rule and test runner changes to follow.

BUG=575975

Committed: https://crrev.com/3bbba07e116e2c5a10303387de6bce832b191023
Cr-Commit-Position: refs/heads/master@{#374695}
==========

[commit-bot: I haz the power, 2016-02-16 22:30:20]

Patchset 3 (id:??) landed as
https://crrev.com/3bbba07e116e2c5a10303387de6bce832b191023
Cr-Commit-Position: refs/heads/master@{#374695}