Include isolate.py in data for Android unit tests

Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
Committed: https://crrev.com/9e299ea6bbf87415d64d9d84003162d71304ccb9
Cr-Commit-Position: refs/heads/master@{#383127}

[agrieve, 2016-03-11 18:36:12]

Description was changed from

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
==========

to

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
==========

[agrieve, 2016-03-11 18:36:13]

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

[agrieve, 2016-03-11 18:37:31]

On 2016/03/11 18:36:13, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:jbudorick@chromium.org

🙉

Note: Generating pydeps takes about 200ms, so don't want to do it as an
exec_script. I'm thinking we should add a presubmit check where if you changing
any files listed in a .pydeps, then we check whether the .pydeps need updating.

[jbudorick, 2016-03-11 19:10:06]

https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn#newc...
build/android/BUILD.gn:84: "//build/android/devil/devil_dependencies.json",
You need to rebase, this collides with
https://codereview.chromium.org/1785483005/ and
https://codereview.chromium.org/1783603003/

https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (right):

https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:2:
../../build/android/devil/android/__init__.py
also, most of these don't exist any more.

https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py
File build/print-python-deps.py (right):

https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py#...
build/print-python-deps.py:8: The primary use-case for this script is to
genererate the list of python modules
I'm really not crazy about this. I have a bad feeling that I'd going to run
afoul of it a lot :(

Why should we specify individual files instead of directories?

[agrieve, 2016-03-15 02:04:02]

https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn#newc...
build/android/BUILD.gn:84: "//build/android/devil/devil_dependencies.json",
On 2016/03/11 19:10:06, jbudorick wrote:
> You need to rebase, this collides with
> https://codereview.chromium.org/1785483005/ and
> https://codereview.chromium.org/1783603003/

Done.

https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
File build/android/test_runner.pydeps (right):

https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
build/android/test_runner.pydeps:2:
../../build/android/devil/android/__init__.py
On 2016/03/11 19:10:06, jbudorick wrote:
> also, most of these don't exist any more.

Done.

https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py
File build/print-python-deps.py (right):

https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py#...
build/print-python-deps.py:8: The primary use-case for this script is to
genererate the list of python modules
On 2016/03/11 19:10:06, jbudorick wrote:
> I'm really not crazy about this. I have a bad feeling that I'd going to run
> afoul of it a lot :(
> 
> Why should we specify individual files instead of directories?

Yeah, I'm not 100% set on it either, but I think with a presubmit check it'll be
okay.

Some reasons to do it:
1. The directories were still including
third_party/catapult/catapult_base/catapult_base/bin/reference_build
2. This avoids including files that aren't needed (e.g. python unit tests)
3. There were files missing from the manual list (e.g. appurify)

[jbudorick, 2016-03-15 02:13:39]

On 2016/03/15 02:04:02, agrieve wrote:
> https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn
> File build/android/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1784373002/diff/1/build/android/BUILD.gn#newc...
> build/android/BUILD.gn:84: "//build/android/devil/devil_dependencies.json",
> On 2016/03/11 19:10:06, jbudorick wrote:
> > You need to rebase, this collides with
> > https://codereview.chromium.org/1785483005/ and
> > https://codereview.chromium.org/1783603003/
> 
> Done.
> 
>
https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
> File build/android/test_runner.pydeps (right):
> 
>
https://codereview.chromium.org/1784373002/diff/1/build/android/test_runner.p...
> build/android/test_runner.pydeps:2:
> ../../build/android/devil/android/__init__.py
> On 2016/03/11 19:10:06, jbudorick wrote:
> > also, most of these don't exist any more.
> 
> Done.
> 
> https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py
> File build/print-python-deps.py (right):
> 
>
https://codereview.chromium.org/1784373002/diff/1/build/print-python-deps.py#...
> build/print-python-deps.py:8: The primary use-case for this script is to
> genererate the list of python modules
> On 2016/03/11 19:10:06, jbudorick wrote:
> > I'm really not crazy about this. I have a bad feeling that I'd going to run
> > afoul of it a lot :(
> > 
> > Why should we specify individual files instead of directories?
> 
> Yeah, I'm not 100% set on it either, but I think with a presubmit check it'll
be
> okay.

hm, I guess I'd be ok with doing this with a presubmit.

> 
> Some reasons to do it:
> 1. The directories were still including
> third_party/catapult/catapult_base/catapult_base/bin/reference_build
> 2. This avoids including files that aren't needed (e.g. python unit tests)
> 3. There were files missing from the manual list (e.g. appurify)

[agrieve, 2016-03-17 15:32:47]

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

[agrieve, 2016-03-17 15:32:48]

jochen@chromium.org: Please review changes in PRESUBMIT*

[agrieve, 2016-03-21 17:28:49]

On 2016/03/17 15:32:48, agrieve wrote:
> mailto:jochen@chromium.org: Please review changes in PRESUBMIT*

Monday morning ping

[agrieve, 2016-03-23 17:23:57]

On 2016/03/21 17:28:49, agrieve wrote:
> On 2016/03/17 15:32:48, agrieve wrote:
> > mailto:jochen@chromium.org: Please review changes in PRESUBMIT*
> 
> Monday morning ping

ping

[jochen (gone - plz use gerrit), 2016-03-24 14:41:58]

lgtm

[jbudorick, 2016-03-24 17:44:51]

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps.py
File build/print-python-deps.py (right):

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:1: #!/usr/bin/env python
Nit: print_python_deps.py instead of print-python-deps.py.

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:21: _SRC_ROOT =
os.path.abspath(os.path.join(__file__, os.pardir, os.pardir))
nit:

  os.path.dirname(__file__), os.pardir

instead of

  __file__, os.pardir, os.pardir

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:31: if m is not None and hasattr(m, '__file__'))
nit: Do you need to check against None specifically, or can this just be

  if m and hasattr(m, '__file__')

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:60: out = open(options.output, 'w') if options.output
else sys.stdout
This file should be closed after you're done writing to it.

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:65: out.write(path + "\n")
nit: single quotes

[agrieve, 2016-03-24 18:47:39]

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps.py
File build/print-python-deps.py (right):

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:21: _SRC_ROOT =
os.path.abspath(os.path.join(__file__, os.pardir, os.pardir))
On 2016/03/24 17:44:51, jbudorick wrote:
> nit:
> 
>   os.path.dirname(__file__), os.pardir
> 
> instead of
> 
>   __file__, os.pardir, os.pardir

Done.

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:31: if m is not None and hasattr(m, '__file__'))
On 2016/03/24 17:44:51, jbudorick wrote:
> nit: Do you need to check against None specifically, or can this just be
> 
>   if m and hasattr(m, '__file__')

Done.

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:60: out = open(options.output, 'w') if options.output
else sys.stdout
On 2016/03/24 17:44:51, jbudorick wrote:
> This file should be closed after you're done writing to it.

Done.

https://codereview.chromium.org/1784373002/diff/60001/build/print-python-deps...
build/print-python-deps.py:65: out.write(path + "\n")
On 2016/03/24 17:44:51, jbudorick wrote:
> nit: single quotes

Done.

[jbudorick, 2016-03-24 19:12:24]

lgtm

[agrieve, 2016-03-24 19:22:00]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-03-24 19:22:00]

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

[commit-bot: I haz the power, 2016-03-24 19:22:32]

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

[commit-bot: I haz the power, 2016-03-24 20:06:48]

Description was changed from

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
==========

to

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
==========

[commit-bot: I haz the power, 2016-03-24 20:06:54]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-24 20:08:13]

Description was changed from

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318
==========

to

==========
Include isolate.py in data for Android unit tests

This is required for any test that uses an .isolate to push files to the
device (e.g. base_unittests).

BUG=589318

Committed: https://crrev.com/9e299ea6bbf87415d64d9d84003162d71304ccb9
Cr-Commit-Position: refs/heads/master@{#383127}
==========

[commit-bot: I haz the power, 2016-03-24 20:08:14]

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

[agrieve, 2016-03-29 15:08:56]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1839103002/ by agrieve@chromium.org.

The reason for reverting is: PRESUBMIT isn't working for non-android checkouts,
as shown by:

https://codereview.chromium.org/1840913002/diff/1/build/android/test_runner.p....