[Android] Move modules_unittest to isolate.

[Android] Move modules_unittest to isolate.

Also:
 - Do some cleanup
 - Move isolate remap dir to src/ and clean up during test tear down

NOTRY=True
BUG=249870, 260332
R=craigdh@chromium.org, hellner@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211756

[frankf, 2013-07-15 19:12:19]

craigdh: Everything
hellner: modules_unittest specific stuff

[hellner1, 2013-07-15 20:43:41]

Seems the cl is doing more than one thing. Might make sense to split up.

I'm not that well versed in this python script but the changes seem OK to me.

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/dispatch.py (right):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:38: 'modules_unittests':
'third_party/webrtc/modules/modules_unittests.isolate',
We are still hard coding the .isolate-path. Will this go away? E.g. WebRTC has
more than one unit test target and it would be nice to keep them as they are but
without having an explicit reference in Chromium. The way it now I would
probably need to create one webrtc target for all WebRTC unit tests (maybe that
makes sense regardless?).

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:85: return False
Function still seems to have a return value. Should this be removed?

[craigdh, 2013-07-15 20:54:54]

lgtm w/ nit.

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/test_runner.py (left):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/test_runner.py:119: logging.info('Did not find an
isolate file for the test suite.')
It might still be good to log something if the isolate deps directory doesn't
exist for debugging purposes.

[frankf, 2013-07-15 21:46:34]

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/dispatch.py (right):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:38: 'modules_unittests':
'third_party/webrtc/modules/modules_unittests.isolate',
So, all these targets have identical dependencies? Will this be true in the
future? If so, it might make sense to have a single isolate files. 

Regarding hardcoding these here: If there are many isolate files, and there's
churn in number/location (which shouldn't be), then I can add a command-line
option to take the path to the isolate files. But we like to avoid adding
options if at all possible.


On 2013/07/15 20:43:41, hellner1 wrote:
> We are still hard coding the .isolate-path. Will this go away? E.g. WebRTC has
> more than one unit test target and it would be nice to keep them as they are
but
> without having an explicit reference in Chromium. The way it now I would
> probably need to create one webrtc target for all WebRTC unit tests (maybe
that
> makes sense regardless?).

[hellner1, 2013-07-15 21:57:13]

lgtm % the comment about return value

Don't want to hold this cl up on a discussion we could take somewhere else
(regarding how to pull in .isolate-files).

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/dispatch.py (right):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:38: 'modules_unittests':
'third_party/webrtc/modules/modules_unittests.isolate',
On 2013/07/15 21:46:34, frankf wrote:
> So, all these targets have identical dependencies? Will this be true in the
> future? If so, it might make sense to have a single isolate files. 
> 
> Regarding hardcoding these here: If there are many isolate files, and there's
> churn in number/location (which shouldn't be), then I can add a command-line
> option to take the path to the isolate files. But we like to avoid adding
> options if at all possible.
> 
> 
> On 2013/07/15 20:43:41, hellner1 wrote:
> > We are still hard coding the .isolate-path. Will this go away? E.g. WebRTC
has
> > more than one unit test target and it would be nice to keep them as they are
> but
> > without having an explicit reference in Chromium. The way it now I would
> > probably need to create one webrtc target for all WebRTC unit tests (maybe
> that
> > makes sense regardless?).
> 

Can the script use the .isolate paths provided in the .gyp[i]-files? Then it
would be more similar to how it works for PC.

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:85: return False
On 2013/07/15 20:43:41, hellner1 wrote:
> Function still seems to have a return value. Should this be removed?

Please don't forget this one.

[frankf, 2013-07-15 22:06:05]

No, these test scripts are decoupled from gyp. Let's continue this in a bug.

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/dispatch.py (right):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/dispatch.py:85: return False
On 2013/07/15 20:43:41, hellner1 wrote:
> Function still seems to have a return value. Should this be removed?

Done.

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
File build/android/pylib/gtest/test_runner.py (left):

https://codereview.chromium.org/19220004/diff/3001/build/android/pylib/gtest/...
build/android/pylib/gtest/test_runner.py:119: logging.info('Did not find an
isolate file for the test suite.')
On 2013/07/15 20:54:54, craigdh wrote:
> It might still be good to log something if the isolate deps directory doesn't
> exist for debugging purposes.

Done.

[commit-bot: I haz the power, 2013-07-15 22:07:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/frankf@chromium.org/19220004/16001

[frankf, 2013-07-16 03:57:42]

Committed patchset #2 manually as r211756 (presubmit successful).

[hellner1, 2013-07-16 14:43:37]

Created https://code.google.com/p/chromium/issues/detail?id=260694 for continued
discussion.